Skip to content

CAS: refuse new nested server_root_id, delete S3 staging on abort, actionable member refusals - #2566

Merged
filimonov merged 13 commits into
antalya-26.6-20261008from
fix/antalya-26.6/cas-member-lifecycle-pr-a
Oct 19, 2026
Merged

filimonov merged 13 commits into
antalya-26.6-20261008from
fix/antalya-26.6/cas-member-lifecycle-pr-a

Conversation

@filimonov

@filimonov filimonov commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

A cas_server_root_id may no longer contain /, for new and existing members alike. Decommission, staging and the GC floor lookup select a member's objects by path prefix, so decommissioning a also removes the namespaces, staging and mountpoint objects of a member a/b. A server whose id contains / refuses to open the disk and must be recreated under a new id.

Changes

  • validateServerRootId refuses / with BAD_ARGUMENTS. It runs at disk config read, in Pool::open and in SYSTEM CAS DROP POOL MEMBER / clickhouse-disks cas-drop-member, so a nested id can neither mount nor be decommissioned. The message tells the operator to recreate the server under a new <cas_server_root_id> without /.
  • An aborted transaction deletes its S3 staging object, best-effort, as a commit does. Before, aborted staging stayed until the next mount of the same server_root_id.
  • The bucket requirements document the AbortIncompleteMultipartUpload lifecycle rule: an upload killed mid-way is invisible to object LIST and is never swept.
  • The owner and decommission refusals name the recovery that works:
    • retired owner: configure a new cas_server_root_id; do not clear the tombstone;
    • foreign owner: restore the uuid file or start under a new id; decommission the old id once its slot is terminated or fenced;
    • SYSTEM CAS DROP POOL MEMBER on a hard-killed member: wait until the disk's GC leader fences it, about one lease TTL plus one renew period after it first saw the lease; SYSTEM CAS GC RUN on the leader helps, but two back-to-back runs do not fence.

Risks / notes

  • Backward incompatible: a server whose cas_server_root_id contains / (for example from a {shard}/{replica} template) refuses to open the disk with BAD_ARGUMENTS. Recreate it under a new id without /, such as {shard}-{replica}. Data stored under the old id is not visible under the new id; the old id's objects stay in the pool and keep their blobs referenced, and SYSTEM CAS DROP POOL MEMBER refuses the old id, so it cannot reclaim them.
  • Decommissioning a still selects the objects of a legacy a/b left in the pool by an earlier build. Such a member can no longer mount.

Cleaning up a legacy id with / (example id a/b)

Before upgrading, on 26.6.4 (preferred):

  1. Stop the server whose id is a/b. If it must stay in the cluster, first start a replacement under an id without / and let it replicate.
  2. From another member, run SYSTEM CAS DROP POOL MEMBER 'a/b' FROM DISK '<disk>'. 26.6.4 accepts / in the id. A live lease is refused; a hard-killed member must be fenced first, as described above.
  3. Rerun until it returns slot_removed = true and empty warnings. The warning namespace(s) are still owned by this member means GC rounds must run first.
  4. Upgrade.

Dropping a/b selects keys under a/b/ only, never a/bc or a, but it does select a deeper a/b/c. Never drop a parent a while a/b must survive: both versions select a/b's objects for a.

After upgrading, if the steps above were missed:

  1. Delete nothing by hand.
    • cas/ref_catalog is one object for the whole pool, updated by compare-and-swap by writers and GC. Its a/b/... rows cannot be removed by hand.
    • cas/ns/stream/<life>/ and cas/ns/state/<life>/ are keyed by life id, not by server_root_id. Only the catalog maps them to a/b.
    • gc/server-roots/a/b/ (owner, epoch, mount) is the slot that decommission claims. Without owner and mount it refuses with unknown pool member. The manifest sweep reads its floor from mount.
    • staging/a/b/, roots/a/b/ and eligible cas/manifests/a/b/ debris are drained by cas-drop-member itself.
    • blobs/: GC reclaims a blob once no namespace references it.
  2. Keep the a/b server stopped. Run the 26.6.4 offline tool against the pool from a config whose CAS disk points at the same pool with <readonly>true</readonly>:
    clickhouse-disks -C config.xml --disk cas cas-drop-member 'a/b'
  3. Rerun it after GC rounds until it prints slot_removed=true and no warning= lines. The upgraded servers' GC deletes the removed catalog rows and then the unreferenced blobs.

Not verified: that a 26.6.4 binary reads every object this build writes. No test runs a 26.6.4 cas-drop-member against a pool written by this build.

Testing

  • New gtests (fail on the base): an existing member with a nested id refuses to open, without claiming the mount or minting an epoch; decommission of a nested id is refused and leaves its owner object untouched; abort deletes S3 staging; the refusal texts. Guards: system.cas_mounts still names a legacy nested slot whole; abort after storage shutdown does not throw; the decommission staging drain removes what the writer staged under the local and native key prefixes; the next mount sweeps a staging object whose abort delete was lost.
  • CAS* unit tests green in release and ASan; test_cas_drop_pool_member passes, including a new case: SYSTEM CAS DROP POOL MEMBER 'node2/nested' is refused with BAD_ARGUMENTS.

Changelog category (leave one):

  • Backward Incompatible Change

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

CAS: cas_server_root_id may not contain /. A server whose id contains / (for example from a {shard}/{replica} template) refuses to start the disk and must be recreated under a new id without /; data stored under the old id is not visible under the new one. An aborted transaction deletes its S3 staging object. SYSTEM CAS DROP POOL MEMBER and the owner refusals name the state or command that resolves them.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Unit tests
  • Performance tests
  • Aarch64 tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

filimonov and others added 8 commits October 14, 2026 01:23
`SYSTEM CAS DROP POOL MEMBER 'a'` and the staging sweep select objects by
key prefix, so a member `a/b` was drained together with `a`. The first
claim of an id (owner absent) now refuses an id containing `/`;
`validateServerRootId` keeps accepting one so members that already own a
nested id keep mounting. The adopt-from-lease decommission fallback goes
through the same claim and is refused for a nested id before any object
is touched.

Tests: `CASServerRootId.NewIdWithSlashIsRefused`,
`CASDecommission.LeaseAdoptFallbackRefusesANestedIdWithoutOwner` (red on
the base: the owner was created), guard
`CASServerRootId.ExistingIdWithSlashStillMounts`.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
An aborted transaction left its S3 staging object until the next mount
of the same `server_root_id`; the comment calling it a re-readable
publication source was wrong, nothing reads `pending_blobs` after the
cleanup. Commit and abort now delete it alike, best-effort.

Incomplete multipart uploads are invisible to object LIST, so the docs
state the bucket lifecycle rule `AbortIncompleteMultipartUpload`.

Tests: `CASS3Staging.AbortDeletesS3Staging` (red on the base: one object
remains), guard `CASS3Staging.AbortAfterStorageShutdownDoesNotThrow`.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The drain lists `<pool_prefix>/staging/<srid>/` as a logical key and the
backend applies the local root itself, while the writer mints keys
through `physicalKey`. Two guard tests stage a blob through a real
transaction, model a lost abort delete, and decommission the member
over an EmulatedSingleProcess (root-prefixed) and a Native (verbatim)
backend. A third pins that the next mount's sweep reclaims the same
object.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The foreign-owner refusal advised a hand delete that the next start
refuses over existing data; it now names `SYSTEM CAS DROP POOL MEMBER`
for a dead owner and the hand delete last. The retired-owner refusal
advised clearing the tombstone, which would let writer epochs repeat; it
now says to configure a new `cas_server_root_id`. The `NoWait` refusal
said "wait for its lease to lapse", but the claim trusts no wall clock:
it now names the terminal states (`terminated`, `fenced`) and how GC
reaches `fenced` for a hard-killed member.

Tests: `CASServerRootClaim.RefusalsNameTheSupportedRecovery`,
`CASDecommission.NoWaitRefusalNamesTheTerminalStates` (red on the base:
substrings absent). The substrings the integration test matches on
(`alive or contended`, `explicitly decommissioned`) are kept.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fencing happens only in a round run by the disk's GC leader, after it has
seen the mount token unchanged for about one lease TTL plus one renew
period. The refusal text and the docs said any live member's next round.
They now name the leader and `is_leader` in `system.cas_mounts`. The test
also pins `alive or contended` and `state = 'terminated'`.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The three CASServerRootSafety owner-claim expectations now assert code and
message, so they pass only through the emptiness check that runs before the
nesting rule. The post-shutdown abort test makes the delete throw so it
reaches the catch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…ifecycle PR

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [596fced]

The GC leader fences an expired mount lease only at the start of a GC
round, and under load a round took 183-245 s. The decommission refusal
and the docs now say so.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CasOperation & op, const Layout & l, const String & srid, const RefCatalog & catalog_observation)
{
const String owned_prefix = srid + "/";
for (const CatalogEntry & entry : catalog_observation.entries)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems there is a corner case here.

As I can see above here we decided support old server_root with '`/'.

We have entry in ref_catalog only after inserting data (precommit stage). So we could have server_rrot_id a/b without entry in ref_catalog and serverRootSubtreeEmpty returns true instead false.

This repoduces the same problem.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. / is now refused for every cas_server_root_id, including ids that already own their owner object, so a nested id can no longer mount or be decommissioned and the subtree check does not have to reason about one. An operator recreates such a member under a new id without /. Fixed in 52cb4b5.

filimonov and others added 4 commits October 17, 2026 19:36
Decommission and the staging sweep select a member's objects by key
prefix, so a member `a/b` would be drained by a decommission of `a`.
An owner-claim check covered only new ids; a nested id with no ref
catalog entry yet still let `a` claim over it. `validateServerRootId`
now refuses '/' for every id, so the disk config, `Pool::open` and
`SYSTEM CAS DROP POOL MEMBER` all refuse it, and the claim-time check
is gone.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@filimonov

Copy link
Copy Markdown
Member Author

CI triage for 596fced. No red is caused by this PR.

This PR and #2517 both change CasPool.cpp and CasServerRoot.*, so the merge with the current base was built locally: release and ASan unit_tests_dbms with CAS*: release 2705 passed, ASan 2709 passed.

@filimonov
filimonov merged commit af662dc into antalya-26.6-20261008 Oct 19, 2026
354 of 370 checks passed
filimonov added a commit that referenced this pull request Oct 19, 2026
Brings in the base merge of PR B: #2566 (member lifecycle PR A, final
review) and #2517 (renewal until a definitive answer).

- The foreign-owner refusal takes the #2566 text, which names
  `SYSTEM CAS DROP POOL MEMBER` for a dead owner.
- Docs keep this branch's rejoin rules and add the #2566 rows for the
  terminal-slot claim and ids with `/`.
- The unverified-tombstone test expects this branch's
  `decommission is not finished` refusal.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@k-morozov k-morozov added the verified Approved for release label Oct 20, 2026
@alsugiliazova

Copy link
Copy Markdown
Member

PR #2566 Audit Review

Verdict

One Medium and two Low distributed-systems findings. No High.

The Medium is the shape of the new / rule rather than the rule itself: refusing / blocks the
decommission that works on a nested id and leaves open the decommission that destroys one. The
abort-delete change — the part that touches the commit protocol — is correct, and the reasoning is
recorded below so the next reader does not have to redo it.

What the change does

1. cas_server_root_id may not contain /. validateServerRootId replaces its per-segment walk
with a whole-string check: non-empty, ≤ 255 bytes, no /, not ./../_files/_manifests. It
runs at config read (ContentAddressedSettings.cpp:281, after macro expansion at line 215, so a
{shard}/{replica} template is caught), in mountWritable (CasPool.cpp:584) and on the victim
id of openForDecommission (CasPool.cpp:915). A member whose id contains / can therefore neither
mount nor be decommissioned.

2. An aborted transaction deletes its S3 staging object. cleanupPendingTempFiles loses the
if (committed) guard: both backends are now reclaimed on commit and on abort alike. Before, an
aborted S3 staging object stayed until the next mount of the same server_root_id swept it.

3. Refusal texts name a recovery. The NoWait decommission refusal now enumerates the mount
states it accepts and who produces them; the retired-owner and foreign-owner refusals name
"configure a new cas_server_root_id" and SYSTEM CAS DROP POOL MEMBER instead of leading with a
hand edit of the owner object.

4. Documentation. The AbortIncompleteMultipartUpload lifecycle rule is added to the bucket
requirements, with the reason: an upload killed mid-way is invisible to object LIST, so neither the
transaction's delete nor the mount-start sweep can ever see it. Three new troubleshooting rows and a
migration section on recreating a member whose id contains /.

Model

Actors Every server mounting the pool under its own server_root_id, the disk's GC leader, an administrative decommission writer impersonating a dead member, and the object store
Channels Conditional (compare-and-swap) object-store writes over a shared pool prefix; no node-to-node RPC. The mount lease object is the only liveness channel between members
Authoritative state gc/server-roots/<srid>/{owner,epoch,mount} for identity, epoch and liveness; the pool-wide cas/ref_catalog for namespace ownership; blobs are content-addressed and shared
Failure model assumed by the code Crash-recover with no trusted shared clock. Liveness is never inferred from expires_at_ms: a slot is reclaimable only on a clock-free certificate (gc_fenced, the clean-farewell marker, or an observation the claimer made on its own CLOCK_BOOTTIME)
Safe-failure criterion No member's objects are selected by another member's administrative operation; a refused mount writes nothing; an aborted transaction publishes no ref and leaks no reachable state

Why the abort delete is safe

The old comment asserted that an S3 staging object "must outlive an aborted transaction so a later
attempt (or the sweeper) can still account for it." It does not have to, and the PR is right to drop
that. Three things had to hold, and all three do.

A staging key exists only in process memory. It is minted as
stagingKeyPrefix() + "/" + getRandomASCIIString(32) + ".tmp"
(ContentAddressedTransaction.cpp:883) and lives in PartStaging::pending_blobs. Nothing durable —
no precommit record, no manifest entry, no catalog row — ever names it, so no recovery path can read
a staging object after its transaction is gone. Each transaction mints its own random key, so there
is no sharing between concurrent transactions either; adoptStagedBlob copies a PendingBlob record
only into another part of the same transaction, and the resulting double delete is harmless for
both backends.

No upload task can outlive the delete. fanOutBlobUploads joins every scheduled task before
returning, on the throw path too: waitForAllToFinish is declared after handles/runner so it
runs first on scope exit, and the success path ends in
waitForAllToFinishAndRethrowFirstError (ContentAddressedTransaction.cpp:1764-1789). The lambdas
that stream from staging_key therefore cannot still be running when the destructor deletes it.

A late-materializing single-attempt blob create does not need the source. Blob publication is sent
once; a lost response can leave the copy to materialize server-side afterwards. The source is read
when the store executes the copy, not when the client later deletes it, and in any case the
transaction that aborted has no ref pointing at that blob — an orphan blob is GC's to reclaim.

The guard tests are well chosen: CASS3Staging.AbortDeletesS3Staging pins the behaviour,
AbortAfterStorageShutdownDoesNotThrow pins that a failing delete cannot escape the noexcept
destructor, and MountStartSweepsALostAbortDelete pins that the backstop still works.

Findings

Configuration & version skew

Medium: the / refusal blocks the decommission that works on a nested id and leaves open the one
that destroys it.

  • Impact: on a pool created by an earlier build with a nested id, the member can no longer mount
    and can no longer be decommissioned, so its namespaces keep their blobs referenced permanently
    with no in-version reclaim path. Meanwhile
    SYSTEM CAS DROP POOL MEMBER 'a' FROM DISK '<disk>' is still accepted — a contains no / — and
    still selects a legacy a/b's namespaces, which is the data-loss shape the rule exists to prevent.
  • Anchor: validateServerRootId (Pool/CasServerRoot.h:84), called from
    ContentAddressedSettings.cpp:281, CasPool.cpp:584 (mountWritable) and CasPool.cpp:915
    (openForDecommission, on the victim id). The selection it is meant to protect is
    CasDecommission.cpp:204-215, const String victim_namespace_prefix = victim_srid + "/".
  • Trigger: upgrade a cluster whose cas_server_root_id expands from a {shard}/{replica}
    template — documented as legal in configuration.md until this PR, and asserted legal by the
    pre-PR EXPECT_NO_THROW(validateServerRootId("shard-01/replica-a")).
  • Why defect: dropping a/b is prefix-correct today. Every selector either appends a trailing
    / — namespaces at CasDecommission.cpp:204, staging at :295
    (pool_prefix + "/staging/" + victim_srid + "/"), the mount-start sweep at
    ContentAddressedMetadataStorage.cpp:896 — or names an exact key (mountKey, epochKey,
    ownerKey). listMounts and computeHeartbeatFloor both slice the srid out by prefix length
    rather than rfind('/') (CasServerRoot.cpp:1258 and :1102), so a nested slot is read and
    fenced correctly. Dropping the parent a is the one broken case, and it still passes validation.
    The documented remedy for the blocked case — run a 26.6.4 cas-drop-member offline against a pool
    this build has since written — needs a binary the operator has just replaced, and the PR body
    itself lists that cross-version read as unverified.
  • Fix direction: keep the pre-PR segment validation for openForDecommission's victim id
    (refuse an empty segment, ., .., _files, _manifests; allow /) so a legacy member can
    still be retired, and refuse instead the drop whose victim is a strict path prefix of another
    slot under gc/server-roots/.
  • Regression test direction: seed a nested member and decommission it end to end, asserting
    slot_removed = true; separately assert that dropping a is refused while a slot for a/b
    exists.

Recovery & rollback

Low: the abort delete's only backstop is a mount that, for a retired id, never happens again.

  • Impact: a staging object whose best-effort delete is lost is reclaimed only by the next mount
    of the same server_root_id or by that member's own decommission. An id retired through the
    migration this PR documents is never mounted again, and if it is nested it cannot be
    decommissioned either, so those bytes stay billed forever.
  • Anchor: the catch (...) in cleanupPendingTempFiles
    (ContentAddressedTransaction.cpp:184-189); the two reclaimers are
    ContentAddressedMetadataStorage.cpp:896 (mount start) and CasDecommission.cpp:295 (drain).
  • Trigger: abort a transaction while the object store refuses the delete, then recreate the
    server under a new cas_server_root_id.
  • Why defect: not a correctness break — nothing references a staging object and no ref points at
    it — but it is both unreclaimable and unreportable. GC lists only blobsPrefix(), so staging/ is
    never walked (CasLayout.h:398-403), and cas-fsck does not count it either. The new
    AbortIncompleteMultipartUpload rule does not cover this case: the upload completed, so the
    object is a normal listable object that no lifecycle rule for incomplete uploads will touch.
  • Fix direction: have cas-fsck report staging/ bytes per srid, so the debris is at least
    visible; a bucket-level expiry on staging/ would also bound it, since a staging object that
    outlives one transaction is garbage by construction.

Observability & diagnosability

Low: the new decommission refusal gives the wrong instruction on the foreign-owner branch and
understates which slots are claimable.

  • Impact: an operator follows "wait until state = 'fenced', then rerun" and loops forever,
    because a fenced foreign lease is still refused.
  • Anchor: the MountClaimPolicy::NoWait branch in Pool::mountWritable emits one text for every
    non-Claimed outcome. claimMount returns ForeignOwner at CasServerRoot.cpp:818-823, which is
    before the gc_fenced || clean_marker certificates at :876-879 — so neither state the message
    names can resolve that branch. The same text says a member is removable "only once its mount slot
    is terminal", while claimMount's absent-slot branch (if (!got), :802) mints a fresh claim, so
    a member with a surviving owner and no mount object is claimable too.
  • Trigger: owner uuid ≠ mount lease uuid (a hand-restored uuid file, or the partial hand cleanup
    openForDecommission explicitly tolerates at CasPool.cpp:950); or a rerun after a decommission
    that deleted mount and epoch but failed the owner tombstone.
  • Why defect: the holder clause does print the foreign uuid, so the facts are in the message —
    the actionable sentence is simply the wrong one for that outcome. For the common LiveDoubleStart
    case the text is accurate: terminated and fenced are exactly the two certificates
    claimMount accepts, and they match the state strings listMounts emits (CasServerRoot.cpp:1269-1276).
  • Fix direction: branch the advice on claim.kind — ForeignOwner deserves the uuid-mismatch
    recovery the owner refusal now carries, not the fence wait.

Coverage summary

Fault class Status Note
1. Network / transport N/A No new request, timeout or retry logic; the new delete reuses removeObjectIfExists under the existing engine
2. Partition / availability Reviewed The claim certificates are unchanged. A single-member pool whose only member is hard-killed still has nobody to fence it, so it cannot be decommissioned at all — now documented in the new troubleshooting row
3. Timing / ordering Reviewed The change strengthens the clock-free story: the refusal explicitly says expired is wall-clock arithmetic the claim never trusts, which matches claimMount
4. Partial / gray failure Reviewed The abort delete is best-effort in a noexcept destructor and pinned by AbortAfterStorageShutdownDoesNotThrow; its residual debris is the Low finding above
5. Idempotency & duplicates Reviewed cleanupPendingTempFiles is called from both commit and the destructor and clears pending_blobs, so the second call is a no-op; a duplicate PendingBlob record from adoptStagedBlob deletes the same key twice, harmlessly
6. Backpressure & resource limits Reviewed One delete per staged blob on a path that already issued one upload per blob
7. Recovery & rollback Reviewed One Low finding. A refused open writes nothing — pinned by ExistingIdWithSlashRefusesToOpen asserting neither mount nor epoch appears
8. Configuration & version skew Reviewed One Medium finding. The rule is correctly applied after macro expansion
9. Security / trust across nodes N/A No change to peer identity, authorization, or credentials
10. Observability & diagnosability Reviewed One Low finding; the rest of the message work is an improvement

Deferred: none.

Assumptions and limits: static review of the PR diff at head, with no build and no execution.
Base-branch CAS behaviour (the ref ledger, GC rounds, manifest sweeps, the blob publication
protocol) was treated as given.

Smaller notes

The PR description says "Decommission, staging and the GC floor lookup select a member's objects by
path prefix." The GC floor lookup does not: computeHeartbeatFloor slices the srid by prefix length
(CasServerRoot.cpp:1102) and handles a nested id correctly. The in-tree documentation is accurate —
mounts-and-leases.md says only "decommission and the staging sweep" — so this is a PR-description
overstatement, not a doc error.

CASServerRootId.ValidationAcceptsCleanPathsRejectsBad now has exactly one accepted input
(replica-a), since shard-01/replica-a moved from EXPECT_NO_THROW to EXPECT_THROW. The name is
the only index into this contract and no longer describes what the test asserts.

The integration test test_drop_pool_member_refuses_id_with_slash checks the refusal text but not
that the refusal is inert. The gtest CASDecommission.RefusesIdWithSlash covers that for the owner
object, mount and epoch; neither covers a nested member's namespaces surviving the refusal.

CI at the time of review

Not triaged here — this is the static audit. For the record, at head the rollup carried failures in
Unit tests (asan_ubsan), Stateless tests (amd_asan_ubsan, cas s3 storage, parallel, 1/2),
RegressionTestsRelease / CAS (selects), RegressionTestsRelease / Swarms,
CASS3CacheLightweightDelete (4), and three Grype scans. The unit-test failure matters most for this
PR, since it adds gtests to three CAS suites; run pr-ci-failure-triage before reading the merge as
green.

filimonov added a commit that referenced this pull request Oct 24, 2026
…ation

Since #2566 an abandoned transaction deletes its S3 staging object, so the first write's key was gone by the time the test counted the keys under the old epoch and the count was 0 on every run (`Unit tests (asan_ubsan)` of #2567 and #2574 after the base merge). The first transaction now lives until the final assertion; the property under test — a self-remount between the two reads refuses the write and stages nothing under the old epoch — is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants