octopus: os/bluestore: fix additional errors during missed shared blob repair.

src/os/bluestore/BlueStore.cc

@@ -8798,9 +8798,9 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
 	  expected_statfs = &expected_pool_statfs[sbi.pool_id];
 	}
 	errors += _fsck_check_extents(sbi.cid,
-				      p->second.oids.front(),
+				      sbi.oids.front(),
 				      extents,
-				      p->second.compressed,
+				      sbi.compressed,
 				      used_blocks,
 				      fm->get_alloc_size(),
 				      repair ? &repairer : nullptr,
@@ -9025,9 +9025,32 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
 	  bluestore_shared_blob_t persistent(sbid, std::move(sbi.ref_map));
 	  encode(persistent, bl);
 	  dout(20) << __func__ << " " << *sbi.sb
-		   << " is " << bl.length() << " bytes, updating" << dendl;
+		   << " is " << bl.length() << " bytes, updating"
+		   << dendl;
 
 	  repairer.fix_shared_blob(db, sbid, &bl);
+	  // we need to account for shared blob pextents at both
+	  // stats and used blocks to avoid related errors.
+	  PExtentVector extents;
+	  for (auto& r : persistent.ref_map.ref_map) {
+	    extents.emplace_back(bluestore_pextent_t(r.first, r.second.length));
+	  }
+	  auto* expected_statfs = &expected_pool_statfs[sbi.pool_id];
+	  int errors = _fsck_check_extents(sbi.cid,
+	    ghobject_t(), // doesn't matter
+	    extents,
+	    sbi.compressed,
+	    used_blocks,
+	    fm->get_alloc_size(),
+	    nullptr,
+	    *expected_statfs,
+	    depth);
+	  if (errors) {
+	    derr << __func__ << " " << errors
+	         << "  unexpected error(s) after missed shared blob repair,"
+	         << " perhaps worth one more repair attempt"
+	         << dendl;
+	  }
         }
       }
     }
@@ -9266,6 +9289,23 @@ void BlueStore::inject_broken_shared_blob_key(const string& key,
   db->submit_transaction_sync(txn);
 };
 
+void BlueStore::inject_no_shared_blob_key()
+{
+  KeyValueDB::Transaction txn;
+  txn = db->get_transaction();
+  ceph_assert(blobid_last > 0);
+  // kill the last used sbid, this can be broken due to blobid preallocation
+  // in rare cases, leaving as-is for the sake of simplicity
+  uint64_t sbid = blobid_last;
+
+  string key;
+  dout(5) << __func__<< " " << sbid << dendl;
+  get_shared_blob_key(sbid, &key);
+  txn->rmkey(PREFIX_SHARED_BLOB, key);
+  db->submit_transaction_sync(txn);
+};
+
+
 void BlueStore::inject_leaked(uint64_t len)
 {
   KeyValueDB::Transaction txn;

src/os/bluestore/BlueStore.h

@@ -2866,6 +2866,8 @@ class BlueStore : public ObjectStore,
   /// methods to inject various errors fsck can repair
   void inject_broken_shared_blob_key(const string& key,
 			 const bufferlist& bl);
+  void inject_no_shared_blob_key();
+
   void inject_leaked(uint64_t len);
   void inject_false_free(coll_t cid, ghobject_t oid);
   void inject_statfs(const string& key, const store_statfs_t& new_statfs);

src/test/objectstore/store_test.cc

@@ -7722,6 +7722,69 @@ TEST_P(StoreTestSpecificAUSize, BluestoreBrokenZombieRepairTest) {
   bstore->mount();
 }
 
+TEST_P(StoreTestSpecificAUSize, BluestoreBrokenNoSharedBlobRepairTest) {
+  if (string(GetParam()) != "bluestore")
+    return;
+
+  SetVal(g_conf(), "bluestore_fsck_on_mount", "false");
+  SetVal(g_conf(), "bluestore_fsck_on_umount", "false");
+
+  StartDeferred(0x10000);
+
+  BlueStore* bstore = dynamic_cast<BlueStore*> (store.get());
+
+  int r;
+
+  // initializing
+  cerr << "initializing" << std::endl;
+  {
+    const uint64_t pool = 555;
+    coll_t cid(spg_t(pg_t(0, pool), shard_id_t::NO_SHARD));
+    auto ch = store->create_new_collection(cid);
+
+    ghobject_t hoid = make_object("Object", pool);
+    ghobject_t hoid_cloned = hoid;
+    hoid_cloned.hobj.snap = 1;
+
+    {
+      ObjectStore::Transaction t;
+      t.create_collection(cid, 0);
+      r = queue_transaction(store, ch, std::move(t));
+      ASSERT_EQ(r, 0);
+    }
+    {
+      ObjectStore::Transaction t;
+      bufferlist bl;
+      bl.append("0123456789012345");
+      t.write(cid, hoid, 0, bl.length(), bl);
+
+      r = queue_transaction(store, ch, std::move(t));
+      ASSERT_EQ(r, 0);
+    }
+    {
+      ObjectStore::Transaction t;
+      t.clone(cid, hoid, hoid_cloned);
+      r = queue_transaction(store, ch, std::move(t));
+      ASSERT_EQ(r, 0);
+    }
+  }
+  // injecting an error and checking
+  cerr << "injecting" << std::endl;
+  sleep(3); // need some time for the previous write to land
+  bstore->inject_no_shared_blob_key();
+
+  {
+    cerr << "fscking/fixing" << std::endl;
+    bstore->umount();
+    ASSERT_EQ(bstore->fsck(false), 3);
+    ASSERT_LE(bstore->repair(false), 3);
+    ASSERT_EQ(bstore->fsck(false), 0);
+  }
+
+  cerr << "Completing" << std::endl;
+  bstore->mount();
+}
+
 TEST_P(StoreTest, BluestoreRepairGlobalStats) {
   if (string(GetParam()) != "bluestore")
     return;