From 06ac915bf49666f0569db9f770a257cb6b830d5a Mon Sep 17 00:00:00 2001 From: Mikhail Filimonov Date: Tue, 13 Oct 2026 02:24:30 +0200 Subject: [PATCH 1/3] Refuse to republish a namespace life retired during a cold admission A cold reader could see the catalog row `Live`, then lose the CPU before `acquireRefTableRuntime` took `ref_queue_mutex`. Removal plus the GC drain retired the life and erased the slot, so the reader inserted a fresh runtime for the dead life. A warm `namespaceLife` then returned it and `putNamespaceFile` acknowledged bytes the dead-life janitor later deletes. - `RefNameSlot` keeps a per-name `retirements` count; `invalidateRemovedCatalogLife` bumps it instead of erasing the slot, also when no runtime is resident. Eviction keeps slots with a non-zero count. - Every cold path captures the count before its first catalog GET; `acquireRefTableRuntime` throws retry-later if it moved. - `namespaceLife` refuses a recovered runtime whose terminal removal is applied. Draft: not built or tested yet. Signed-off-by: Mikhail Filimonov Co-Authored-By: Claude Fable 5.1 --- .../ContentAddressed/Pool/CasRefLedger.cpp | 74 ++++++++++++++---- .../ContentAddressed/Pool/CasRefLedger.h | 27 +++++-- .../tests/gtest_cas_ns_file_incarnation.cpp | 76 +++++++++++++++++++ 3 files changed, 154 insertions(+), 23 deletions(-) diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp index 196984a9aa68c..31031c3422cef 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp @@ -565,20 +565,31 @@ std::shared_ptr CasRefLedger::lookupRefTableRunti return it == ref_name_slots.end() ? nullptr : it->second.current; } +uint64_t CasRefLedger::refNameRetirements(const RootNamespace & ns) const +{ + std::lock_guard lock(ref_queue_mutex); + const auto it = ref_name_slots.find(ns.string()); + return it == ref_name_slots.end() ? 0 : it->second.retirements; +} + std::shared_ptr CasRefLedger::acquireRefTableRuntime( - const NamespaceLifeId & life, uint64_t admitted_generation) + const NamespaceLifeId & life, uint64_t admitted_generation, uint64_t retirements_seen) { refuseUnlessAdmitted(mount_requests.resume(admitted_generation), "ref-table runtime install"); std::shared_ptr result; bool generation_moved = false; + bool retired_since_observation = false; bool identity_conflict = false; { std::lock_guard lock(ref_queue_mutex); generation_moved = fence_generation_fn() != admitted_generation; - if (!generation_moved) + const auto it = ref_name_slots.find(life.ns.string()); + /// The catalog row this life came from may have been removed and drained after the caller read + /// it; inserting then would publish a runtime for a life the janitor sweeps. + retired_since_observation = (it == ref_name_slots.end() ? 0 : it->second.retirements) != retirements_seen; + if (!generation_moved && !retired_since_observation) { - const auto it = ref_name_slots.find(life.ns.string()); if (it != ref_name_slots.end() && it->second.current) { const auto & current = it->second.current; @@ -596,13 +607,18 @@ std::shared_ptr CasRefLedger::acquireRefTableRunt next_ref_runtime_id.fetch_add(1, std::memory_order_relaxed) + 1, life, admitted_generation); - ref_name_slots.emplace(life.ns.string(), RefNameSlot{.current = result}); + ref_name_slots[life.ns.string()].current = result; } } } if (generation_moved) refuseUnlessAdmitted(mount_requests.resume(admitted_generation), "ref-table runtime install"); + if (retired_since_observation) + throwCasWriteRetryLater(fmt::format( + "CAS namespace '{}': a life of this name was retired while life {} was being admitted; " + "retry from a fresh catalog observation", + life.ns.string(), renderIncarnation(life.incarnation))); if (identity_conflict) throwCasWriteRetryLater(fmt::format( "CAS namespace '{}': the cached runtime identity changed while publishing catalog life {}; " @@ -636,6 +652,7 @@ std::shared_ptr CasRefLedger::acquireReadableRefT } const uint64_t admitted_generation = fence_generation_fn(); + const uint64_t retirements_seen = refNameRetirements(ns); CasOperation op = mount_requests.resume(admitted_generation); const CasRefCatalog::Snapshot first_catalog = CasRefCatalog::read(op, layout); refuseUnlessAdmitted(op, "cold readable runtime admission"); @@ -672,20 +689,22 @@ std::shared_ptr CasRefLedger::acquireReadableRefT "retry from a fresh catalog observation", ns.string(), renderIncarnation(life.incarnation))); - return acquireRefTableRuntime(life, admitted_generation); + return acquireRefTableRuntime(life, admitted_generation, retirements_seen); } std::shared_ptr CasRefLedger::acquireMutableRefTableRuntime( const RootNamespace & ns) { + /// Captured before `namespaceLife`, whose answer may come from a runtime retired right after. + const uint64_t admitted_generation = fence_generation_fn(); + const uint64_t retirements_seen = refNameRetirements(ns); const NamespaceLifeId life = namespaceLife(ns); if (const auto current = lookupRefTableRuntime(ns)) { if (current->life == life) return current; } - const uint64_t admitted_generation = fence_generation_fn(); - return acquireRefTableRuntime(life, admitted_generation); + return acquireRefTableRuntime(life, admitted_generation, retirements_seen); } void CasRefLedger::invalidateRemovedCatalogLife(const NamespaceLifeId & life) @@ -693,12 +712,15 @@ void CasRefLedger::invalidateRemovedCatalogLife(const NamespaceLifeId & life) std::shared_ptr rt; { std::lock_guard lock(ref_queue_mutex); - const auto it = ref_name_slots.find(life.ns.string()); - if (it == ref_name_slots.end()) - return; - rt = it->second.current; + /// Recorded even with no resident runtime: a cold admission may be between its catalog read + /// and its insertion. + RefNameSlot & slot = ref_name_slots[life.ns.string()]; + rt = slot.current; if (!rt) + { + ++slot.retirements; return; + } } /// Invalidation must neither materialize a cache entry nor perform catalog I/O on GC's thread. @@ -718,7 +740,10 @@ void CasRefLedger::invalidateRemovedCatalogLife(const NamespaceLifeId & life) std::lock_guard queue_lock(ref_queue_mutex); const auto it = ref_name_slots.find(life.ns.string()); if (it != ref_name_slots.end() && it->second.current == rt) - ref_name_slots.erase(it); + { + it->second.current.reset(); + ++it->second.retirements; + } } rt->cv.notify_all(); rt->recovery_cv.notify_all(); @@ -1806,7 +1831,8 @@ void CasRefLedger::enforceRefTableCacheBudget(const RootNamespace & keep_ns) continue; /// re-check under the still-held ref_queue_mutex total -= c.weight; evicted.push_back(std::move(rt)); /// keep alive past the erase and lock release - ref_name_slots.erase(it); + if (it->second.retirements == 0) + ref_name_slots.erase(it); ProfileEvents::increment(ProfileEvents::CASRefTableEvictions); } } @@ -4943,12 +4969,14 @@ NamespaceLifeId CasRefLedger::namespaceLife(const RootNamespace & ns) if (rt) { ensureRefTableRecovered(ns, *rt); + refuseTerminalLife(ns, *rt); return rt->life; } } /// A cold mutation observes or births the durable identity before allocating any local state. const uint64_t admitted_generation = fence_generation_fn(); + const uint64_t retirements_seen = refNameRetirements(ns); CasOperation op = mount_requests.resume(admitted_generation); const CasRefCatalog::Snapshot catalog = CasRefCatalog::read(op, layout); refuseUnlessAdmitted(op, "cold mutable namespace admission"); @@ -4964,11 +4992,23 @@ NamespaceLifeId CasRefLedger::namespaceLife(const RootNamespace & ns) ? NamespaceLifeId::fromCatalogEntry(entry_it->ns, entry_it->incarnation) : resolveNamespaceLife(ns, admitted_generation, live_epoch_fn()); refuseUnlessAdmitted(op, "cold mutable namespace admission, after life resolution"); - rt = acquireRefTableRuntime(life, admitted_generation); + rt = acquireRefTableRuntime(life, admitted_generation, retirements_seen); ensureRefTableRecovered(ns, *rt); + refuseTerminalLife(ns, *rt); return rt->life; } +void CasRefLedger::refuseTerminalLife(const RootNamespace & ns, RefTableRuntime & rt) +{ + /// Bytes written under a removed life are swept by the dead-life janitor, so the caller must not be + /// handed one to write into. A never-born stream is also `Removed` but has no `remove_txn_id`. + std::lock_guard lock(rt.state_mutex); + if (rt.state.getLifecycle() != RefLifecycle::Live && rt.state.getRemoveTxnId().has_value()) + throwCasWriteRetryLater(fmt::format( + "CAS namespace '{}': its resident life {} is already removed; retry after the removal completes", + ns.string(), renderIncarnation(rt.life.incarnation))); +} + std::optional CasRefLedger::namespaceFilesLifeIfReadable(const RootNamespace & ns) { const auto rt = acquireReadableRefTableRuntime(ns); @@ -5016,6 +5056,7 @@ bool CasRefLedger::namespaceStillLogicallyPresent(const RootNamespace & ns) /// the one answer that must never be manufactured by a race, so it alone is re-confirmed by a /// second read of this namespace's row before being trusted. const uint64_t admitted_generation = fence_generation_fn(); + const uint64_t retirements_seen = refNameRetirements(ns); CasOperation op = mount_requests.resume(admitted_generation); const CasRefCatalog::Snapshot first_catalog = CasRefCatalog::read(op, layout); refuseUnlessAdmitted(op, "namespace presence probe"); @@ -5060,7 +5101,7 @@ bool CasRefLedger::namespaceStillLogicallyPresent(const RootNamespace & ns) /// therefore not proof that the CURRENT logical namespace `ns` is absent; a fresh catalog read is /// required before answering `false` for the name. const NamespaceLifeId life = NamespaceLifeId::fromCatalogEntry(entry->ns, entry->incarnation); - const auto rt = acquireRefTableRuntime(life, admitted_generation); + const auto rt = acquireRefTableRuntime(life, admitted_generation, retirements_seen); ensureRefTableRecovered(ns, *rt); bool this_incarnation_terminal = false; { @@ -5099,6 +5140,7 @@ DropNamespaceStats CasRefLedger::dropNamespaceImpl( /// class shares the bigger complete-table byte budget (encodeRefLogTxn's own `checkBudget`, keyed /// off the presence of a `RemoveNamespace` op) and is exempt from the ordinary per-op admission /// check (it only ever shrinks state; see `flushRefBatch`'s `state_growing` filter). + const uint64_t retirements_seen = refNameRetirements(ns); CasOperation initial_op = mount_requests.admit(); const CasRefCatalog::Snapshot initial_catalog = CasRefCatalog::read(initial_op, layout); const auto initial_it = std::find_if(initial_catalog.catalog.entries.begin(), initial_catalog.catalog.entries.end(), @@ -5143,7 +5185,7 @@ DropNamespaceStats CasRefLedger::dropNamespaceImpl( const NamespaceLifeId observed_life = NamespaceLifeId::fromCatalogEntry(initial_it->ns, initial_it->incarnation); const uint64_t runtime_generation = fence_generation_fn(); - const auto rt = acquireRefTableRuntime(observed_life, runtime_generation); + const auto rt = acquireRefTableRuntime(observed_life, runtime_generation, retirements_seen); ensureRefTableRecovered(ns, *rt); { /// A real terminal transaction is idempotent. The representation also calls an empty, never-born diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h index 73ce041c73da3..1b3fca06f80c1 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h @@ -189,7 +189,8 @@ class CasRefLedger /// resolution, and the ONLY one that CREATES: recovery's step 0 (`resolveNamespaceLife`) mints a /// life when the catalog names none, so the first namespace file a table ever writes births the /// namespace exactly as its first ref op would. A read or a removal must not use this -- see the - /// sibling below for why that is a correctness matter and not a preference. + /// sibling below for why that is a correctness matter and not a preference. Throws retry-later for + /// a life whose terminal removal is already applied, since the janitor sweeps whatever is written there. NamespaceLifeId namespaceLife(const RootNamespace & ns); /// The life a READER (or a REMOVER) of this namespace's files must use, or `nullopt` when there are @@ -474,7 +475,7 @@ class CasRefLedger { std::lock_guard g(ref_queue_mutex); const auto it = ref_name_slots.find(ns.string()); - return it == ref_name_slots.end() ? 0 : it->second.current->pending.size(); + return it == ref_name_slots.end() || !it->second.current ? 0 : it->second.current->pending.size(); } /// Reports whether `ns` currently has an active append-lane leader (the baton). Under the queue mutex. @@ -482,7 +483,7 @@ class CasRefLedger { std::lock_guard g(ref_queue_mutex); const auto it = ref_name_slots.find(ns.string()); - return it != ref_name_slots.end() && it->second.current->leader_active; + return it != ref_name_slots.end() && it->second.current && it->second.current->leader_active; } /// Returns the number of items carved by the current tenure and not yet released by its exit guard. @@ -491,7 +492,7 @@ class CasRefLedger { std::lock_guard g(ref_queue_mutex); const auto it = ref_name_slots.find(ns.string()); - return it == ref_name_slots.end() ? 0 : it->second.current->carved.size(); + return it == ref_name_slots.end() || !it->second.current ? 0 : it->second.current->carved.size(); } /// Returns whether the `carved` entry for `ref_name` (if any) is already completed. Lets a test @@ -501,7 +502,7 @@ class CasRefLedger { std::lock_guard g(ref_queue_mutex); const auto it = ref_name_slots.find(ns.string()); - if (it == ref_name_slots.end()) + if (it == ref_name_slots.end() || !it->second.current) return false; for (const auto & item : it->second.current->carved) if (item->scope.kind == MutationScope::Kind::Ref && item->scope.ref_name == ref_name) @@ -936,6 +937,10 @@ class CasRefLedger struct RefNameSlot { std::shared_ptr current; + /// Bumped by `invalidateRemovedCatalogLife`. A slot with a non-zero count is never erased (only + /// remount clears it, and remount also moves the fence generation), so a cold admission compares + /// the count it captured before its catalog read with the one at insertion. + uint64_t retirements = 0; }; static constexpr size_t kMaxRefBatch = 1000; /// Recovery retries at most this many times when an object selected by LIST vanishes before GET. @@ -1037,9 +1042,17 @@ class CasRefLedger /// Publishes or returns one runtime for an exact catalog-observed life and admitted mount-fence /// generation. A conflicting attached identity is a stale observation and fails closed rather than - /// retargeting either runtime. + /// retargeting either runtime. `retirements_seen` is `refNameRetirements` captured before the + /// catalog read that produced `life`; if a retirement of this name landed since, the observation is + /// stale and the call throws retry-later instead of republishing a possibly retired life. std::shared_ptr acquireRefTableRuntime( - const NamespaceLifeId & life, uint64_t admitted_generation); + const NamespaceLifeId & life, uint64_t admitted_generation, uint64_t retirements_seen); + + /// The name's retirement count, under `ref_queue_mutex`. Zero for a name with no slot. + uint64_t refNameRetirements(const RootNamespace & ns) const; + + /// Throws retry-later when `rt` (already recovered) has applied its namespace's terminal removal. + void refuseTerminalLife(const RootNamespace & ns, RefTableRuntime & rt); /// Lookup-first non-minting read acquisition. A cold name consults the catalog and materializes only /// an exact `Live` life; absence/`Creating`/`Removing` returns no runtime. diff --git a/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp b/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp index 89d55e8dbb653..29972435dc60a 100644 --- a/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp +++ b/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp @@ -7,6 +7,7 @@ #include #include "cas_test_helpers.h" #include +#include /// Namespace files are keyed by an opaque LIFE, not by its name: `cas/ns/state//_files/` /// (Stage B Task 4b, directive design change 2). This file pins the three properties that re-key exists @@ -30,6 +31,11 @@ /// bytes present while that evidence is adopted; their later reclamation belongs to the perpetual /// janitor and is not a precondition for same-name reuse. +namespace DB::ErrorCodes +{ +extern const int NETWORK_ERROR; +} + using namespace DB::Cas; using namespace DB::Cas::tests; @@ -53,6 +59,28 @@ void publishWithProductionBirth(const PoolPtr & store, const RootNamespace & ns, build->promote(ns, ref, build->buildId(), id); } +/// Runs `after_catalog_read` once, right after the `fire_on_read`-th read of `catalog_key` returns: the +/// window between a cold reader's last catalog GET and its runtime insertion, where no hook exists. +class CatalogReadRaceBackend : public CountingBackend +{ +public: + String catalog_key; + size_t fire_on_read = 0; + std::function after_catalog_read; + + std::optional read(const String & key, DB::Cas::TransportAccess & access) override + { + auto result = CountingBackend::read(key, access); + if (after_catalog_read && key == catalog_key && --fire_on_read == 0) + { + auto once = std::move(after_catalog_read); + after_catalog_read = nullptr; + once(); + } + return result; + } +}; + } /// THE COUPLED HEADLINE. A real removal reaches a catalog-absent cut even when LIST permanently omits @@ -123,6 +151,54 @@ TEST(CASNsFileIncarnation, ColdReaderUsesCatalogCutWhileOldFileSurvivesRemoval) EXPECT_EQ(key.find("/_cleanup/"), String::npos) << key; } +/// A cold reader sees the row `Live` in both catalog reads; removal and the GC drain retire that life +/// before the reader inserts its runtime. The reader must not publish a runtime for the retired life, +/// and the next writer must get a fresh life whose acknowledged file survives GC. +TEST(CASNsFileIncarnation, ColdReaderDoesNotRepublishALifeRetiredAfterItsCatalogRead) +{ + auto backend = std::make_shared(); + const RootNamespace ns{kNsString}; + std::optional retired_life; + { + PoolPtr writer = openPoolForTest(backend, /*gc_fold_max_defer_rounds=*/0); + publishWithProductionBirth(writer, ns, "predecessor"); + retired_life = writer->namespaceLife(ns); + } + + /// A fresh pool holds no runtime for `ns`, so the reader below takes the cold path. + PoolPtr store = openPoolForTest(backend, /*gc_fold_max_defer_rounds=*/0); + const Layout & layout = store->layout(); + ASSERT_EQ(store->refTableRuntimeIdentityForTest(ns), 0u); + + CasRequests catalog_requests = DB::Cas::tests::openRequestsForTest(backend); + CasOperation catalog_op = catalog_requests.admit(); + Gc gc(store, kGcId); + bool retired = false; + backend->catalog_key = layout.refCatalogKey(); + backend->fire_on_read = 2; + backend->after_catalog_read = [&] + { + store->dropNamespace(ns); + (void)runRegularRoundReclaiming(gc); + (void)runRegularRoundReclaiming(gc); + retired = !CasRefCatalog::lifeIfCataloged(catalog_op, layout, ns).has_value(); + }; + + expectThrowsCode(DB::ErrorCodes::NETWORK_ERROR, [&] { (void)store->namespaceFilesLifeIfReadable(ns); }); + ASSERT_TRUE(retired) << "the race must fire after the reader's second catalog read and drain the row"; + EXPECT_NE(store->refTableLifeForTest(ns), retired_life) + << "a runtime for the retired life was published"; + + const NamespaceLifeId writer_life = store->namespaceLife(ns); + EXPECT_NE(writer_life, *retired_life); + const String bytes = "fresh-life\n"; + store->putNamespaceFile(writer_life, kFile, bytes); + (void)runRegularRoundReclaiming(gc); + (void)runRegularRoundReclaiming(gc); + EXPECT_EQ(store->getNamespaceFile(writer_life, kFile), std::optional{bytes}) + << "an acknowledged namespace file was swept"; +} + /// The non-minting reader assignment site accepts exactly a catalog `Live` row. `Creating`, /// `Removing`, and absence neither install a runtime life nor mutate durable catalog/stream state. TEST(CASNsFileIncarnation, FreshReaderAssignsOnlyLiveCatalogLifeWithoutMutation) From e8177d913989cf531927e9a8dff1594583a9232a Mon Sep 17 00:00:00 2001 From: Mikhail Filimonov Date: Tue, 13 Oct 2026 02:39:22 +0200 Subject: [PATCH 2/3] Record a namespace retirement even when a successor is resident `invalidateRemovedCatalogLife` returned without bumping the name's retirement count when a successor life was resident. A delayed cold admission of the predecessor could then insert it once the successor was evicted. The count is now bumped for every retirement, while invalidation and detachment stay limited to the exact predecessor runtime. New tests: - a delayed predecessor retirement with a successor resident advances the count, leaves the successor attached, and the count survives eviction; - `namespaceLife` refuses a recovered terminal runtime whose removal-admission flag is clear, on the cold call and on the warm one; - the cold `namespaceLife` window between its catalog read and its insertion. Draft: not built or tested yet. Signed-off-by: Mikhail Filimonov Co-Authored-By: Claude Fable 5.1 --- .../ContentAddressed/Pool/CasPool.h | 4 + .../ContentAddressed/Pool/CasRefLedger.cpp | 11 +- .../ContentAddressed/Pool/CasRefLedger.h | 2 + .../tests/gtest_cas_ns_file_incarnation.cpp | 113 ++++++++++++++++++ 4 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.h b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.h index c566a9164f1a7..506bb70ac7d5b 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.h +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.h @@ -1131,6 +1131,10 @@ class Pool : public std::enable_shared_from_this { return ref_ledger.refTableRuntimeAdmittedFenceGenerationForTest(ns); } + uint64_t refNameRetirementsForTest(const RootNamespace & ns) const + { + return ref_ledger.refNameRetirementsForTest(ns); + } std::optional refTableLifeForTest(const RootNamespace & ns) { return ref_ledger.refTableLifeForTest(ns); diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp index 31031c3422cef..a17bc6c767375 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.cpp @@ -712,15 +712,13 @@ void CasRefLedger::invalidateRemovedCatalogLife(const NamespaceLifeId & life) std::shared_ptr rt; { std::lock_guard lock(ref_queue_mutex); - /// Recorded even with no resident runtime: a cold admission may be between its catalog read - /// and its insertion. + /// Recorded whatever is resident, a successor included: a cold admission of `life` may be + /// between its catalog read and its insertion, and the successor may be evicted before it. RefNameSlot & slot = ref_name_slots[life.ns.string()]; + ++slot.retirements; rt = slot.current; if (!rt) - { - ++slot.retirements; return; - } } /// Invalidation must neither materialize a cache entry nor perform catalog I/O on GC's thread. @@ -740,10 +738,7 @@ void CasRefLedger::invalidateRemovedCatalogLife(const NamespaceLifeId & life) std::lock_guard queue_lock(ref_queue_mutex); const auto it = ref_name_slots.find(life.ns.string()); if (it != ref_name_slots.end() && it->second.current == rt) - { it->second.current.reset(); - ++it->second.retirements; - } } rt->cv.notify_all(); rt->recovery_cv.notify_all(); diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h index 1b3fca06f80c1..34cb10ef3bc52 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h @@ -553,6 +553,8 @@ class CasRefLedger const auto it = ref_name_slots.find(ns.string()); return it == ref_name_slots.end() || !it->second.current ? 0 : it->second.current->admitted_fence_generation; } + /// How many lives of `ns` catalog retirement has recorded since the last remount. + uint64_t refNameRetirementsForTest(const RootNamespace & ns) const { return refNameRetirements(ns); } /// The physical life currently pinned in the cached runtime, without resolving or recovering it. std::optional refTableLifeForTest(const RootNamespace & ns) { diff --git a/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp b/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp index 29972435dc60a..b35582ae0dee8 100644 --- a/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp +++ b/src/Disks/tests/gtest_cas_ns_file_incarnation.cpp @@ -199,6 +199,119 @@ TEST(CASNsFileIncarnation, ColdReaderDoesNotRepublishALifeRetiredAfterItsCatalog << "an acknowledged namespace file was swept"; } +/// The cold MUTABLE admission (`namespaceLife`) has the same window between its one catalog read and +/// its runtime insertion. +TEST(CASNsFileIncarnation, ColdWriterDoesNotRepublishALifeRetiredAfterItsCatalogRead) +{ + auto backend = std::make_shared(); + const RootNamespace ns{kNsString}; + std::optional retired_life; + { + PoolPtr writer = openPoolForTest(backend, /*gc_fold_max_defer_rounds=*/0); + publishWithProductionBirth(writer, ns, "predecessor"); + retired_life = writer->namespaceLife(ns); + } + + PoolPtr store = openPoolForTest(backend, /*gc_fold_max_defer_rounds=*/0); + const Layout & layout = store->layout(); + ASSERT_EQ(store->refTableRuntimeIdentityForTest(ns), 0u); + + CasRequests catalog_requests = DB::Cas::tests::openRequestsForTest(backend); + CasOperation catalog_op = catalog_requests.admit(); + Gc gc(store, kGcId); + bool retired = false; + backend->catalog_key = layout.refCatalogKey(); + backend->fire_on_read = 1; + backend->after_catalog_read = [&] + { + store->dropNamespace(ns); + (void)runRegularRoundReclaiming(gc); + (void)runRegularRoundReclaiming(gc); + retired = !CasRefCatalog::lifeIfCataloged(catalog_op, layout, ns).has_value(); + }; + + expectThrowsCode(DB::ErrorCodes::NETWORK_ERROR, [&] { (void)store->namespaceLife(ns); }); + ASSERT_TRUE(retired) << "the race must fire after the writer's catalog read and drain the row"; + EXPECT_NE(store->refTableLifeForTest(ns), retired_life) << "a runtime for the retired life was published"; +} + +/// A recovered runtime whose stream holds the terminal removal, with no local removal in progress (its +/// removal-admission flag is clear): `namespaceLife` must not hand that life to a writer, on the cold +/// call that recovers it or on the warm call that finds it resident. +TEST(CASNsFileIncarnation, NamespaceLifeRefusesARecoveredTerminalLife) +{ + auto backend = std::make_shared(); + PoolPtr store = openPoolForTest(backend, /*gc_fold_max_defer_rounds=*/0); + const Layout & layout = store->layout(); + const RootNamespace ns{kNsString}; + { + RefOp remove_op; + remove_op.kind = RefOpKind::RemoveNamespace; + appendRefLogSeed(*backend, layout, ns, {remove_op}); + } + writeRecoverableCkptForRawFixture(*backend, layout, ns, RefCkpt{ + .life_epoch = 1, + .committed_through = RefTxnId{1, 1}, + .checkpoint_snapshot_id = std::nullopt, + .last_epoch_seal = std::nullopt, + }); + CasRequests catalog_requests = DB::Cas::tests::openRequestsForTest(backend); + CasOperation catalog_op = catalog_requests.admit(); + const CasRefCatalog::Snapshot catalog = CasRefCatalog::read(catalog_op, layout); + const auto row = std::find_if(catalog.catalog.entries.begin(), catalog.catalog.entries.end(), + [&](const CatalogEntry & entry) { return entry.ns == ns; }); + ASSERT_NE(row, catalog.catalog.entries.end()); + ASSERT_EQ(row->state, NsState::Live) << "a Removing row would be refused for a different reason"; + const NamespaceLifeId terminal_life = NamespaceLifeId::fromCatalogEntry(row->ns, row->incarnation); + + expectThrowsCode(DB::ErrorCodes::NETWORK_ERROR, [&] { (void)store->namespaceLife(ns); }); + ASSERT_EQ(store->refTableLifeForTest(ns), std::optional{terminal_life}) + << "the cold call must have recovered the terminal runtime and left it resident"; + expectThrowsCode(DB::ErrorCodes::NETWORK_ERROR, [&] { (void)store->namespaceLife(ns); }); +} + +/// A delayed retirement callback for a predecessor arrives while a successor is resident. It must +/// still advance the name's record, and the record must survive the successor's eviction: otherwise a +/// cold admission that read the predecessor's row could insert it once the slot empties. +TEST(CASNsFileIncarnation, RetirementWithASuccessorResidentIsRecordedAndSurvivesEviction) +{ + auto backend = std::make_shared(); + PoolConfig config{.pool_prefix = "p", .server_root_id = "test"}; + config.gc_fold_max_defer_rounds = 0; + config.ref_table_cache_bytes = 1; + PoolPtr store = Pool::open(backend, std::move(config)); + const RootNamespace ns{kNsString}; + const RootNamespace other{"00/bb@cas@"}; + + publishWithProductionBirth(store, ns, "predecessor"); + const NamespaceLifeId predecessor = store->namespaceLife(ns); + store->dropNamespace(ns); + Gc gc(store, kGcId); + (void)runRegularRoundReclaiming(gc); + (void)runRegularRoundReclaiming(gc); + + publishWithProductionBirth(store, ns, "successor"); + const NamespaceLifeId successor = store->namespaceLife(ns); + ASSERT_NE(successor, predecessor); + const uint64_t successor_runtime = store->refTableRuntimeIdentityForTest(ns); + ASSERT_NE(successor_runtime, 0u); + const uint64_t retirements_before = store->refNameRetirementsForTest(ns); + + store->invalidateRemovedCatalogLife(predecessor); + + EXPECT_EQ(store->refNameRetirementsForTest(ns), retirements_before + 1) + << "a predecessor's retirement must be recorded even with a successor resident"; + EXPECT_EQ(store->refTableRuntimeIdentityForTest(ns), successor_runtime) + << "and must not detach the successor"; + EXPECT_EQ(store->namespaceLife(ns), successor); + + /// A 1-byte budget evicts the idle successor when another table is touched. + publishWithProductionBirth(store, other, "x"); + ASSERT_EQ(store->refTableRuntimeIdentityForTest(ns), 0u) << "the successor must have been evicted"; + EXPECT_EQ(store->refNameRetirementsForTest(ns), retirements_before + 1) + << "eviction must keep a slot that records a retirement"; +} + /// The non-minting reader assignment site accepts exactly a catalog `Live` row. `Creating`, /// `Removing`, and absence neither install a runtime life nor mutate durable catalog/stream state. TEST(CASNsFileIncarnation, FreshReaderAssignsOnlyLiveCatalogLifeWithoutMutation) From 19b5df85a6659acbaee3f9ab7f2a8509e9fabf9d Mon Sep 17 00:00:00 2001 From: Mikhail Filimonov Date: Tue, 13 Oct 2026 02:46:54 +0200 Subject: [PATCH 3/3] Use Allman braces in the new ref-name retirement accessor Draft: not built or tested yet. Signed-off-by: Mikhail Filimonov Co-Authored-By: Claude Fable 5.1 --- .../MetadataStorages/ContentAddressed/Pool/CasRefLedger.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h index 34cb10ef3bc52..81906b28baa98 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasRefLedger.h @@ -554,7 +554,10 @@ class CasRefLedger return it == ref_name_slots.end() || !it->second.current ? 0 : it->second.current->admitted_fence_generation; } /// How many lives of `ns` catalog retirement has recorded since the last remount. - uint64_t refNameRetirementsForTest(const RootNamespace & ns) const { return refNameRetirements(ns); } + uint64_t refNameRetirementsForTest(const RootNamespace & ns) const + { + return refNameRetirements(ns); + } /// The physical life currently pinned in the cached runtime, without resolving or recovering it. std::optional refTableLifeForTest(const RootNamespace & ns) {