From: Samuel Just <sjust@redhat.com>
Date: Mon, 3 Oct 2016 17:34:51 +0000 (-0700)
Subject: PG: block writes and scan log for scrub using inclusive upper bound
X-Git-Tag: v10.2.4~78^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=8833c64459edb77fa0d394b2eda2f79cd0f6dba9;p=ceph.git

PG: block writes and scan log for scrub using inclusive upper bound

See comment in commit.

Signed-off-by: Samuel Just <sjust@redhat.com>
---

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index c5b73247251d6..d72b91fc3cc95 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -3482,10 +3482,16 @@ void PG::sub_op_scrub_map(OpRequestRef op)
   dout(10) << " got " << m->from << " scrub map" << dendl;
   bufferlist::iterator p = m->get_data().begin();
 
-  scrubber.received_maps[m->from].decode(p, info.pgid.pool());
+  ScrubMap &map = scrubber.received_maps[m->from];
+  map.reset_bitwise(get_sort_bitwise());
+  map.decode(p, info.pgid.pool());
   dout(10) << "map version is "
-	     << scrubber.received_maps[m->from].valid_through
-	     << dendl;
+	   << map.valid_through
+	   << dendl;
+
+  // Account for http://tracker.ceph.com/issues/17491
+  if (!map.objects.empty() && map.objects.rbegin()->first == scrubber.end)
+    map.objects.erase(scrubber.end);
 
   --scrubber.waiting_on;
   scrubber.waiting_on_whom.erase(m->from);
@@ -4172,7 +4178,7 @@ void PG::chunky_scrub(ThreadPool::TPHandle &handle)
 	  if (!_range_available_for_scrub(scrubber.start, candidate_end)) {
 	    // we'll be requeued by whatever made us unavailable for scrub
 	    dout(10) << __func__ << ": scrub blocked somewhere in range "
-		     << "[" << scrubber.start << ", " << candidate_end << ")"
+		     << "[" << scrubber.start << ", " << candidate_end << "]"
 		     << dendl;
 	    done = true;
 	    break;
@@ -4186,7 +4192,8 @@ void PG::chunky_scrub(ThreadPool::TPHandle &handle)
              p != pg_log.get_log().log.rend();
              ++p) {
           if (cmp(p->soid, scrubber.start, get_sort_bitwise()) >= 0 &&
-	      cmp(p->soid, scrubber.end, get_sort_bitwise()) < 0) {
+	      cmp(p->soid, scrubber.end, get_sort_bitwise()) <= 0) {
+	    // inclusive upper bound, @see write_blocked_by_scrub
             scrubber.subset_last_update = p->version;
             break;
           }
diff --git a/src/osd/PG.h b/src/osd/PG.h
index cda845f68f175..7cb85b9e9c915 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -1230,11 +1230,28 @@ public:
 
     bool is_chunky_scrub_active() const { return state != INACTIVE; }
 
-    // classic (non chunk) scrubs block all writes
-    // chunky scrubs only block writes to a range
+    /* We use an inclusive upper bound here because the replicas scan .end
+     * as well in hammer (see http://tracker.ceph.com/issues/17491).
+     *
+     * The boundary can only be
+     * 1) Not an object (object boundary) or
+     * 2) A clone
+     * In case 1), it doesn't matter.  In case 2), we might fail to
+     * wait for an un-applied snap trim to complete, or fail to block an
+     * eviction on a tail object.  In such a case the replica might
+     * erroneously detect a snap_mapper/attr mismatch and "fix" the
+     * snap_mapper to the old value.
+     *
+     * @see _range_available_for_scrub
+     * @see chunk_scrub (the part where it determines the last relelvant log
+     *      entry)
+     *
+     * TODO: switch this logic back to an exclusive upper bound once the
+     * replicas don't scan the upper boundary
+     */
     bool write_blocked_by_scrub(const hobject_t &soid, bool sort_bitwise) {
       if (cmp(soid, start, sort_bitwise) >= 0 &&
-	  cmp(soid, end, sort_bitwise) < 0)
+	  cmp(soid, end, sort_bitwise) <= 0)
 	return true;
 
       return false;
diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 6dfa97b7cb0bc..f04ef6028a93e 100644
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -12487,7 +12487,8 @@ bool ReplicatedPG::_range_available_for_scrub(
   next.second = object_contexts.lookup(begin);
   next.first = begin;
   bool more = true;
-  while (more && cmp(next.first, end, get_sort_bitwise()) < 0) {
+  // inclusive upper bound, @see write_blocked_by_scrub
+  while (more && cmp(next.first, end, get_sort_bitwise()) <= 0) {
     if (next.second && next.second->is_blocked()) {
       next.second->requeue_scrub_on_unblock = true;
       dout(10) << __func__ << ": scrub delayed, "
diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index 2334b70ed33b9..b9222fed117ed 100644
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -5292,7 +5292,7 @@ void ScrubMap::decode(bufferlist::iterator& bl, int64_t pool)
 
   // handle hobject_t upgrade
   if (struct_v < 3) {
-    map<hobject_t, object, hobject_t::ComparatorWithDefault> tmp;
+    map<hobject_t, object, hobject_t::ComparatorWithDefault> tmp(objects.key_comp());
     tmp.swap(objects);
     for (map<hobject_t, object, hobject_t::ComparatorWithDefault>::iterator i = tmp.begin();
 	 i != tmp.end();