From 0b90ee74e06cc5a3a30088fa39fde5634148bda1 Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Mon, 27 May 2024 13:21:41 +0000 Subject: [PATCH] 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 --- src/crimson/osd/object_context.h | 10 +++++++++- src/crimson/osd/object_context_loader.cc | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h index e1a3cc92987..b2236933851 100644 --- 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 b53cbabd04c..acf59637e2a 100644 --- 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(obc); - if (existed) { + if (existed && obc->is_loaded()) { if (!obc->is_loaded_and_valid()) { ERRORDPP( "obc for {} invalid -- fully_loaded={}, " -- 2.39.5