Skip to content

CAS GC: cut batch deletes to the storage's batch-delete limit - #2484

Merged
filimonov merged 1 commit into
antalya-26.6from
fix/antalya-26.6/cas-batch-delete-key-limit
Oct 7, 2026
Merged

filimonov merged 1 commit into
antalya-26.6from
fix/antalya-26.6/cas-batch-delete-key-limit

Conversation

@filimonov

Copy link
Copy Markdown
Member

CAS GC sent a write-once delete cohort (up to cas_gc_bulk_delete_chunk_keys, 1000 keys) as one request, ignoring objects_chunk_size_to_delete on S3 and the one-key limit of a storage without DeleteObjects. GC now cuts each cohort into requests of the storage's batch-delete limit. The S3 delete paths also keep the store's error name in S3Exception.

Changes

  • IObjectStorage::batchDeleteKeyLimit (default 1). S3ObjectStorage returns objects_chunk_size_to_delete, or 1 once DeleteObjects is known unsupported.
  • Cas::Backend::bulkDeleteKeyLimit, forwarded by every decorator; kBulkDeleteMaxKeys for the in-memory and the emulated backend.
  • Gc::bulkDeleteChunkKeys is the smaller of the setting and that limit. The manifest_deletes phase and cleanupRefObjects cut a cohort into requests of that size through the existing removeChunkWriteOnceOrOneByOne. The cohort, its budget accounting and the one authorityHolds read per cohort are unchanged.
  • S3ObjectStorage::removeObjectsIfExistImpl and deleteFileFromS3 pass the store's error name into S3Exception: isRefreshableCredentialError recognises errors the SDK does not model only by getExceptionName.

Risks / notes

  • Touches shared code outside CAS: one virtual method in IObjectStorage.h, one override and two throw sites in S3ObjectStorage.cpp, one throw site in src/IO/S3/deleteFileFromS3.cpp. The exception type and message are the same; only the name is added.
  • A disk with objects_chunk_size_to_delete below 1000 now gets more, smaller DeleteObjects requests from GC phases 15 and 17.
  • The namespace janitor of CAS GC: batch the namespace janitor's deletes under a soft time budget #2483 does not use this limit yet. That needs both PRs in the base and comes as a follow-up.
  • No on-S3 format change.

Testing

  • Unit: CAS* gate, debug and ASan. New: CASBulkDeleteBackend limit forwarding and clamping (5 tests), CASGCManifestBulkDelete.StorageLimitCutsTheCohortIntoRequestsOfAtMostThatManyKeys, CASRefGc.RefObjectCleanupCutsRequestsToTheStorageLimitWithoutExtraAuthorityReads, CASS3BatchDelete (the S3 limit; error names for a whole-request error and for a per-key error in a 200 reply). With bulkDeleteChunkKeys ignoring the backend limit, both GC-level tests and two CASBulkDeleteBackend tests fail.
  • Not done: the CASS3BatchDelete error-name test was not run against the old throw sites; no integration run on RustFS or S3.

Related: #2483
Related: #2468

Changelog category (leave one):

  • Improvement

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

CAS GC batch deletes respect the storage's batch-delete limit (objects_chunk_size_to_delete on S3), and S3 delete errors keep the store's error name.

Documentation entry for user-facing changes

  • Documentation written in this PR (docs/en/antalya/cas/configuration.md)

CI/CD Options

Regression jobs to run:

  • CAS (content-addressed storage; Antalya only)

GC sent a write-once cohort (up to `gc_bulk_delete_chunk_keys`, 1000)
as one `removeManyWriteOnce`, ignoring `objects_chunk_size_to_delete`
on S3 and the single-key reality of storages without `DeleteObjects`.

Add `IObjectStorage::batchDeleteKeyLimit` and `Backend::bulkDeleteKeyLimit`
(forwarded by every decorator). `Gc::bulkDeleteChunkKeys` takes the
smaller of the setting and that limit; `removeCohortWriteOnce` cuts the
`manifest_deletes` and `cleanupRefObjects` cohorts into requests of that
size. The cohort, its budget accounting and the one `authorityHolds`
read per cohort are unchanged.

The S3 delete paths also pass the store's error name into `S3Exception`:
`isRefreshableCredentialError` recognises SDK-unmodelled errors only by
`getExceptionName`.

Tests: `CASBulkDeleteBackend` limit forwarding and clamping, GC cut for
`manifest_deletes` and ref cleanup (no extra `gc/state` reads), and
`CASS3BatchDelete` for the S3 limit and error names.

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

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Workflow [PR], commit [7946c7c]

@filimonov
filimonov marked this pull request as ready for review October 5, 2026 15:57
@filimonov
filimonov requested a review from k-morozov October 5, 2026 15:58
@filimonov
filimonov merged commit 5b7c9f5 into antalya-26.6 Oct 7, 2026
473 of 518 checks passed
@filimonov filimonov added the port-antalya PRs to be ported to all new Antalya releases label Oct 7, 2026
@zvonand zvonand mentioned this pull request Oct 7, 2026
51 tasks done
@alsugiliazova

Copy link
Copy Markdown
Member

PR #2484 CI Triage

No CI failure is caused by this PR. GitHub is red, and every red check is a pre-existing failure or an image scan. The pull request is already merged.

Field Value
Head 7946c7cb53f59e3fc14d2b62ff197ed859845df0
Base at CI time a782c4c5 (2026-10-05, merge of #2396)
Run 37336745278
Report ci_run_report.html

GitHub reports 344 successful checks, 129 skipped, and 45 failures. The 45 failures are duplicate commit-status contexts of 22 jobs, plus the aggregate PR status: 14 CAS regression jobs, 5 stateless jobs, 1 stress job, and 2 Grype scans.

Category Count What
regression 0 —
unknown 0 —
pre-existing-flaky 20 jobs uniqApacheHLL suites, aarch64 attach and selects, 5 stateless jobs, arm ASan stress
infrastructure 2 jobs Grype keeper and alpine server
cascade parent TestFlows paths /aggregate functions, /alter/attach partition, /selects fail because a leaf under them failed

uniqApacheHLL (12 jobs)

All six CAS aggregate suites, both architectures, fail because the server returns UNKNOWN_FUNCTION / UNKNOWN_AGGREGATE_FUNCTION for uniqApacheHLL, uniqApacheHLLState, and uniqApacheHLLDistinct. Part 2 is the single leaf uniqApacheHLL_finalizeAggregation_Merge (Code: 63). Parts 1 and 3 fail the whole feature (Code: 46). Parent features and modules are cascade.

uniqApacheHLL is not in this PR and not in the CI base a782c4c5. git grep on both commits finds no AggregateFunctionUniqApacheHLL. The warning and the function files are also absent from the diff, which only adds bulkDeleteKeyLimit / batchDeleteKeyLimit. The function is on the later branch parent of the merge commit 5b7c9f50 (f4bc71ad), which is why #2483, based on that merge, passed these same suites. The regression suite runs the scenarios on any Antalya 26.6 build. #2437 measured the same leaves at about 37–40 fails in 47–50 branch runs.

Other CAS regression

  • aarch64 cas_alter_attach_3: 14 scenarios failed with Code: 210 (CAS write could not be committed, including gave up at the policy deadline) surfaced as Code: 341 from a mutation. Release cas_alter_attach_3 and both S3-cache attach-3 jobs passed. Same class as #2483 and #2351 (4 of 6 branch runs, 16 of 27 other PRs, measured 2026-09-24).
  • aarch64 cas_selects: concurrent FINAL scenarios (joins, SELECT LIMIT, SELECT LIMIT BY, SELECT PREWHERE) failed with content-addressed disk 'cas_disk' -- mount lease not held (Code: 210). Release selects and both S3-cache selects jobs passed. #2483 measured this signature at 4 of 5 branch suite runs and 28 of 32 other PR runs.

Stateless

Job Test What the log shows
amd_binary, cas storage 01164_detach_attach_partition_race Stderr warning CasWriteRetryLater: promote of detached/attaching_all_13_13_1 hit the unique-ref invariant. That string is already at CasPartWriteTxn.cpp:970 on the base. The diff does not touch it. amd_binary cas s3 storage passed, so this test passed there. 05008_cas_gc_snapshot_prune passed in the same job.
amd_tsan, cas s3, 1/2 03823_optimize_dry_run_projections, 04033_tpc_ds_q37 Stdout matches the expected result. The fail reason is stderr: ConnectionGroup: Too many active sessions in group Disk. Same warning #2483 recorded on 04033.
amd_tsan, cas s3, 2/2 04056_mutation_session_timezone_datetime_literals, 04338_async_insert_current_user, 02884_create_view_with_sql_security_option, 01164_detach_attach_partition_race Same ConnectionGroup stderr warning. Stdout for the mutation, async-insert, and view tests is the expected result.
amd_msan, cas s3, 1/3 01019_alter_materialized_view_consistent Reference expects 1, the run printed 0, and inconsistencies stayed 0. The same test passed on the release, ASan, TSan, and arm CAS stateless jobs in this run. Upstream: 1 fail in 273,303 runs over 60 days.
arm_asan_ubsan, azure 02354_vector_search_rescoring_distance_in_select_list Float32 distance 0.1875 where the reference expects 0.203125. Same two numbers as #2483, whose merge-base failed this test 3 of 3. Upstream: 0 fails in 274,857 runs. The diff does not touch vector search.

Sanitizer rows marked BROKEN in these jobs (Timeout! Processes left in process group, exit 159) matched the known sanitizer-timeout rule and did not fail the jobs on their own.

Stress and Grype

  • Stress test (arm_asan_ubsan): Hung check failed. fatal.log is empty. The server accepted 00001_select_1. The hung query is a cancelled AST-fuzzer mutation of 01730_distributed_group_by_no_merge_order_by_long (max_memory_usage 100MB, elapsed ~699s). The other stress jobs in this run passed, including arm_asan_ubsan with S3 and every amd_* CAS S3 stress job. #2483 failed this same job on a ready-probe timeout, a different mechanism.
  • Grype: alpine server image and keeper image. Base-image findings (zlib CVE-2026-85091 on the alpine server image; keeper also flags OpenSSL CVEs). The diff does not change image dependencies. infrastructure.

Coverage of the change

Unit tests passed under ASan, TSan, and MSan. 05008_cas_gc_snapshot_prune passed. CAS stress on S3 passed for amd debug, ASan, TSan, and MSan. Release and aarch64 CAS suites for engines, atomic insert, lightweight delete, and the other alter jobs passed.

The 60-day gh-data rates for this run were not re-queried. The CAS rates above are the measurements already recorded for #2437, #2351, and #2483 on the same error text. Stateless rates are from upstream default.checks plus the other CAS jobs in this run.

zvonand pushed a commit that referenced this pull request Oct 8, 2026
…ete-key-limit

CAS GC: cut batch deletes to the storage's batch-delete limit

Source-PR: #2484 (#2484)
@alsugiliazova

Copy link
Copy Markdown
Member

PR #2484 Distributed-Systems Audit

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

Summary

No confirmed distributed-systems defects in the reviewed scope.

A write-once cohort is still licensed once, then sent as consecutive requests of at most min(cas_gc_bulk_delete_chunk_keys, storage batch limit) keys, clamped to 1..1000. A repeated delete of a key that is already gone succeeds. A deposed leader that finishes the cohort names only keys already in that cohort, and those keys carry the life or the manifest build. A store that rejects DeleteObjects still falls back to one admitted delete per key.

Model assumptions

Actors The GC leader, a successor leader on another node, the ref catalog, and the object store
Channels Batch DELETE (DeleteObjects or one key), catalog GET, gc/state read, mount-fence admission on each request
Authoritative state The catalog cut and the GC lease license a cohort. The object store holds the bytes. A _log, _snap, or manifest body is write-once at a key that includes the life incarnation or the manifest build
Failure model assumed by the code The store can reject a whole batch, fail some keys inside a 200 reply, or not implement DeleteObjects. The leader can lose the mount fence or the GC lease between requests. Delivery is at-least-once. Retry::standard() is bounded
Safe failure criterion A missed delete leaks storage until a later round or the orphan-manifest sweep. It must not delete another life's objects, and a retry must not fail because the key is already gone

Evidence

  • Gc::bulkDeleteChunkKeys (CasGc.cpp) takes gc_bulk_delete_chunk_keys and poolBackendPtr()->bulkDeleteKeyLimit(), then clamps to [1, kBulkDeleteMaxKeys] (kBulkDeleteMaxKeys is 1000 in CasRequests.h). An empty request cannot stall the loop in removeCohortWriteOnce.
  • S3ObjectStorage::batchDeleteKeyLimit returns 1 once DeleteObjects is known unsupported, otherwise max(1, objects_chunk_size_to_delete). IObjectStorage::batchDeleteKeyLimit defaults to 1, so a storage without an override is one key per request.
  • ObjectStorageBackend::bulkDeleteKeyLimit forwards the native storage's limit. Emulated and in-memory backends return 1000, matching the one-by-one loop they already run under their own lock. InstrumentedBackend and ThrottlingBackend forward the inner limit. The default on Backend is 1, and the comment there says a decorator that does not forward would split every batch into single keys.
  • Phase 15 still builds a cohort of gc_bulk_delete_chunk_keys, then removeCohortWriteOnce cuts that cohort. Recording of ManifestDelete still happens only after the whole call returns. A throw from a later sub-request leaves the already-deleted prefix unrecorded and the rest of the cohort for the orphan-manifest sweep, which is the recovery the phase comment already names for a failed request.
  • Phase 17 calls authorityHolds once per cohort, then the same cut. authorityHolds re-reads the catalog cut and gc/state (CasGc.cpp). Each sub-request still enters CasOperation::removeManyWriteOnce, whose readLoop calls gate. gate (CasRequests.cpp) refuses a lost mount fence before the HTTP call. The keys are writeOnceRefLogKey / writeOnceRefSnapshotKey, built from namespaceStreamPrefix, which is cas/ns/stream/<incarnation>/.
  • removeChunkWriteOnceOrOneByOne still catches only NOT_IMPLEMENTED and then deletes one key per admission. Any other error leaves the retry policy. NoSuchKey inside a 200 DeleteResult is still skipped (S3ObjectStorage::removeObjectsIfExistImpl).
  • The new S3Exception name is the store's GetExceptionName() or the per-key code. isRefreshableCredentialError (CasRequests.cpp) matches ExpiredToken, AccessDenied, and the other names it already listed. A refreshed retry deletes the same write-once keys again.
  • On this head the namespace janitor still exact-deletes one listed key (CasNamespaceJanitor.cpp, op.remove). It does not build a batch, so this limit does not apply to it.
  • Unit tests: a limit of 3 sends 8 manifest bodies as 3+3+2 (CASGCManifestBulkDelete.StorageLimitCutsTheCohortIntoRequestsOfAtMostThatManyKeys); ref cleanup under a limit of 3 issues more calls than an unlimited backend and the same number of gc/state reads (CASRefGc.RefObjectCleanupCutsRequestsToTheStorageLimitWithoutExtraAuthorityReads); CASS3BatchDelete covers the limit, a whole-request error name, and a per-key error name in a 200 reply.

Coverage summary

Fault class Status Notes
Network / transport Reviewed Each sub-request uses Retry::standard() inside removeManyWriteOnce. NOT_IMPLEMENTED falls back to one key. Other errors propagate. The new exception name is what the credential check already matched by string.
Partition / availability Reviewed Mount-fence loss fails the next sub-request through gate. GC leadership is sampled once per cohort in phase 17, as it was once per chunk before. A deposed leader can finish only the keys already in the cohort.
Timing / ordering Reviewed No new wall-clock dependence. Cohort order is the vector order. Sub-requests are sequential, so a later request starts only after the earlier one has returned.
Partial / gray failure Reviewed A 200 reply that still lists a real per-key error throws after skipping NoSuchKey. The retry resends the sub-request. Keys the store already removed are absence, which this path treats as success.
Idempotency & duplicates Reviewed Write-once delete of an absent key succeeds. Two leaders deleting the same life-qualified key converge. CasOperation::removeManyWriteOnce still rejects a chunk above 1000.
Backpressure & resource limits Reviewed One request is at most 1000 keys, and at most the storage limit. The phase samples the limit once; after the store is learned to lack DeleteObjects, later chunks throw NOT_IMPLEMENTED locally and fall back per key. The round thread waits for each request.
Recovery & rollback Reviewed Crash or throw after a successful sub-request leaves those keys gone and the rest of a manifest cohort to the orphan sweep. Ref cleanup replans the same remaining candidates from durable state next round.
Configuration & version skew Reviewed No object format change. An old leader may still send 1000 keys; a new leader sends the storage limit. Both deletes are safe to repeat. objects_chunk_size_to_delete of 0 becomes a limit of 1.
Security / trust N/A No new peer RPC or credential path. Deletes use the pool's existing object-store credentials.
Observability & diagnosability Reviewed Phase metric requests counts sub-requests, so a cut cohort shows up as more requests. The effective limit itself is not a metric. See Operational Readiness.

Limits

Static review of head 7946c7cb53f5. No fault injection was run against two live servers and a shared bucket.

The accepted tradeoffs, not scored as defects because a missed delete leaks storage and a repeated delete is the same key:

  • Phase 17 does not re-read gc/state between sub-requests of one cohort. The mount fence is re-checked on every request. The cohort was chosen under the catalog cut, and every key in it is incarnation-qualified or build-qualified.
  • bulkDeleteChunkKeys() is read once per phase. A store that answers the first multi-key call with "no DeleteObjects" does not shrink the later chunks in that same phase; those chunks take the existing one-key fallback.
  • The namespace janitor on this commit is unchanged and deletes one key at a time. Batching that janitor, and applying this limit there, is the follow-up described on the PR.

Operational Readiness

Scope: GC phases 15 and 17 after PR #2484, head 7946c7cb53f5
Persona: Operator / DBA
Environment: One or more ClickHouse nodes sharing an object-storage CAS pool. GC runs on the leader.

Primary flows (ranked by expected usage)

  1. Leave cas_gc_bulk_delete_chunk_keys at 1000. GC deletes owner-removed manifest bodies and covered ref logs and snapshots in batches the store accepts.
  2. Set objects_chunk_size_to_delete below 1000 on the S3 disk, or point CAS at a store with no DeleteObjects.
  3. Read system.cas_gc_log for manifest_deletes and ref_object_cleanup when a round looks slow or a delete error appears.

Issues

Flow 2: a store with a small batch limit

2.1 The effective batch size is not in the GC log
  • Symptom: manifest_deletes and ref_object_cleanup show more requests than cas_gc_bulk_delete_chunk_keys would suggest. The row does not say which limit won (Gc::bulkDeleteChunkKeys in CasGc.cpp).
  • Trigger: objects_chunk_size_to_delete is lower than cas_gc_bulk_delete_chunk_keys, or the store has already been learned to lack DeleteObjects.
  • Signal quality: confusing
  • Severity: low
  • Recovery: self-service. Compare the two settings. A limit of 1 means one HTTP delete per key, so a large cohort takes longer and the next GC round waits behind gc_round_mutex.

Readiness verdict

  • Flow 1 (default batch size) in isolation: ready. The storage limit is at least the GC chunk, so the cohort stays one request.
  • Flow 2 (small store limit) in isolation: ready with caveats, because 2.1.
  • Flow 3 (cas_gc_log) in isolation: ready with caveats, because 2.1.
  • Overall in production: ready with caveats, because 2.1. A failed sub-request leaks the rest of that manifest cohort until the orphan sweep, and ref cleanup replans those keys next round. Neither path deletes a different life.

Operator guidance

  • When requests is much larger than the cohort divided by cas_gc_bulk_delete_chunk_keys, check objects_chunk_size_to_delete on the disk. That setting now caps GC batches (2.1).
  • A store that cannot do DeleteObjects is correct at one key per request. Expect the first multi-key attempt in a phase to fail with NOT_IMPLEMENTED, then one delete per key.
  • A manifest_deletes error after some requests still leaves the unsent tail for the orphan-manifest sweep. ref_object_cleanup computes the same remaining keys on the next round.

Recommended fixes

  • Record the effective bulkDeleteChunkKeys() on the manifest_deletes and ref_object_cleanup rows so a raised requests count has a cause on the same row (2.1).

@alsugiliazova alsugiliazova added the verified Approved for release label Oct 8, 2026
zvonand pushed a commit that referenced this pull request Oct 8, 2026
…ete-key-limit

CAS GC: cut batch deletes to the storage's batch-delete limit

Source-PR: #2484 (#2484)
zvonand pushed a commit that referenced this pull request Oct 10, 2026
…ete-key-limit

CAS GC: cut batch deletes to the storage's batch-delete limit

Source-PR: #2484 (#2484)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.6 antalya-26.6.8.20001 CAS port-antalya PRs to be ported to all new Antalya releases verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants