Repository navigation
Conversation
|
Stateless tests (amd_asan_ubsan, cas s3 storage, parallel, 1/2) and Stateless tests (amd_binary, cas s3 storage, parallel): |
Two processes of one server_uuid could adopt the same writer epoch: when our counter write timed out and a twin wrote the same next value, the resolve read found identical bytes and reported our write as landed. Each allocation decision now writes a fresh random `allocation_nonce`, so only our own write matches our bytes. The field is a plain key that 26.6.4 readers skip. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every owner write sets `owner_life` to the read value + 1, so no two owner writes carry the same bytes and a delayed If-Match cannot land after a later write; `retirement_verified` records a finished decommission. Both are optional keys that 26.6.4 readers skip. Every claimer re-reads the owner after its mount write and requires its own uuid and life; otherwise it fences its own body and fails, so a successor does not wait the lease out. A remount that finds another life stops for good. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Decommission no longer deletes owner, epoch or mount: the counter keeps epochs from repeating across a member's lives, and the farewell body keeps a paused claimer from minting a lower epoch into an absent slot. Each run tombstones the owner right after its claim, bounded by its lease, confirms the claim, drains on its fenced mount plane, writes its farewell and marks the retirement verified. A lost claim rolls the tombstone back unless an earlier run verified it. After an unresolved tombstone the admin withholds its farewell, so the victim does not run before a late tombstone lands. The result column slot_removed becomes member_retired and namespaces_pending_gc is appended. The admin refuses a member without an epoch counter instead of minting one, and a mount that skips the capability probe now logs it. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A mount's startup sweep deleted every staging object under its prefix, including a successor's in-flight uploads when two processes of one member overlapped. Keys are now staging/<srid>/<writer_epoch>/<random>.tmp and the sweep deletes only epochs below the live one; flat keys count as epoch 0 and a nested member's keys are left alone. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A decommission keeps the epoch counter, so allocateWriterEpoch never mints over an absent counter for it and EpochMintPolicy goes. The vanished-slot pacing and the observation prune stay, because a 26.6.4 decommission or a hand delete still removes a mount slot. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MountLeaseRenewer::renewOn sampled the stop flag before the write. A stop that landed after the sample refused the admission with nothing sent, yet the renewal went terminal and finishTeardown skipped the farewell. Read the flag after the refusal instead. 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>
9fc9db0 to
3a2641d
Compare
Brings in #2566 (member lifecycle PR A, final review) and #2517 (renewal until a definitive answer). - `MountLeaseRenewer::renewOn`: the stop flag is read once, after the refusal, inside the #2517 call series, so a stop racing the admission stays releasable (93adba9). - The decommissioned-owner refusal keeps this branch's text: with the epoch nonce and `owner_life`, a retired id can come back through the owner rewrite in the troubleshooting docs. - #2566 tests for the terminal-state refusal and ids with `/` join the `CASDecommission` fixture. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uite `CASDecommission` is a `TEST_F` fixture on this branch; gtest refuses a plain `TEST` in the same suite. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
k-morozov
left a comment
There was a problem hiding this comment.
I am wandering about retries in remount loop.
Model explanation:
Self-remount over a retired owner retries forever instead of going terminal
Scenario: member r1 loses its lease (pause, partition), and GC fences it. Then SYSTEM CAS DROP POOL MEMBER 'r1' runs on another node and tombstones the owner (retired_at_ms, owner_life + 1). When r1 wakes up, its renewal ends as
terminal and remountLoop calls tryRemountOnce.
In tryRemountOnce, claimOwnerOrThrow (CasPool.cpp:1498, default RetiredOwnerPolicy::Refuse) throws CORRUPTED_DATA on the retired owner. This happens before the life check at :1499. The catch (...) at :1640 returns false.
remountLoop (CasMountRuntime.cpp:887-899) waits (backoff up to 30 s) and calls it again. remount_handled_generation is updated only on success, and remountTerminal() stays false, so the loop never ends until restart.
This retry can never succeed. The tombstone already moved owner_life, and life only grows. So after the tombstone the owner can only be in one of two states:
- still retired: the same throw again;
- un-retired (rollback or hand edit): a life mismatch, which leads to IdentityLost.
Effects:
- the pool stays in TransientNotLive (lifecycle = 'not_live' in system.cas_mounts), the same as a normal short lease loss;
- every 30 s it reads _pool_meta, the catalog and the owner;
- every attempt adds CASRemountFailed and a WARNING;
- CASIdentityLost is not incremented, so there is no terminal signal and no alert.
There is no safety problem, because nothing is written under the tombstone. This is an operability and consistency issue. The PR description says "an owner at another life ends the remount loop in IdentityLost", but a tombstone is
also a new life and does not end the loop. The PR notes the retry as known behavior ("still retries ... until the server restarts"), but does not explain why it is kept.
Suggested fix: on remount, compare owner_life before the retired check, or handle the retired owner the same way as a life mismatch, so that both call enterIdentityLost. Also add a troubleshooting row for this case and a test that
pins the lifecycle state.
| UInt128 write_attempt_id{}; | ||
|
|
||
| /// The holder's clean farewell (release): `min_active_build_sequence == UINT64_MAX`. | ||
| bool isFarewell() const { return min_active_build_sequence == std::numeric_limits<uint64_t>::max(); } |
There was a problem hiding this comment.
Suggest rename isCleanRelease() .
Model:
There are several reasons, and all of them are visible in the code.
-
The flag is encoded in a field that means something else. isFarewell (CasServerRootFormats.h:82) checks min_active_build_sequence == UINT64_MAX. That field is the build watermark ("the minimal active build"), and UINT64_MAX is
a sentinel value inside it. The method name does not show that it reads the watermark. A reader of body.isFarewell() is more likely to expect a separate field. -
One concept has several names. The state "the holder released the slot cleanly by itself" is named differently in different places:
- isFarewell (the new helper);
- clean_marker in claimMount;
- FenceCertificate::CleanFarewell in classifyFenceCertificate;
- "terminated" in listMounts / system.cas_mounts;
- release / terminate in MountLeaseRenewer;
- "farewell/retired sentinel" in prefixEligibleUnder.
The operator sees terminated, the code says farewell, and the renewer calls it release.
-
"Retired" now means two different things. The field comment says UINT64_MAX = retired (farewell) (CasServerRootFormats.h:75). In the same PR, "retired" also means the owner tombstone (retired_at_ms, retirement_verified,
member_retired). A farewell on the mount does not mean a retired member: every clean shutdown writes a farewell, and the member is not retired by that. In the decommission code the two concepts appear side by side, for example
in verifyAndMark: farewell_held && tombstone_held. -
The name does not say who wrote it. This is a property of the body that the holder wrote. But fencedCopy keeps min_active_build_sequence, so a body can be gc_fenced and isFarewell at the same time. Correctness depends on the
check order: claimMount, classifyFenceCertificate and listMounts check gc_fenced first. The name hides this detail.
Suggestions
- Rename it to isCleanRelease() or holderReleased(), or at least to isFarewellSentinel(), so that it is clear it is a sentinel in the watermark.
- Use one word for the concept: release in the code and terminated in the system table. This can go into a separate refactoring PR, because the helper itself is a drive-by change from 86b20da (the Ockham note about the
replacements in GC). - In the field comment, replace "retired (farewell)" with "clean release (farewell)", so that "retired" is not confused with owner retirement.
PR #2567 Audit ReviewVerdictOne High defect. The epoch counter and the kept control objects do what the PR says. The drain's reaction to a lost claim does not. A decommission that loses its slot after it has already deleted data rolls the owner back to an unretired life. The member can mount again, immediately, onto the hole. Two narrower findings sit beside that: a retired owner never ends a live remount loop, and a late roots delete can hit a rejoined member's identical bytes. What the change does
The run now:
Every owner write goes through Model
The central argument, and where it holdsKeeping the control objects is the right fix for the two bugs named in the PR.
A tombstone is None of that is what the High finding breaks. The break is the policy that undoes a tombstone after the drain has already deleted. FindingsRecovery and rollbackHigh: a fence during the drain un-retires the member after its data is already gone.
Observability and diagnosabilityMedium: a decommissioned member's still-running process retries remount forever and never says the identity is lost.
Partial and gray failureLow: a delete still in flight removes a rejoined member's roots object when the bytes match.
Checked and clean
Coverage summary
Operational readinessScope: Primary flows
Flow 1: Drop a stopped member1.1 A fenced drain reports failure and has already deleted data
Flow 2: The victim process is still up2.1 The process looks like a lease blip after it has been decommissioned
Flow 3: Hand-edit the owner to rejoin3.1 The access-check object can vanish after the new server creates it
Readiness verdict
Operator guidance
Recommended fixes
|
PR #2567 CI VerificationVerdictCI blocks approval. One job is The head is decommission, owner life, and the epoch nonce. The diff has no Iceberg file, no
The How the list was reduced
The regression database has fail rows in three jobs. Each recorded leaf is a scenario path. The shorter prefixes of that path (
Findings
|
| Who | Commits | Commits that fail this test |
|---|---|---|
| Other PRs | 174 | 34 (20%) |
antalya-26.6 branch |
12 | 1 (8%) |
| PR 2567 | 5 | 5 |
The five failing commits are 81e5e081 (2026-10-14), d6ffc7b7, 9fc9db0f, 3a2641dc, and the head. A 20% test can fail twice in a row. Five commits in a row is a different rate, and there is no path from owner life, the epoch nonce, or decommission into stripNothing.
What would close this: a run of the merge base that fails in stripNothing at a similar rate, or a frame that shows this diff passing a null type into that function. Until one of those exists, this job blocks approval.
Stateless tests (amd_asan_ubsan, cas s3 storage, parallel, 1/2) — infrastructure
03755_circular_dictionary failed once on the head. The server log in the test context is a refused connection while publishing a blob:
CasBlobPublishAttempt: Publication of CAS blob cas_s3/blobs/ch128/cf/cf7ba47141a3b0b20d1d56789c00082e (streaming) failed, attempt 1 of 8:
Poco::Exception. Code: 1000, e.code() = 111, Connection refused
bucket test, key cas_s3/blobs/ch128/cf/cf7ba47141a3b0b20d1d56789c00082e
The job then reran with the same randomized settings: 6 runs, 0 failed. Job 118354995975. Check summary: Failed 1, Passed 5554, Skipped 142.
Across every check, 60 days, this test has 0 fails in 572 branch runs and 0 fails in 3,365 other-PR runs. On this pull request the earlier four commits passed it. The failure is the storage endpoint refusing the connection. Rerun the job.
RegressionTestsRelease / CASAlter (attach, 3) — pre-existing-flaky
Job 118344581525. Seven leaves failed. Each one is NETWORK_ERROR (210) while a CAS write was being committed. Two wordings show up:
read of 'data/cas/ref_catalog': gave up at the policy deadline after N attempt(s)duringCREATE TABLECAS _ckpt for namespace '...@cas@': persistent CAS contention, the checkpoint contribution was not published
/alter/attach partition/part 3/operations on attached partitions/update in partition is code 341: mutation mutation_62.txt failed because the inner error is that same code 210.
Parent rows (/alter, /alter/attach partition, /alter/attach partition/part 3, and the feature nodes directly above each leaf) are cascade.
Same job, 60 days, excluding this commit:
| Scenario | Runs | Fails |
|---|---|---|
.../partition key datetime/attach partition from |
183 | 66 |
.../partition key/attach partition from without id |
183 | 63 |
.../check simple attach partition 2/simple attach partition |
183 | 61 |
.../partition key/attach partition from with id |
183 | 56 |
.../operations on attached partitions/update in partition |
183 | 54 |
.../rbac/check attach partition from with privileges |
183 | 38 |
.../conditions/primary key |
183 | 18 |
The job itself has a fail row on 19 of 19 other commits in the window. One red run is the baseline for this lane. The deadline and contention sentences are the existing CAS commit retry, not the decommission steps in this diff.
RegressionTestsRelease / CASS3CacheAlter (attach, 3) — pre-existing-flaky
Job 118344581389. One leaf:
/alter/attach partition/part 3/partition key/attach partition from with id
Code: 210. DB::Exception: CAS write could not be committed
(CAS _ckpt for namespace 'alter-cas-clickhouse1/store/c78/c7847f38-...@cas@':
persistent CAS contention, the checkpoint contribution was not published)
The four shorter prefixes of that path are cascade. On this job, excluding this commit, the leaf failed 34 times in 180 runs. The job has a fail row on 17 of 19 other commits.
RegressionTestsRelease / CASLightweightDelete (4) — pre-existing-flaky
Job 118344582054. One leaf:
/lightweight delete/concurrent delete/MergeTree/random delete half of the table without overlap
concurrent_delete.py:350
assert r.output == f"{block_size * partitions * (100 - percent_to_delete) // 100}"
AssertionError: Oops! Assertion failed
/lightweight delete, /lightweight delete/concurrent delete, and .../MergeTree are cascade. On this job, excluding this commit, the leaf failed 16 times in 190 runs. The job has a fail row on 18 of 19 other commits. The concurrent-delete row count is the usual miss for this scenario.
Grype — pre-existing-flaky
Both scans fail on Alpine High advisories. The keeper image reports CVE-2026-84782, CVE-2026-84784, CVE-2026-85091, and CVE-2026-72897. The Alpine server image reports CVE-2026-85091. Jobs 118352883660 and 118353778287. The diff does not change a Dockerfile, a package list, or an image base. A rerun scans the same base and fails the same way.
Recommendation
Do not approve on CI grounds while Unit tests (asan_ubsan) is unknown.
Rerun Stateless tests (amd_asan_ubsan, cas s3 storage, parallel, 1/2) if you want that box green. A rerun will not clear Grype, and it will not clear cas_alter_attach_3: that job is red on every other commit recorded in the last 60 days. The cache-alter and lightweight-delete jobs are red on 17 and 18 of 19. The unit test has failed on every commit of this pull request; a single green rerun would still leave the five earlier commits to explain.
…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>
The helper reads the `UINT64_MAX` sentinel in `min_active_build_sequence`, the build watermark, and "retired" in its field comment collided with the owner tombstone (`retired_at_ms`, `member_retired`) this series adds: every clean shutdown writes a farewell, and that does not retire the member. The name now says what the body records — the holder released the slot cleanly — and the comment says it is a sentinel in the watermark, independent of `gc_fenced`. Review: #2567 (comment) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Brings the StagingEpochMatchesAdmissionGeneration fix and the `isCleanRelease` rename; the two `isFarewell` call sites this branch added are renamed with it. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
SYSTEM CAS DROP POOL MEMBERno longer deletes a member'sowner,epochandmountobjects; it deletes only the member's data. Deleting those objects let a restarting victim be issued a writer epoch it already used and let a late tombstone land on a live successor. Keeping them makes epochs strictly increasing perserver_root_idacross lives and removes the tail's race handling.Changes
Writer-epoch counter nonce. Every counter write carries a fresh
allocation_nonce. Two processes with the sameserver_uuidthat race on an ambiguous counter write no longer resolve to the same epoch: their bodies differ, so the byte-equality resolve cannot adopt the other's write.Owner life. The owner object gains
owner_life, a counter every owner write bumps byIf-Match, andretirement_verified. A mount records the life it started under. After every slot claim (start and self-remount) it re-reads the owner and requires its own uuid, that life, and no tombstone; otherwise it fences its own slot body and fails. A self-remount compares the life before it allocates an epoch: an owner at another life ends the remount loop inIdentityLost, and a restart picks up the current life.Decommission run:
Every exit between steps 2 and 5 runs the confirm. When the tombstone's outcome is unknown and the owner does not read back as the tombstone, the run fails without its clean farewell (
Pool::withholdFarewell): the slot keeps the admin's live body, so the victim restarts only after GC fences it (about 1.05 × the lease TTL plus a GC round) instead of reclaiming at once.member_retired = 1only for a verified mark. An owner that moved on to a later unretired life (a rejoin, or another run's rollback) reportsmember_retired = 0with its own warning. A run that cannot tell whether its tombstone or verified mark landed re-reads the owner before advising a rerun.S3 staging keys carry the writer epoch (
staging/<srid>/<epoch>/<random>.tmp), built and parsed inLayout. The startup sweep deletes only earlier epochs, so a paused older process can no longer delete its successor's staging. The epoch and the admission generation come from one capture.Removed: the epoch re-mint policy for decommission (
EpochMintPolicy). A member whose counter is missing (left by an older decommission) is refused, not repaired.A mount that skips the capability probe (
skip_access_check, always the case for decommission) logs it.Lease renewer fix, all CAS mounts. The stop flag was read before the renewal's admission. A stop landing in between made the renewal terminal with nothing sent, so a clean shutdown skipped its farewell and the next start paid the unclean-reclaim wait.
Risks / notes
slot_removedis renamedmember_retiredat the same position, andnamespaces_pending_gcis appended afterwarnings. Rows stillRemovingafter the drain are reported innamespaces_pending_gc; GC deletes them and no rerun is needed.server_root_idrefuses to mount, as before. The refusal names the owner object, its retirement and its life; the troubleshooting table gives the owner rewrite a restart accepts. The alternative is a newserver_root_id. Rejoining a retired id without a hand edit is a separate change.allocation_nonce,owner_lifeandretirement_verifiedare optional plain keys; 26.6.4 readers skip them. An owner withoutowner_lifereads as life 0, and a body without the new fields keeps its exact bytes.member_retired = 0with "the slot does not hold this run's farewell" and asks for a rerun, which waits until GC fences the slot (about one mount lease TTL plus a GC round).test_cas_drop_pool_memberaccepts exactly this outcome;CASDecommission.LongDrainOutlivesOneTtlplaces the stop between two renewals and requires the farewell. Follow-up: shutdown waits for an in-flight renewal.Testing
CASDecommission.UnresolvedTombstoneLeavesNoFarewell: an unresolved tombstone leaves the admin's live body, and the victim reclaims only after a fence.CaEpochAllocCoreround 12 (docs/superpowers/models/CaEpochAllocCore_RESULTS.mdon the CAS design branch): the fenced tombstone keepsNoTombOverLive; without it, and with the wider rollback rule, the sabotages are red.CAS*unit tests: release 2720, ASan 2724, all passed.test_cas_drop_pool_member: 2/2.CAS*.Commits
8c0f14fd7fc4Add an allocation nonce to the CAS writer-epoch counter86b20da89e7fRecord the CAS owner life and recheck the owner after a mount claimd1f194489a2cKeep the CAS control objects on decommission and fence the tombstone133ee5786ae3Put the writer epoch into CAS S3 staging keyse25a8282614eRemove the CAS epoch re-mint policy for decommission93adba90c85dKeep a CAS renewal refused by a mid-renewal stop releasable3a2641dc7032Document the CAS member lifecycle and decommissionChangelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
SYSTEM CAS DROP POOL MEMBERkeeps the member'sowner,epochandmountobjects, tombstones the owner before draining and verifies the retirement. Its result columnslot_removedis renamedmember_retired, andnamespaces_pending_gcis appended. Every CAS mount re-reads its owner after claiming its slot. Writer-epoch allocation is unique between same-uuid processes, and S3 staging keys carry the writer epoch. A clean CAS shutdown no longer skips its farewell when the stop races the start of a lease renewal. A decommission whose tombstone outcome is unknown no longer writes its clean farewell, so the member restarts only after GC fences its slot.Documentation entry for user-facing changes
CI/CD Options
Exclude tests:
Regression jobs to run: