Date a GC mount-token sighting after the read that returned it by filimonov · Pull Request #2509 · Altinity/ClickHouse

filimonov · 2026-10-08T11:14:48Z

The GC credited the first sighting of a mount token with a clock sample taken before the LIST of gc/server-roots/ and the slot read. A slow walk to the slot counted as time the token had been watched, and a later round could write gc_fenced over a holder whose last renewal still gave it authority. The sighting is now dated after the read that returned the token.

Why it matters for the lease

The GC on another server is the observer that decides a writer is dead: it waits until the slot token has stayed unchanged for cas_mount_lease_ttl_ms + 5 % + its own poll step on its own clock (about 41.5 s with the defaults), then sets gc_fenced and drops the protection of that writer's unfinished work. It never compares clocks across servers; the only thing that makes the decision safe is that the wait starts when the observer actually saw the token. Starting it earlier shortens the window below the lease, so a holder that still has authority can be fenced: a ref-log append that lands after such a fence leaves a reference to blobs the GC may delete.

This becomes reachable with the next change of the series (#2431): the background renewal will keep retrying through a storage outage instead of fencing itself after 4 s, so a live holder will routinely spend tens of seconds without changing its token. Roll this change out on GC nodes before that one reaches the holders.

Changes

computeHeartbeatFloor takes the observer clock as a function; both callers (the regular round and the rebuild) pass it. The first sighting is sampled after the read; the re-decision after a refused fence-out dates its observation anew.
The stability test keeps its entry sample, and a sample older than the sighting never counts as time held (MountIncarnationObservation::heldFor), so the elapsed time cannot wrap.
A transient read error leaves the accumulated observation untouched (behaviour kept, now pinned by a test).

Risks / notes

GC-side only, in-memory state; no format or protocol change. Mixed versions are safe in either order for correctness, but the deployment order above is needed before the renewal change.
The rebuild path cannot fence in production (it runs a one-shot Gc with an empty observation map); the fix there keeps both callers consistent.

Testing

CaMountObserveCore rows sab_sighting_before_read and sab_redecision_stale_sample show the fence inside authority without the fix; main is green with it.
Unit: five regression tests red on the base (gtest_cas_mount.cpp, gtest_cas_gc_ack_floor.cpp: round and rebuild), two behaviour-preservation tests; CAS* gate 2615 (release), 2619 (ASan, no error).

Closes backlog task CAS-342.
Related: #2431

Changelog category (leave one):

Bug Fix (user-visible misbehavior in an official stable release)

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

CAS GC no longer fences a mount whose token it watched for less than the observation threshold after a slow walk to the slot: the first sighting is dated after the read that returned the token.

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

CI/CD Options

Exclude tests:

Regression jobs to run:

The GC credited the first sighting of a mount token with a clock sample taken before the LIST of gc/server-roots/ and the slot read. A slow walk then counted as time the token was watched, and a later round could write gc_fenced over a holder whose last renewal still gave it authority. Both callers of computeHeartbeatFloor (the regular round and the rebuild) now pass the observer clock as a function; the sighting is sampled after the read, including the re-decision after a refused fence-out. The stability test keeps its entry sample, and a sample older than the sighting never counts as time held, so the elapsed time cannot wrap. Closes backlog task CAS-342. Related: #2431 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

github-actions · 2026-10-08T11:16:12Z

Workflow [PR], commit [bc7f453]

…ng-after-read Date a GC mount-token sighting after the read that returned it Source-PR: #2509 (#2509)

alsugiliazova · 2026-10-13T13:05:55Z

PR #2509 Distributed-Systems Audit

AI audit note: This review was generated by AI (Claude Opus 5.5).

Summary

No confirmed distributed-systems defects in the reviewed scope.

The fence is safe only if the GC fences no earlier than one observation threshold after the holder's last renewal was written. Before this PR, the first sighting of a token was dated with a clock sample taken at round entry, before the LIST and the slot GET. A renewal that landed during that walk was credited with time that passed before it existed. Now the sighting is sampled after the read that returned the token, and that read is ordered after the write that installed the token. So first_seen >= write time, and a fence that needs round_start - first_seen >= threshold cannot land earlier than write time + threshold. The stability test still uses the entry sample, which is older than the read and the guarded fence write, so it can only undercount the time held.

Model assumptions


Actors	The mount holder (renews its slot), the GC leader on another server (observes slots and fences them), and the object store
Channels	Slot `GET` and conditional `PUT` on `gc/server-roots/<srid>/mount`, and the `LIST` of `gc/server-roots/`
Authoritative state	The slot body and its etag. Every renewal increments `seq`, so the etag changes. The GC's observation map (`mount_obs`) is in memory only
Failure model assumed by the code	Crash-recover processes. Unbounded store latency. Read-after-write consistency on a single key. No cross-node clock comparison: each side measures time on its own `CLOCK_BOOTTIME`, with a 5 % rate allowance
Safe failure criterion	A holder is never fenced while its last renewal still gives it authority. A late fence or a missed fence only delays reclaiming garbage

Evidence

The sighting sample cannot predate the token. readModifyWrite calls decide only after observe returns (first pass) or after the write's resolve read, followed by the conflict pause (re-decision). Both are after a read that returned current. mono_ms_fn() inside decide is therefore no earlier than that read. Any extra delay (a backoff pause, a slow thread) only moves first_seen later, which is the safe direction.
The stability test is conservative. heldFor(stable_threshold_ms, round_start_ms) measures with the sample taken before every read in the call. The fence PUT is guarded by the observed etag, so it lands only if the token is still current at a point after round_start_ms.
Re-decision after a refused fence. On a conflict, the new body has a different etag. The !stable arm then re-inserts with a fresh post-read sample and returns nullopt, so readModifyWrite declines and does not loop again. If a body with the same etag is re-entered in the same call, first_seen > round_start_ms, and the sample_ms >= first_seen_mono_ms guard in heldFor returns false instead of wrapping.
An unchanged token keeps its first sighting. Within one leader, two sightings of the same etag separated by the threshold are sufficient whatever happened in between: no renewal can leave the etag unchanged. This holds across a failed walk (FailedSlotReadKeepsTheWatch) and across a gap in GC leadership.
Observer clock. Gc defaults mono_ms_fn to store->bootMsNow(), which uses CLOCK_BOOTTIME, as earlier audits in this directory established. A GC host suspend counts as elapsed time, and real time passes for the holder's lease too, so the observer stays sound.
Callers. Both callers now pass the member std::function instead of its value. The function outlives the call. The rebuild call starts from the instance's mount_obs. The PR says the rebuild runs on a one-shot Gc with an empty map, so it cannot reach a fence on its first sighting.
Unit tests. They cover a first sighting after a slow walk (FirstSightingCountsFromAfterItsRead), a re-decision after a refused fence (RedecisionAfterARefusedFenceCountsFromItsRead), a confirming call whose walk crosses the threshold (ConfirmingCallDecidesOnItsEntrySample), a backward clock (SampleOlderThanTheSightingNeverFences), and a failed slot read (FailedSlotReadKeepsTheWatch). Round-level and rebuild-level cases are in gtest_cas_gc_ack_floor.cpp.

Notes (not defects)

The deployment order is documented, not enforced. The PR asks for GC nodes to take this change before holders take the follow-up from #2431 (renewal retries through a storage outage instead of self-fencing after 4 s). Nothing in the code stops a holder carrying CAS: an 8-second S3 503 fences the mount lease while the lease budget remains #2431 from running against a GC without this fix. Check this in the CAS: an 8-second S3 503 fences the mount lease while the lease budget remains #2431 review, for example with a pool-generation or compatibility gate, or a release note that makes the order mandatory.
A slow walk costs liveness. All slots in one call are judged against the entry sample. If the walk takes long, slots late in the LIST can miss the threshold by up to the walk's duration and are fenced one round later. This delays garbage reclaim and does not affect safety.
This PR does not cover an in-flight holder write. Safety also depends on the holder's ref-log append landing before the fence. The margin for a request still in flight is ttl_ms / 20 + mount_renew_period, which is the existing design and not changed here. That part belongs in the CAS: an 8-second S3 503 fences the mount lease while the lease budget remains #2431 review, because CAS: an 8-second S3 503 fences the mount lease while the lease budget remains #2431 lengthens the time a live holder spends without renewing.

Coverage summary

Fault class	Status	Notes
Network / transport	Reviewed	Slot reads and the fence `PUT` use `Retry::standard()`. A read that gives up throws out of the walk and leaves `obs` unchanged, so a transient error neither restarts nor shortens the watch.
Partition / availability	Reviewed	A GC that cannot reach the store cannot fence. When only the holder loses the store, the GC fences after the threshold, which is the intended outcome once the holder's authority has lapsed.
Timing / ordering	Reviewed	This is the class the PR fixes. The sighting is ordered after the token's write through the read, and the threshold is measured on one node's `CLOCK_BOOTTIME`. The wrap guard in `heldFor` covers a sample older than the sighting.
Partial / gray failure	Reviewed	A slow store or a slow walk delays the sighting and the fence. It never brings either earlier. An ambiguous fence `PUT` goes through the existing resolve-by-read path in `writeLoop`.
Idempotency & duplicates	Reviewed	The fence is etag-guarded and sets `seq + 1`. A repeated round sees `gc_fenced` and counts the slot `already_fenced`.
Backpressure & resource limits	Reviewed	One extra clock read per new sighting. `obs` is still pruned of srids that are no longer listed.
Recovery & rollback	Reviewed	`obs` is in memory only. A new leader starts empty, which delays a fence and never brings it earlier.
Configuration & version skew	Reviewed	No format or protocol change. Old and new GCs are each safe against today's holders. The order relative to #2431 is in Notes.
Security / trust	N/A	No new peer, credential, or request path.
Observability & diagnosability	Reviewed	The fence log line and `GcFenceOut` event are unchanged. No metric shows how long the walk took relative to the threshold. That is acceptable, because it only affects how soon a fence lands.

Limits

Static review of head bc7f453aa4a7. The new gtests were not re-run, and no fault injection was run against two live servers sharing a bucket. The #2431 holder change was not reviewed.

alsugiliazova · 2026-10-13T13:06:24Z

PR #2509 CI Verification

AI note: This verification was generated by AI (Claude Opus 5.5).

Field	Value
PR	Altinity/ClickHouse#2509 — Date a GC mount-token sighting after the read that returned it
Head	`bc7f453aa4a7e2a11deff1004813abbf8437291d`
Run	37768797721
State	Merged 2026-10-08 into `antalya-26.6-20261008` as `2f34581b`
Method	`pr-ci-failure-triage`: job artifacts, plus a branch baseline from `gh-data.checks` (60 days)

Verdict

None of the failures is caused by this PR. Of 5 failing checks, 3 are pre-existing failures, 1 is a cascade of one of them, and 2 are Grype image scans, which are not tests.

Coverage gap: the CAS regression suite did not run. The CAS checkbox was ticked after CI started, so ci_regression_jobs did not include cas. All nine CAS regression jobs show as skipped. The only CAS regression run since the merge was on antalya-26.6 at 0758ad1f, which does not contain this fix.

Failures

Check	Failure	Category	Evidence
Unit tests (asan_ubsan)	`IcebergMetadataGenerator.ModifyColumnAfterItselfWithoutTypeIsNoop`, UBSan: member call on null pointer (`IDataType.h:380`, from `Iceberg::stripNothing`)	`pre-existing-flaky` (consistent since 2026-10-07)	Fails in 14 other PRs and on the branch (1 of 6 runs). It first failed 2026-10-07 21:23, before this PR's run. The code is Iceberg, not CAS.
Stress test (amd_asan_ubsan)	`Cannot start clickhouse-server`: the Hybrid table `test_2.t_settings_only` cannot attach because `test_2.local_hot` is missing	`pre-existing-flaky`	Same error on the branch (2 of 2 runs, 2026-10-02 and 2026-10-09) and in PR 2470. The test name has failed since 2026-08-18 in 6 other PRs.
Stress test (amd_asan_ubsan)	`Check failed`	`cascade`	Consequence of the server failing to restart.
Integration tests (amd_asan_ubsan, db disk, old analyzer, 5/8)	`test_dns_cache/test.py::test_host_is_drop_from_cache_after_consecutive_failures`, `wait_for_log_line` timed out	`pre-existing-flaky`	Branch: 1 of 24 runs. Fails in 5 other PRs since 2026-08-28. DNS cache code, which this PR doesn't touch.
GrypeScanKeeper, GrypeScanServer (-alpine)	4 and 1 high/critical CVEs in the image	not a test	Image vulnerability scan. It doesn't depend on this C++ change.

What this PR's own tests show

Unit tests (asan_ubsan): all 2570 CAS* tests ran and passed before the Iceberg UBSan abort. That includes the five new CASHeartbeatFloor tests and CASGcFenceOut.RoundCountsASightingFromAfterItsRead / RebuildCountsASightingFromAfterItsRead.
Stress test (amd_asan_ubsan, cas s3 storage) and Stress test (amd_debug, cas s3 storage): passed.
Stateless tests with CAS storage (amd_binary cas storage, amd_binary cas s3 storage, amd_asan_ubsan cas s3 storage 1/2 and 2/2): passed.
The selected regression suites passed (Iceberg, OAuth, Parquet, S3 Export, Swarms).

Recommendation

Approval is not blocked by any failure. Before relying on the fix, run the CAS regression suite on antalya-26.6-20261008 (or on the build that carries 2f34581b). This change alters when the GC fences a mount, which is exactly what CAS regression covers and the unit tests only cover in isolation.

k-morozov approved these changes Oct 8, 2026

View reviewed changes

filimonov added the CAS label Oct 8, 2026

This was referenced Oct 8, 2026

Wait for the lease a CAS mount holder declares before fencing or reclaiming its slot #2516

Merged

Renew the CAS mount lease until a definitive answer; an expired lease is a pause #2517

Merged

filimonov added antalya antalya-26.6 labels Oct 8, 2026

filimonov merged commit 2f34581 into antalya-26.6-20261008 Oct 8, 2026
305 of 316 checks passed

svb-alt added the port-antalya PRs to be ported to all new Antalya releases label Oct 11, 2026

zvonand pushed a commit that referenced this pull request Oct 12, 2026

Merge pull request #2509 from Altinity/fix/antalya-26.6/cas-gc-sighti…

9a09242

…ng-after-read Date a GC mount-token sighting after the read that returned it Source-PR: #2509 (#2509)

This was referenced Oct 12, 2026

Antalya 26.8: auto-grp-pr-2159: combined port of 23 PRs #2540

Merged

Port graph for antalya-26.8 #2322

Open

k-morozov added the verified Approved for release label Oct 20, 2026

filimonov mentioned this pull request Oct 22, 2026

Merge antalya-26.6-20261008 into antalya-26.6 #2644

Merged

10 of 30 tasks

svb-alt added the antalya-26.6.8.20003 label Oct 27, 2026

zvonand removed the port-antalya PRs to be ported to all new Antalya releases label Oct 28, 2026

Navigation Menu

Repository navigation

Date a GC mount-token sighting after the read that returned it - #2509

Date a GC mount-token sighting after the read that returned it#2509
filimonov merged 1 commit into
antalya-26.6-20261008from
fix/antalya-26.6/cas-gc-sighting-after-read

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Uh oh!

Changelog category (leave one):

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

Documentation entry for user-facing changes

CI/CD Options

Exclude tests:

Regression jobs to run:

Uh oh!

Uh oh!

Uh oh!

PR #2509 Distributed-Systems Audit

Summary

Model assumptions

Evidence

Notes (not defects)

Coverage summary

Limits

Uh oh!

PR #2509 CI Verification

Verdict

Failures

What this PR's own tests show

Recommendation

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants