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 <sage@redhat.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 42a346aa103bda97e1c74aa87a8c82fad8ec7406..5ed8d46f8d3aa28c4927db3244d7bab8472669b0 100644 (file)
--- 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 6496f45e910157c7200e60a9bae6d275513a0a32..f36e97de65a83f413f2ae1179b37e55ab46a911d 100644 (file)
--- 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 9614444b184ce26d7bbf501c9284de29998f9dab..fd058616c21ee59a9292e5749ba5b3a821547e25 100644 (file)
--- 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 379d76183f7dd74de9d820c82d70b74b79ada920..334e0f561f797cdf9495e4e9d32057dea3a877e6 100644 (file)
--- 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<pair<epoch_t, epoch_t>, epoch_t> get_bounds() const override {
+  pair<epoch_t, epoch_t> 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<pg_shard_t> get_all_participants(
@@ -3096,8 +3096,8 @@ public:
   void clear() override {
     *this = pi_compact_rep();
   }
-  pair<pair<epoch_t, epoch_t>, epoch_t> get_bounds() const override {
-    return make_pair(make_pair(first, first + 1), last + 1);
+  pair<epoch_t, epoch_t> get_bounds() const override {
+    return make_pair(first, last + 1);
   }
   set<pg_shard_t> get_all_participants(
     bool ec_pool) const override {

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index c860f8cfe8cc9135d3bb14bc04e63e294b206746..196f90f4d9ce13deed0329b61139a8984580edb0 100644 (file)
--- 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<pair<epoch_t, epoch_t>, epoch_t> get_bounds() const = 0;
+    virtual pair<epoch_t, epoch_t> get_bounds() const = 0;
     virtual set<pg_shard_t> 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<pair<epoch_t, epoch_t>, epoch_t> get_bounds() const {
+  /* Return the set of epochs [start, end) represented by the
+   * past_interval set.
+   */
+  pair<epoch_t, epoch_t> get_bounds() const {
     assert(past_intervals);
     return past_intervals->get_bounds();
   }