From: Sage Weil Date: Tue, 11 Apr 2017 19:20:34 +0000 (-0400) Subject: osd/osd_types: simplify bounds X-Git-Tag: v12.0.3~129^2~14 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bb2a2fe2679d12aa1f88582943f1488a4ed8cf27;p=ceph.git osd/osd_types: simplify bounds The bounds are based on last_epoch_clean, which can happen at any point during an interval (usually not the beginning!). Instead of trying to ensure that the PastIntervals include the oldest interval, just ensure that they go at least as far back as last_epoch_clean. This means that we might have *more* intervals, but given that all we ever do is *clear* past_intervals when we go clean, I don't think there is much value in trying assert more. Signed-off-by: Sage Weil --- diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 42a346aa103b..5ed8d46f8d3a 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -3731,7 +3731,7 @@ void OSD::build_past_intervals_parallel() } else { auto apib = pg->past_intervals.get_bounds(); if (rpib.second == apib.second && - apib.first.first <= rpib.first) { + apib.first <= rpib.first) { if (pg->info.history.same_interval_since == 0) { pg->info.history.same_interval_since = rpib.second; } diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 6496f45e9101..f36e97de65a8 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -722,46 +722,46 @@ void PG::check_past_interval_bounds() const auto rpib = get_required_past_interval_bounds( info, osd->get_superblock().oldest_map); - if (rpib.first == rpib.second) { + if (rpib.first >= rpib.second) { if (!past_intervals.empty()) { osd->clog->error() << info.pgid << " required past_interval bounds are" - << " empty [" << rpib << ") but past_intervals is not " + << " empty [" << rpib << ") but past_intervals is not: " << past_intervals; - derr << info.pgid << " required past_interval bounds are " - << " empty [" << rpib << ") but past_intervals is not " + derr << info.pgid << " required past_interval bounds are" + << " empty [" << rpib << ") but past_intervals is not: " << past_intervals << dendl; assert(past_intervals.empty()); } - } - if (past_intervals.empty()) { - osd->clog->error() << info.pgid << " required past_interval bounds are" - << " not empty [" << rpib << ") but past_intervals " - << past_intervals << " is empty"; - derr << info.pgid << " required past_interval bounds are " - << " not empty [" << rpib << ") but past_intervals " - << past_intervals << " is empty" << dendl; - assert(!past_intervals.empty()); - } - auto apib = past_intervals.get_bounds(); - - if ((apib.first.first > rpib.first) || - (apib.first.second <= rpib.first)) { - osd->clog->error() << info.pgid << " past_intervals [" << apib - << ") start interval does not contain the required" - << " bound [" << rpib << ") start"; - derr << info.pgid << " past_intervals [" << apib - << ") start interval does not contain the required" - << " bound [" << rpib << ") start" << dendl; - assert(0 == "past_interval start interval mismatch"); - } - if (apib.second != rpib.second) { - osd->clog->error() << info.pgid << " past_interal bound [" << apib - << ") end does not match required [" << rpib - << ") end"; - derr << info.pgid << " past_interal bound [" << apib - << ") end does not match required [" << rpib - << ") end" << dendl; - assert(0 == "past_interval end mismatch"); + } else { + if (past_intervals.empty()) { + osd->clog->error() << info.pgid << " required past_interval bounds are" + << " not empty [" << rpib << ") but past_intervals " + << past_intervals << " is empty"; + derr << info.pgid << " required past_interval bounds are" + << " not empty [" << rpib << ") but past_intervals " + << past_intervals << " is empty" << dendl; + assert(!past_intervals.empty()); + } + + auto apib = past_intervals.get_bounds(); + if (apib.first > rpib.first) { + osd->clog->error() << info.pgid << " past_intervals [" << apib + << ") start interval does not contain the required" + << " bound [" << rpib << ") start"; + derr << info.pgid << " past_intervals [" << apib + << ") start interval does not contain the required" + << " bound [" << rpib << ") start" << dendl; + assert(0 == "past_interval start interval mismatch"); + } + if (apib.second != rpib.second) { + osd->clog->error() << info.pgid << " past_interal bound [" << apib + << ") end does not match required [" << rpib + << ") end"; + derr << info.pgid << " past_interal bound [" << apib + << ") end does not match required [" << rpib + << ") end" << dendl; + assert(0 == "past_interval end mismatch"); + } } } diff --git a/src/osd/PG.h b/src/osd/PG.h index 9614444b184c..fd058616c21e 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -913,16 +913,13 @@ public: const pg_info_t &info, epoch_t oldest_map) { epoch_t start = MAX( - info.history.last_epoch_started, - MAX(oldest_map, info.history.epoch_created)); + info.history.last_epoch_clean ? info.history.last_epoch_clean : + info.history.epoch_created, + oldest_map); epoch_t end = MAX( info.history.same_interval_since, info.history.epoch_created); - if (start == end) { - return make_pair(0, 0); - } else { - return make_pair(start, end); - } + return make_pair(start, end); } void check_past_interval_bounds() const; PastIntervals::PriorSet build_prior(); diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc index 379d76183f7d..334e0f561f79 100644 --- a/src/osd/osd_types.cc +++ b/src/osd/osd_types.cc @@ -2879,16 +2879,16 @@ public: size_t size() const override { return interval_map.size(); } bool empty() const override { return interval_map.empty(); } void clear() override { interval_map.clear(); } - pair, epoch_t> get_bounds() const override { + pair get_bounds() const override { auto iter = interval_map.begin(); if (iter != interval_map.end()) { auto riter = interval_map.rbegin(); return make_pair( - make_pair(iter->second.first, iter->second.last + 1), + iter->second.first, riter->second.last + 1); } else { assert(0 == "get_bounds only valid if !empty()"); - return make_pair(make_pair(0, 0), 0); + return make_pair(0, 0); } } set get_all_participants( @@ -3096,8 +3096,8 @@ public: void clear() override { *this = pi_compact_rep(); } - pair, epoch_t> get_bounds() const override { - return make_pair(make_pair(first, first + 1), last + 1); + pair get_bounds() const override { + return make_pair(first, last + 1); } set get_all_participants( bool ec_pool) const override { diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index c860f8cfe8cc..196f90f4d9ce 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -2528,7 +2528,7 @@ public: virtual size_t size() const = 0; virtual bool empty() const = 0; virtual void clear() = 0; - virtual pair, epoch_t> get_bounds() const = 0; + virtual pair get_bounds() const = 0; virtual set get_all_participants( bool ec_pool) const = 0; virtual void add_interval(bool ec_pool, const pg_interval_t &interval) = 0; @@ -2718,13 +2718,10 @@ public: return past_intervals->get_all_participants(ec_pool); } - /* Return the set of epochs - * [[start_interval_start, start_interval_end), end) represented by the - * past_interval set. Annoyingly, pg_info_t records last_epoch_started, - * but doesn't pin it to the start of the interval, so we have to return - * the first interval so a user can verify that last_epoch_started falls - * within it */ - pair, epoch_t> get_bounds() const { + /* Return the set of epochs [start, end) represented by the + * past_interval set. + */ + pair get_bounds() const { assert(past_intervals); return past_intervals->get_bounds(); }