osd/ReplicatedPG: do not hit_set_persist while potentially backfilling hit_set_*

The hit_set transactions may include both a modify of the new hit_set and
deletion of an old one, spanning the backfill boundary, and we may end up
sending a backfill target a blank transaction that does not correctly
remove the old object.  Later it will notice the stray object and
throw an assertion.

Fix this by skipping hit_set_persist() if any of the backfill targets are
still working on the very first hash value in the PG (which is where all
of the hit_set objects live).  This is coarse but simple.

Another solution would be to send separate ops for the trim/deletion and
new hit_set update, but that is a bit more complex and a bit more
runtime overhead (twice the messages).

Fixes: #7983
Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index ed07e82fd7c9352a5db4d3cb9848dbf0de51a9d6..1b798c1116a138efe9fd18eb8e471963440f1926 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -10390,6 +10390,26 @@ void ReplicatedPG::hit_set_persist()
   if (is_degraded_object(oid))
     return;
 
+  // If backfill is in progress and we could possibly overlap with the
+  // hit_set_* objects, back off.  Since these all have
+  // hobject_t::hash set to pgid.ps(), and those sort first, we can
+  // look just at that.  This is necessary because our transactions
+  // may include a modify of the new hit_set *and* a delete of the
+  // old one, and this may span the backfill boundary.
+  for (set<pg_shard_t>::iterator p = backfill_targets.begin();
+       p != backfill_targets.end();
+       ++p) {
+    assert(peer_info.count(*p));
+    const pg_info_t& pi = peer_info[*p];
+    if (pi.last_backfill == hobject_t() ||
+       pi.last_backfill.hash == info.pgid.ps()) {
+      dout(10) << __func__ << " backfill target osd." << *p
+              << " last_backfill has not progressed past pgid ps"
+              << dendl;
+      return;
+    }
+  }
+
   if (!info.hit_set.current_info.begin)
     info.hit_set.current_info.begin = hit_set_start_stamp;