PG: scrubber.end should be exactly a boundary

Let scrubber.end be (foo, HEAD, 10) where the oid is foo , HEAD is the
snap, and 10 is the hash and scrubber.begin similarly be (bar, 5, 1).

After choosing to scan [(bar, 5, 1), (foo, HEAD, 10)), we block writes
on that interval.

1) A write might then come in for foo (which isn't blocked) which
creates a new snap (foo, 400, 10) which happens to fall in the interval.
This will result in a crash in _scrub() when it attempts to compare
clones since it will get (foo, 400, 10) but not the head object
(foo, HEAD, 10).

2) Alternately, the write from 1) has already happened.  When we scan
the log, we find 34'10 and 34'11 are the clone operation creating
(foo, 400, 10) and the modify on (foo, HEAD, 10) respectively.  Both
primary and replica will wait for last_update_applied to be 34'10
before scanning, but last_update_applied will in fact skip to 34'11
since 34'10 and 34'11 happened in the same transaction.  This can
result in IO hanging on the scrubber interval.

Instead, we ensure that scrubber.end is exactly a hash boundary
(min hobject_t a with the specified hash).  No such object can
exist since we don't create objects with empty oids, so no writes
can occur on that object.

Signed-off-by: Samuel Just <sam.just@inktank.com>

2 files changed, 10 insertions, 1 deletions

diff --git a/src/os/hobject.h b/src/os/hobject.h
index 00097f47157..d75ae8570c4 100644
--- a/src/os/hobject.h
+++ b/src/os/hobject.h
@@ -59,6 +59,15 @@ public:
     pool(pool),
     key(soid.oid.name == key ? string() : key) {}
 
+  /// @return min hobject_t ret s.t. ret.hash == this->hash
+  hobject_t get_boundary() const {
+    if (is_max())
+      return *this;
+    hobject_t ret;
+    ret.hash = hash;
+    return ret;
+  }
+
   /* Do not use when a particular hash function is needed */
   explicit hobject_t(const sobject_t &o) :
     oid(o.oid), snap(o.snap), max(false), pool(-1) {
diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 2bc5d127c9c..b81bfed36f4 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -3694,7 +3694,7 @@ void PG::chunky_scrub() {
             // search backward from the end looking for a boundary
             objects.push_back(scrubber.end);
             while (!boundary_found && objects.size() > 1) {
-              hobject_t end = objects.back();
+              hobject_t end = objects.back().get_boundary();
               objects.pop_back();
 
               if (objects.back().get_filestore_key() != end.get_filestore_key()) {