[k-morozov]

Changelog category (leave one):

• Critical Bug Fix (crash, data loss, RBAC)

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

Fix races in CAS garbage collection that could delete manifests during namespace creation or recreation, causing data loss.

Documentation entry for user-facing changes

...

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)

[github-actions]

Workflow [PR], commit [4d43cbd]

[alsugiliazova]

PR #2730 Audit Review

AI audit note: This review was generated by AI (Grok 4.7).

Verdict

No confirmed defects in reviewed scope.

The race the changelog names is closed. A staged manifest is nominated only after the mount floor says its build is finished and a second catalog read still shows the same row, or no row.

Confirmed defects

No confirmed defects in reviewed scope.

What the change does

planManifestCursorPage used to apply the mount floor only when the first catalog cut, K0, already had a row for the namespace. A manifest written by stageManifest before precommitAdd has no row, so the old planner nominated it. Phase 18 then deleted the body by exact etag after the round CAS. That etag does not change when a ref starts pointing at the body, so the delete left a dangling ref.

Before	After
Floor check only when K0 has a catalog row	Floor check for every listed manifest, including a namespace with no row
No second catalog read	One K1 read after every floor observation and before any candidate body hint, only when the page has tentative candidates
A missing row skipped the protection view and could be nominated immediately	A missing row is still skipped for recovery, and it is nominated only when the build is under the floor and the row is absent in both cuts
prefixEligible / prefixEligibleUnder take a MountLease	buildPrefixEligibleUnderFloor takes a BuildFloor of writer_epoch and min_active_build_sequence. The four comparisons are unchanged

CasGc, fsck, and decommission call the renamed pool helper. sweepNamespace still returns 0 when the namespace is absent or Creating.

Why the fix holds

A key is nominated only when all three of these are true:

1. isBuildUnderFloor says the build is retired. No mount lease admits nothing. Within one writer_epoch, retirement is min_active_build_sequence > build_sequence. An older epoch is retired. The UINT64_MAX farewell sentinel retires every sequence in that epoch.
2. The K0 row, when present, still passes the existing protection view and manifestDeletionPremise. A missing row skips that view: there is no life to recover.
3. excludeCandidatesWithChangedCatalogEntries keeps the candidate only when the row is absent in both cuts, or CatalogEntry compares equal, including incarnation, lifecycle, creator fence, and removal_started_round. Any other change increments skipped and drops the candidate. The page does not rebuild the protection view from the newer row.

That lines up with the writer order at this head.

• retireBuildSeq runs after promote has committed, after abandon has removed a live precommit, or from the destructor when no precommit was attempted. A destructor that still owes a precommit enqueues a cleanup duty and leaves the sequence active.
• minActive() is the lowest sequence still in active_build_seqs, or next_build_seq when the set is empty. The published floor lags low. A lagging floor keeps the manifest.
• A floor that already shows the build retired was sampled after that retire, so a committed ref's catalog row already existed. K1 is read after the floor observation, so a row that appeared after K0 fails the equality check and the body stays.
• casUpdate cannot add or delete catalog entries. deleteCompletedRemoving erases a row only when the adopted seal has cleanup_evidence and the coverage is not held. cancelStalledCreating erases only a Creating row whose creator fence is already terminal, and a Creating row cannot publish.
• precommitAdd accepts a fresh ManifestId only from the transaction that staged it. allocateBuildSeq does not reuse a sequence. A finished build cannot gain an owner on that key later.

A K1 transport error, a malformed catalog, or an ambiguous life index throws inside the recheck, before any body read. Gc::fold copies manifest_sweep_cursor from the returned result into the round CAS. The throw leaves the previous cursor in place.

Keys spared by K1 stay spared until the cursor wraps cas/manifests/. The page does not roll the cursor back and does not refill the nomination budget. The new request-count tests lock that in. The cursor is a cleanup-progress hint, not reachability authority.

The renamed predicate is the same four outcomes: older epoch, same epoch below the floor, same epoch at or above the floor, and the farewell sentinel. toBuildFloor copies the two fields the old predicate read off MountLease.

What was checked and found clean

• Floor cache. One mount read per namespace per page. A low sample retains every later build of that namespace on the page. Retirement only moves forward, so an admission stays valid.
• List-only and empty-candidate pages. nomination_budget == 0 advances the cursor and does not read K1. excludeCandidatesWithChangedCatalogEntries returns before the read when candidates is empty.
• Work budget. An ineligible rowless namespace now continues before the protection-view slot is taken. An eligible one still consumes a slot, as every rowless namespace did before this PR.
• Phase 18. Deletion is still an exact-etag remove after the round CAS adopts the source retirements. A mismatch throws CORRUPTED_DATA.
• Rename call sites. Gc::fold, rebuildBaseline, fsck's pre-precommit classification, and the decommission comment all use buildPrefixEligibleUnderFloor. No remaining prefixEligible at this head in the changed files.
• Planner concurrency and C++ lifetime. One thread. The floor map and the candidate vector are local. The K1 compare holds pointers into the two snapshots for the duration of erase_if. Unsigned epoch and sequence compares. The farewell sentinel is tested before the sequence compare. A throw drops the nominations with the stack. No new lock.

Coverage

Category	Status	Note
Active build, no catalog row	Reviewed	Floor retains. CursorPageRetainsStagedManifestUntilFirstPublication
Publish and floor advance between K0 and the floor read	Reviewed	K1 sees the new row and retains
Incarnation, disappearance, or Live → Removing between the cuts	Reviewed	Inequality retains. Another namespace's change does not
Absent in both cuts, build below the floor	Reviewed	Nominated. The staging transaction has retired, so it cannot publish that key
No mount lease, sequence equal to the floor, newer epoch	Reviewed	Retained
K1 transport, decode, ambiguous life index	Reviewed	Fail closed. No body read, no cursor CAS
Stale-low cached floor	Reviewed	Retains
Cursor progress after a K1 spare	Reviewed	Not rewound. Wrapped only at the real end of the keyspace
Eligibility rename	Reviewed	Same four comparisons
Exception / partial update	Reviewed	Throw before nomination and before the round CAS
Memory lifetime, iterator invalidation, overflow, data races	Reviewed	Local containers. No shared mutable state added
sweepNamespace / decommission policy	Reviewed	Unchanged: absent and Creating names are not deleted there
Version skew / on-disk format	N/A	No object layout change. An old GC leader keeps the old planner until that replica is upgraded
Auth across nodes	N/A	No new RPC. The mount fence and catalog CAS are unchanged

Assumptions and limits. Static reading of ddcb71f1. The new gtests were read and not run. The author's note says CASOrphanManifestSweep, CASOrphanSweepRequests, and CASSweepDeletionPremise passed after the fix (53 tests).

Operational readiness

Scope. Background orphan sweep on the GC leader while a writer stages a manifest for a new or recreated namespace. Persona: operator / DBA.

Primary flows.

1. INSERT or FREEZE stages a manifest before precommitAdd, while GC is planning a page.
2. A namespace row appears, disappears, or changes incarnation during that page.
3. A build dies before precommitAdd and leaves a body for a later page to reclaim.

No operator-facing defect in those flows. There is no new setting. A body whose namespace does not resolve to a mount lease stays until a lease exists. A body the second catalog read spares is visited again after the manifest cursor wraps cas/manifests/.

The data-loss race remains on a replica that still runs the old planner and wins GC leadership. Upgrade that replica onto this build.

[alsugiliazova]

PR #2730 CI Verification

AI audit note: This review was generated by AI (Grok 4.7).

Field	Value
PR	Altinity/ClickHouse#2730 — Fix early manifest deletion in CAS GC
Base	antalya-26.8 @ fc6d4404b640140fe27cb3f19f4f9e39b9184d77 (merge parent of the head)
Head	ddcb71f151d8d81de87fa6b16e5d365578cf1428
Run	41465210599 attempt 1
Report	ci_run_report.html
Method	pr-ci-failure-triage. Job logs and the CI report for this attempt. No gh-data query: the database password was not available. The report's comparison with fc6d4404 is the baseline used below

Verdict

No failure in scope is caused by this pull request.

The diff is the orphan-manifest sweep. Unit tests passed (fail: 0, passed: 23744), including the new sweep gtests. Every CAS regression lane that finished passed. Iceberg, Parquet, OAuth, and S3 export partition passed.

The red integration jobs are a named-collection grant, an S3 multipart assert in copyS3File.cpp, and a cluster that did not start. Swarm is out of scope for this pull request and is not part of the verdict. Two regression jobs were cancelled before they reported a result.

Category	Count	Where
regression	0	—
pre-existing-flaky	54	21 Iceberg export tests; 33 database-backup tests
infrastructure	4	test_custom_s3_disk_remote_url_allow_hosts on shard 7/8 and on the targeted job
cascade	0	—
unknown	0	—

How the list was reduced

The CI report has 58 integration failures. The Swarms job is out of scope and is not classified.

1. No cascade in scope. The integration rows are separate tests hitting one of three errors, not a server death followed by later tests.
2. No bulk rate query. The remaining rows were placed from the log and from whether fc6d4404 already failed them. The "New Fails in PR" tab contains only the 18 targeted-job rows.
3. Infrastructure. The two compatibility tests time out while the cluster is starting. Shard 7/8 also prints manifest for clickhouse/clickhouse-server:23.3.19.33.altinitystable not found. Four rows (two tests, two jobs).
4. Investigated. The Iceberg grant and the backup abort.

pre-existing-flaky

21 tests in test_export_partition_iceberg.py, all in Integration tests (amd_asan_ubsan, db disk, old analyzer, 5/8).

create_iceberg_table fails at test_export_partition_iceberg.py:156 with code 497: default: Not enough privileges. To execute this query, it's necessary to have the grant SHOW NAMED COLLECTIONS SECRETS ON s3. The stack is NamedCollectionsHelpers.cpp and ContextAccess.cpp. This diff does not touch grants or named collections. The CI report does not list these rows as new against fc6d4404.

33 test_database_backup rows: 17 in shard 6/8, 16 in Integration tests (amd_asan_ubsan, targeted).

The server aborts with Logical error: 'Chosen multipart upload for an empty file. This must not happen' in src/IO/S3/copyS3File.cpp (UploadHelper::performMultipartUpload). Both cas_policy and default cases die this way. This diff does not touch copyS3File.cpp. Shard 6/8 is absent from the new-vs-base tab, so that shard already failed on fc6d4404. The targeted job is the same tests run again; the report marks that copy new.

No 60-day fail rate was queried.

infrastructure

test_table_on_custom_s3_disk_loads_after_upgrade and test_database_on_custom_s3_disk_loads_after_upgrade, shard 7/8 and the targeted job.

Setup never reaches the test. Shard 7/8: manifest for clickhouse/clickhouse-server:23.3.19.33.altinitystable not found, then Timed out while waiting for instance node to start with the container still running. The targeted job reports the same timeout. The image tag is not produced by this pull request.

Out of scope

The Swarms job failed three leaf scenarios (wrong cluster name, CPU overload, CPU overload of every node) and their parent paths. This pull request does not change swarm discovery or remote reads. Those rows are not classified and do not affect the verdict.

Cancelled before a result

These are not test failures. They also are not a pass.

Job	What happened
RegressionTestsRelease / S3Export (part)	Cancelled during Run s3 suite after about 3.5 hours. s3_export_partition in the same workflow passed
RegressionTestsRelease / CASS3CacheLightweightDelete (3)	Cancelled while Run lightweight_delete suite had not finished. The other CAS lightweight-delete lanes passed, including non-cache lane 3

What passed that this pull request cares about

CAS regression, including S3-cache lanes that finished: all success. cas itself is 52/52. Iceberg (1) and (2), Parquet, Parquet S3 (minio and aws_s3), OAuth, and S3 export partition passed. Stateless debug is 13637 passed, 0 failed. The two selected stateless shards passed. Unit tests (asan_ubsan) passed.

[filimonov]

maybe let's backport it to 26.6 later.