crimson/osd/object_context_loader: Fix obc cache existence usage

with_head_obc() uses get_cached_obc() to get_or_create obc instances.

If the obc exists in cache, get_or_load_obc is called with `existed`=true.
The assumption above is wrong.
Cache existence (`existed`) only guarantees that the obc instance was created (and inserted) in the obc_registery.
However, it does **not** assure that the obc was actually loaded.

As obc-loading is now concurrent, it's possible for the first user to only create the obc in
cache (without loading yet) and the second concurrent user to assume it was already loaded.

With this patch, we verify that the obc was loaded in get_or_load_obc.

* make loading-obc concurrent PR: https://github.com/ceph/ceph/pull/55488

Fixes: https://tracker.ceph.com/issues/64206
Fixes: https://tracker.ceph.com/issues/66214
Signed-off-by: Matan Breizman <mbreizma@redhat.com>
(cherry picked from commit 0b90ee74e06cc5a3a30088fa39fde5634148bda1)

diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h
index e1a3cc9298784e9486026e6607045a31936585d6..b223693385198dd9fd6618c3690210d6bb570b47 100644 (file)
--- a/src/crimson/osd/object_context.h
+++ b/src/crimson/osd/object_context.h
@@ -117,8 +117,16 @@ public:
     }
   }
 
+  bool is_loaded() const {
+    return fully_loaded;
+  }
+
+  bool is_valid() const {
+    return !invalidated_by_interval_change;
+  }
+
   bool is_loaded_and_valid() const {
-    return fully_loaded && !invalidated_by_interval_change;
+    return is_loaded() && is_valid();
   }
 
 private:

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index b53cbabd04c91c7d3b2dca655a343b650b861ba4..acf59637e2af9d03a196c8c1e54d7e076ac477d4 100644 (file)
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -146,7 +146,7 @@ using crimson::common::local_conf;
     LOG_PREFIX(ObjectContextLoader::get_or_load_obc);
     auto loaded =
       load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
-    if (existed) {
+    if (existed && obc->is_loaded()) {
       if (!obc->is_loaded_and_valid()) {
        ERRORDPP(
          "obc for {} invalid -- fully_loaded={}, "