CAS improvements - #2300
Conversation
…blication Two defects surfaced by the `content_addressed_garbage_collection_log` scenario cards for issue #2233. Any `S3_ERROR` timeout during a GC round was recorded as an indistinguishable `Failed` outcome with a free-text error, and a failed round zeroed out the real counters and cleared `i_am_leader`, suppressing the heartbeat and provoking leadership ping-pong on a flaky backend. Transient error codes (`S3_ERROR`, `NETWORK_ERROR`, `ABORTED`, timeouts, `MEMORY_LIMIT_EXCEEDED`) now produce an `Aborted` outcome while keeping leadership, and `system.cas_gc_log` gains an `error_code` column alongside the `Aborted` outcome. Separately, the emulated blob-publication path materialized the whole blob body in memory (about 1 GiB for a 512 MiB blob) under a global mutex; it now streams the body instead. Related: #2233 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…_ERROR A fetch-by-relink that loses the offer-to-confirm race -- the source's ref moved (a merge, a mutation, an outdated-part drop) between the offer and the confirm -- is a designed, fail-closed outcome: the receiver abandons the relink and the replication queue retries, re-selecting the source and the covering part. It was thrown as `NETWORK_ERROR`, which misdescribes it three ways: - both queue executors (`processQueueEntry`, `ReplicatedMergeTreeQueue`-driven `ReplicatedMergeMutateTaskBase`) treat `NETWORK_ERROR` as an unclassified failure, so every refusal printed an Error-level log line with a full stack trace; issue #2219 records a multi-hour false triage chasing a network fault that was never there (up to 53% of relink proofs refuse under small-part load); - stateless `part_log` hygiene checks tolerate the fetch-transient class under the code upstream fetches use for it, `NO_REPLICA_HAS_PART` (e.g. `02265_column_ttl` whitelists exactly that code), so a refusal landing in `part_log` as `NETWORK_ERROR` fails them -- this is what broke `02265_column_ttl` in the CAS lanes on PR #2159 (13/14 reruns under `prefer_fetch_merged_part_size_threshold=1`); - the label suggests retrying the transport, while the one recovery that is unsound here is a byte re-request to the same source. Both relink retry-later throw sites (taxonomy row 3, the confirm refusal, and row 5b, the unresolved promote) now throw `NO_REPLICA_HAS_PART`. The queue behavior is unchanged -- the exception is stored on the entry, backed off, and re-executed -- but both executors demote it to INFO with no stack trace. Unlike `ABORTED` (the other demoted code), it keeps `need_to_save_exception`, so a refusal storm stays visible in `system.replication_queue`; `ABORTED`'s save-nothing shape is the known pathology where a refusal loop runs invisibly with no backoff accounting. `test_confirm_refuses_when_source_dropped_in_window` now pins the classification: the refusal must not appear at Error level, must appear at Information level, and must reach `part_log` only as `NO_REPLICA_HAS_PART`. No message text changed; no generic queue code changed. Closes: #2219 Related: #2159 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…eys renamed yet) Behavior-preserving groundwork for an atomic rename of the CAS wire-format JSON keys from abstract letters (`t`, `k`, `s`, ...) to semantic names (`kind`, `outcome`, `state`, ...), landed as its own phase so the rename itself is a single reviewable diff. Adds `WireKey` and per-encoding field write helpers, and `EnumWireTable` — a table pairing each enum value with its wire word, proven complete against the enum by a set-equality coverage check with a failing witness for every member. `kMinBlobHeaderLen` gets one compile-time owner instead of several hand-kept constants. `TokenType`, `ObjectKind`, and `BlobHashAlgo` move onto `EnumWireTable`, and the blob-meta, pool-meta, GC state/heartbeat/ maintenance, server-root, blob-envelope, ref-log/ref-ckpt/ref-snapshot/ ref-catalog, run, fold-seal, and gc-outcomes codecs are all migrated onto the carriers — every one of them still writing its existing wire spelling. `RunMarker` becomes a typed enum, and the format test battery is closed out with a set-equality check over the codec registry. No wire-format bytes change in this phase; the follow-up phase (next commit) performs the actual key cut. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
The actual key rename, on top of the phase-1 carrier infrastructure. The
format generation history is first reset to a `{1, 1}` baseline, since CAS
has no released, persisted data yet and pre-release generations exist only
to prove the evolution machinery.
Every CAS wire format switches its JSON keys from single-letter/abbreviated
spellings to descriptive names in one pass: the shared `BlobRef`/`Token`/
`ManifestRef`/binding fields, `cas_blob_meta`, `cas_pool_meta` (`algos_used`
becomes a JSON word array instead of a bitmask), GC state/heartbeat/
maintenance state, the server-root record (`MountLease::min_active` becomes
`min_active_build_sequence`), `cas_ref_ckpt`, `cas_ref_log` (the seal link
becomes `!prev_epoch`/`!prev_seq`), `cas_ref_snapshot`, `cas_part_manifest`,
`cas_run`, `cas_gc_outcomes` (`kind`/`outcome`), the fold-seal record and its
`CoverageClass` words, the blob descriptor (with its 239-byte worst case
proved at compile time against the 240-byte floor), and `cas_ref_catalog`.
Golden tests are re-pinned to the new bytes throughout.
Token-group requiredness is unified through `TokenFields::build`: an outcome
missing its token now fails closed instead of serializing a partial group.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…enchmark Small correctness and review follow-ups to the wire-key cut: the part manifest names its namespace field the way every other object does, the algorithm set is read from the proven `EnumWireTable` instead of two independent hand-kept lists, the GC lease and heartbeat keep their separate owner spellings (documented, not merged), and the wire-format word writer gets the contract it was always assumed to have. Extends the `benchmark_cas_ref_protocol` harness to cover every format and direction the wire-keys design measures, plus a review-round fix to that harness. Also fixes `c++expr`: the generated work function needs internal linkage, without which ClickHouse-mode compilation did not work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…speedup Full before/after measurement of the wire-key cut found decode of four of the five formats barely slower, with `cas_fold_seal` the exception (its short strings make the longer keys dominate). Chasing that, the JSON object reader and the per-format row reader are now reused across a stream's rows instead of rebuilt for each one, cutting decode time 57-81% (53-79% net of the key-length cost). A separate copy-free string-read attempt was measured at a 6-7% regression on `cas_ref_catalog` and is not included here. Also lets the full stateless test suite run locally: `functional_tests.py` turns on verbose output for the dataset-attach step (so a `DNS_ERROR` that only fires outside CI doesn't get swallowed and misread as a Kafka failure downstream) and extends the "skip stateful tests when running locally" guard to a local run with no test selector at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
`ALTER TABLE ... EXPORT PARTITION` was refused with `SUPPORT_IS_DISABLED` on a content-addressed disk because it is absent from the partition-command allowlist in `MergeTreeData`. The rejection it fell into says the command "clones parts file-by-file with no transaction, which would corrupt the clone", and that reason does not describe exporting. `ExportPartTask` reads the source part through `MergeTreeSequentialSource` (`MergeTreeSequentialSourceType::Export`) under `readLockParts` and writes rows into the destination through a `SinkToStorage` on an ordinary query pipeline. Nothing is hard-linked or copied on the source disk; the command's own bookkeeping is in ZooKeeper. So the allowlist was rejecting it by omission rather than by an argument that applies to it, which the code around it already half concedes: `EXPORT_PARTITION` is listed among the commands permitted to target `PARTITION ALL` a few lines above. Verified end to end rather than by inspection, on a server built from this change: a `ReplicatedMergeTree` source on a CAS disk holding (1,2020), (2,2020), (3,2021), exported to an `IcebergLocal` destination. `EXPORT PARTITION ID '2020'` succeeds and the destination holds exactly (1,2020) and (2,2020) — the right partition, and the 2021 row correctly absent. Two limitations surfaced on the way and are NOT addressed here, because neither is about CAS. Export is implemented only for `ReplicatedMergeTree`: a plain `MergeTree` source now returns `Code: 48 NOT_IMPLEMENTED` instead of the CAS refusal, so the reproduction in the report — which uses a plain MergeTree — will still fail, just for its real reason. And the operation remains behind the server setting `allow_experimental_export_merge_tree_partition`. Closes: #2291 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KiHKrvEVy8u4nA1A8qYFUY Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
… answer On a generation-dialect (GCS) mount, `ObjectStorageBackend::checkPoolPreconditions` refused to mount both when the bucket was verified to have versioning enabled and when the probe simply couldn't get an answer. The first live run against a real GCS bucket hit the second case: the service account lacked `storage.buckets.get`, `GetBucketVersioning` returned 403, and every writable CAS mount on that bucket failed with `NOT_IMPLEMENTED` at server start — a missing IAM grant turned into a hard outage, even though an unreadable bucket configuration is not evidence the bucket is actually versioned. The probe now logs a warning naming what it couldn't verify and how to fix it (grant `storage.buckets.get`, or confirm by hand) and lets the mount proceed. A bucket confirmed versioned still refuses, because a token-exact `DELETE` there archives a noncurrent generation instead of reclaiming storage. That first credentialed run against Google (HMAC groups) also surfaced three test-suite assumptions the real service doesn't meet (`system.cas_log.token` isn't always a numeric generation for build-lifecycle rows; a second `COUNT()` over a Parquet object is answered from the per-file row-count cache, not the Parquet metadata cache; a disk over an absent bucket refuses at `CREATE TABLE`, not the first `INSERT`) and one open question — whether process-wide `system.events` deltas can be attributed to one statement when a mount-lease renewal shares the same counters. The suite now attributes every counter it asserts through `system.query_log.ProfileEvents` instead. Also documents GCS's request-rate limits in the CAS bucket requirements. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
… whole namespace Same-pool replication transfers only a part's manifest: the receiver publishes its own ref over the sender's blobs, then asks the sender a read-only question — "do you still hold exactly this manifest for this part?" — and promotes only on `Yes`. Rule 3 of `CasRefLedger::confirmExactRef` answered `Unknown` whenever ANY mutation of the same namespace was queued, in flight, or awaiting its checkpoint frontier, not just a mutation of the asked-about ref. On the live GCS stand, two replicas answered each other `Unknown` almost every time for forty minutes: every replica is also a receiver, and each failed fetch appends two records to its own lane (a precommit, then its removal on abort), so under load neither side ever observed the other's lane quiet. Both replication queues wedged at 1.5-1.7k entries, the replicas diverged to 123k against 166k rows, and the soak died on `SYSTEM SYNC REPLICA`. Nothing was lost — once one side stopped fetching, the other drained in two minutes — but the lane-wide refusal made every sustained-write workload look like data loss in progress. On RustFS in a LAN the window closing this fast never showed the defect; GCS limits checkpoint publication to about one mutation per second per object, so the window is long enough to matter. Rule 3 now refuses only when the asked-about ref itself has a queued or in-flight mutation, via `RefTableRuntime::carved` mirroring the tenure's carved items and validating a ref-scoped item's ops against its `MutationScope` before durability. Covered by a two-node liveness case against a fake GCS with delayed `_ckpt` writes (`test_cas_gcs_relink_liveness`), and every confirm refusal is now attributed and counted rather than silent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
A manifest id names one content, ever: `stageManifest` mints it from `(writer_epoch, build_sequence, next_manifest_ordinal++)` — all three either a durable counter bumped by a conditional write or strictly increasing per process — and writes the body exactly once. The only other mutations of a manifest are exact-token deletes (writer cleanup, GC's owner-removed cleanup, the orphan sweep). So the token carried in `ManifestCacheKey` distinguished nothing, and the `HEAD` that supplied it (`CasManifestReader::readManifestShared`) was a per-read check of a GC-side invariant, not of the cached content's validity — it cost one serial round trip per uncached or `ForceFresh` access and could only ever detect a protocol violation (something deleting a manifest the ref graph still names), never serve wrong bytes if removed, since id-to-content is a function. The cache now keys by `ManifestId` alone: no `HEAD` on a hit, exactly one `GET` on a miss. Detection of a dangling reference moves from "the next read" to "the first uncached read, or fsck". The `part_folder_validate` setting, which existed only to pace that now-removed `HEAD`, is retired along with it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Six review rounds across two earlier designs found nine classes of defect in how CAS
handled its conditional-write tokens, eight of them tracing to one root: `Token` was
`struct { String value; TokenType type; }`, anyone could construct one from anything, and
the backend accepted whatever it was handed. Concretely this let an empty value pass as a
token (and on S3/Azure an empty `If-Match` is *omitted*, so a fenced write silently becomes
an unconditional overwrite), let a write commit against a token that was really the result
of a later, unrelated `HEAD`, and let `TokenMismatch` — documented as remote evidence that
another incarnation is current — be returned for what was actually a local refusal, with GC
acting on it and mislabelling live blobs `Replaced`. Every earlier revision patched one
symptom at one call site; the next review round found the same root through a different
one. A second, independent waste rode along: `Backend::get` always issued both a `HEAD` and
a `GET`, though a `GET` already returns everything a `HEAD` does plus the body — doubling
the request cost of every control-object read.
This introduces the replacement, starting with its core (the migration of every CAS
subsystem onto it is the next commit): `Backend` becomes a string-in/string-out transport
callable only through a `TransportAccess` key; `Incarnation` replaces the free-form `Token`
as a type that can only be minted by the backend from an actual store response;
`CasRequests` owns a backend and a `Fence`, and `admit()`/`resume(generation)` hand out a
`CasOperation` carrying the admitted generation and an optional liveness predicate. Every
verb on that operation (`read`, `head`, `list`, `remove`, `publish`, `create`, `replace`,
`readModifyWrite`, ...) takes a `Retry` policy; the engine re-checks admission before every
attempt, before every sleep, and once more after a proven commit, settles every conflict and
ambiguity by one exact read, and reports one of `Committed | Declined | Conflict | Refused |
GaveUp` — never an exception for an ordinary lost race. An upstream slice under `src/IO`
and `S3ObjectStorage` adds a `SingleAttempt` request mode so a marked `GET` answers with the
same incarnation identity a `HEAD` does (closing the two-request cost) and a reissue that
gets back a different ETag is treated as body drift, not silently accepted. The old
controller stays in place during the migration; the next commits delete it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Follows the previous commit's engine introduction by moving every production caller off the old ad-hoc backend controller and onto `CasOperation`: pool bootstrap (sentinel probe, capability probe, plain objects, pool meta, manifest and ref-protocol readers), GC (maintenance state, namespace janitor, decommission, the GC core's lease/heartbeat/commits/ folds/persisted redelete), part-write (blob meta, the part-write transaction, create-first marker reconciliation), the ref lane (catalog and checkpoint publisher, namespace creation lifecycle, the resumed-operations arms, the catalog erase loop), and mount (renew, farewell, claim, epoch allocation, the heartbeat floor, remount re-anchoring). `PersistedIncarnation` replaces the ad-hoc token in the wire vocabulary, the record stream, the outcomes and the condemned rows. Each subsystem's move keeps its behavior but inherits the engine's guarantees for free: every write is admitted under a fence and re-checked before each attempt/sleep/commit, every conflict is settled by one exact read instead of an assumed outcome, and a credential refresh mid-attempt is never mistaken for a landed write. Along the way this fixes real bugs the engine surfaces mechanically rather than by inspection — e.g. two double-counting fault-injection doubles in the GC maintenance-state path, and several sites that treated an unobserved conflict as corruption instead of "vanished or a competing leader also wrote". The bulk of the diff is the matching migration of every test double (the `cp4` series) off the legacy backend overrides and onto the primitives the production code now actually calls — direct-Backend doubles for the primitives, virtualized clocks for every retry/backoff path that used to sleep for real, and fault injection that latches instead of pinning `max_attempts`, so a shut gate can no longer hang the test binary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
… the engine, delete the old controller
Two closing passes over the request-engine migration. First, a naming decision: `Incarnation`
becomes `Etag` (`PersistedIncarnation` -> `PersistedEtag`, `CasIncarnation.{h,cpp}` ->
`CasEtag.{h,cpp}`) and `TokenType` folds into a `Dialect` alias, because the class was
carrying its wire field's name rather than its actual role — an ETag on S3-compatible
stores, a generation on GCS's JSON dialect, a minted sequence value on the emulated
backends. This does not touch the blob envelope's `incarnation_tag` or the catalog's
incarnation namespace, which are unrelated concepts the rename exists to stop colliding
with.
Second, the entire gtest suite (~120 files) moves off the legacy backend overrides and onto
`CasOperation`/`CasRequests`, naming the etag and the listed key the way production code now
does. With every test migrated, the old controller and its 1500-line test file
(`gtest_cas_request_control.cpp`) are deleted outright — this was the last thing keeping it
alive. `MountLeaseKeeper` is renamed to `MountLeaseRenewer` in the same pass (it renews a
lease; it is not ClickHouse's Keeper, and the old name kept reading as if it were).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…gration Small correctness and hardening fixes found while the engine migration was under review: a read now ends only on an authoritative absence, not on every unretryable code; a raced `claimMount` reports the occupant it actually observed instead of its own proposal; an absent-key read through the request engine is no longer logged as an error (it's an ordinary outcome the engine already models); a hand-written retry loop now freezes one deadline and shares it across every call it makes, instead of re-deriving it per call; and an unobserved conflict is named for what it actually is — a vanish or a competing leader, never assumed corruption. The bulk of this is test hardening that follows from the engine actually enforcing pacing and admission where the old ad-hoc calls didn't: transport-fault doubles now inject `Poco::TimeoutException` (what production code actually throws) instead of `std::runtime_error`; several tests that asserted a schedule the engine never promised, or counted requests instead of asserting an outcome, are corrected; retry/backoff-dependent tests get their own virtual clock so they assert the engine actually reissued, rather than timing a real sleep; and the throttling coverage gate gains both a unit and an integration leg. Two properties orphaned by the old controller's test-file deletion (previous commit) are restored under the new API. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…gc_read_concurrency) The fold's `fold_ref_intake` and `fold_reduce` phases issue their checkpoint, walk-position, manifest-edge and zero-in-degree-HEAD reads one at a time, in the round's own decision order — a live-GCS soak measured `fold_ref_intake` at 2303 s of a 4352 s phase wall (53%), and a separate finding recorded one fold round holding the GC lease for hours on a real bucket, still unfinished after 97 minutes. `GcReadAhead` sits in front of the fold's one admitted `CasOperation`: callers hint keys the sequential walk will need next, workers fetch them on a bounded pool under the same admitted generation, and the walk takes results at exactly the sites and in exactly the order it reads today — no decision, decode, counter or event moves off the round thread. A key nobody hinted is still read inline. Concurrency 1 issues no hints and is byte-for-byte today's behavior; the new `cas_gc_read_concurrency` setting is plumbed like `gc_meta_pool_size` and refused at 0 like `gc_shards`, with three `ProfileEvent`s for hits, misses and wasted results. Measured against a fixed per-request latency: `fold_ref_intake` 2.4x, `fold_reduce` 1.2x, the round overall 1.65x. Intake's speedup stops there because the round issues ref-log and manifest `GET`s one to one and a manifest key is only known once its log is decoded — that chain, and the graduation gate's inline meta re-check, are recorded as follow-up items. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Fetch-by-relink existed but was opportunistic: the receiver advertised ONE guessed pool identity before it knew where the sender actually kept the part (the caller's `dest_disk` if content-addressed, else the first content-addressed disk of the table's storage policy), then reserved the target disk the ordinary way — the TTL move rule's destination, `balancedReservation`, or the first volume with space — and accepted the relink offer only when the reservation happened to land on a disk of the advertised pool. Otherwise it re-requested the bytes. So a part already in the shared pool moved as bytes whenever the policy's placement disagreed with the guess: a tiered policy whose local volume comes first, a TTL rule naming the local tier for a fresh part, a policy holding two pools with the sender's in second place. The relink is the whole point of a shared pool — a fetch should move no bytes — and the storage policy could veto it by accident. The receiver now advertises every pool of its storage policy (any volume; a disk configured on the server but absent from the policy is not a candidate, since a part on it wouldn't load at startup), the sender names the one it matched, and the part lands on that pool's disk ahead of volume order, JBOD balancing and TTL move rules — the mover carries it to a TTL destination afterwards, the same way `perform_ttl_move_on_insert=0` already places first and moves later. A caller-supplied `dest_disk` (zero-copy `MOVE`) stays authoritative and untouched; a content-addressed disk never enters that path (`supportZeroCopyReplication()` is false for CAS). A read-only or broken disk on the right pool is not a candidate — nothing can publish a ref there. The offered pool must itself be an advertised pool (not matched by disk name), and the confirm's gate 0 compares mounts rather than disk names, closing a second-order gap the first pass left. Non-live pool disk = fail-close: a disk whose mount isn't live is left out of both the advertise and the placement, never guessed at. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
`ContentAddressedMetadataStorage::shutdown` contained two unbounded waits on the GC round: a `std::lock_guard` on the same mutex a synchronous round (`SYSTEM CAS GC`, `GC REBUILD`) holds for its whole duration, and `CasGcScheduler::stop`'s join, which the scheduler loop only even looks at at the top of its wait — a round already in flight never observes it, and a comment in the loop recorded an accepted extra full round if `stop` lands while the loop is blocked behind a manual round. A round has no wall-clock budget at all: `GcRoundWorkBudget` caps destructive work, not time, and against a slow bucket the wall clock is whatever the bucket makes it. Nothing in this wait protects durable state — the round is one-pass, committed by a single `gc/state` conditional write at the end, so an interrupted round is a crash the protocol already survives — the wait existed purely so no thread would touch a freed object. Shutdown and the storage destructor now arm the pool's teardown flag before the lock or join they would otherwise wait behind. The open request plane carries that flag as its fence, so a round in flight is refused at its next request, its next retry sleep, or its next streamed refill — the check lives at the request because a phase is long from making thousands of requests, not from making one long one, and `CasOperation` already re-checks admission before every attempt and sleep. Every join and every object's ownership stay unchanged: the join became short, not optional. A round cut this way is recorded `Stopped` rather than `Aborted`. Decommission is deliberately not armed: an already-latched self-remount completes one more step whose pool-identity probe runs on the open plane, and no arm point early enough to bound the GC join leaves that step intact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…bulk delete) A 15-minute real-GCS soak measured a sweep round costing 300-617 s per phase, all from per-object request loops on keys that are write-once by construction — an object whose only writer mints it once and whose only other mutations are exact-token deletes, so nothing about it needs re-reading once known. Three loops dominated: `fold_reduce`'s `GET` volume (2870-3400 per round) turned out to be the sweep's mount-floor probing, not manifest bodies — `floorForNamespace` reads the mount key of every `/`-prefix of a namespace for every listed manifest, though the floor is one value per server root; `manifest_deletes` cost 617 s for 3250 sequential conditional deletes at ~190 ms each; and `ref_object_cleanup` cost 199-204 s for 512-516 keys at four requests each. The fix cuts each loop to what the write-once property actually allows: one mount-floor read per namespace per sweep page (memoized), manifest bodies read only for nominated orphans and through the existing read-ahead instead of on every listed key, and a new write-once bulk-delete verb (`removeManyWriteOnce`, backed by `DeleteObjects` where the store has it) replacing the sequential per-key deletes for owner-removed manifests and for ref-object cleanup, which now revalidates its cohorts before batching them. None of this changes what gets deleted or when a namespace or manifest is judged eligible — only how many requests that judgment costs. Measured on real GCS: `fold_reduce` 300-380 s -> 2-5 s; `manifest_deletes` 617 s -> 2 s on a 1506-key round; `ref_object_cleanup` 204 s -> under 1 s. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…own writes to a hot control object Every `CREATE TABLE`/`DROP TABLE` on a content-addressed disk mutates one pool-wide object, `cas/ref_catalog`, through a conditional write. Measured on ten parallel stateless jobs: `DROP TABLE` p50 2.4 s, p90 11.9 s, max 34.7 s; 113 `PreconditionFailed` in 80 s from 53 threads; the losing writer alone racking up 35 attempts with gaps growing to the 5 s cap; one `DROP` losing eight races in a row for 15.4 s. `CasRefCatalog::casUpdateImpl` starts every write with a `GET` and paces a lost race with `Retry::backoff`, a schedule shared with transport faults — a writer that has lost several races sleeps for seconds while a fresh one starts at zero, so the oldest loser is the least likely to win next. Worse, every writer in one process races every other writer in the *same* process: compare-and-swap is only needed against other servers, so every intra-process race is pure waste, each costing a `GET`, a refused `PUT`, a resolve `GET` and a sleep. `CasHotKeys` sits above the request engine as one FIFO ticket per pool and key: writers to the same hot object queue instead of racing, their conditional writes are combined into one physical attempt where safe (as-if-serial semantics, a `Conflict` cascade on a lost race so combined members see the answer a serial retry would have given them), and a last-known- object cache lets a lane holder skip the leading `GET` under one rule. Losing a race against *another server* still paces with a flat jitter, not the transport-fault backoff. The GC erase over `ref_catalog` (`deleteCompletedRemovingAtSnapshot`) becomes the lane's first caller, and the pool owns the lane. This is phase A only — combining, spacing, the clamp and moving the GC erase itself onto the lane in full are follow-on work; the design and its 34 review revisions are recorded separately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…ed locals Several gtest fixtures declared a test-only backend hook or fault clock as a local, captured other locals in it by reference, and then let the store (declared before the hook) call the hook again during its own teardown — after the captured locals had already gone out of scope. Under ASan this is a use-after-scope: the store's destructor writes a "farewell" record that can invoke a still-armed hook whose captured references are already dead. Fixed by declaring the test clock/hook before the store that keeps calling it (two transient-round tests, the straggler-epoch test), and by clearing the checkpoint-advance recovery test's backend hook before the locals it captures die. A separate scripted S3 client fix allocates its response body with `Aws::New`, matching how the SDK actually frees it, instead of a mismatched allocator. Also corrects two suites that had started asserting a schedule the engine never promised. Also: the stateless CAS lanes now run the GC scheduler every 20 s instead of every 5 s, matching the interval those tests actually need. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
`S3ObjectStorage::getSingleAttemptClient` (`SingleAttemptRetryStrategy`, `max_retries=0`) and the write path carrying `WriteSettings::object_storage_retry_profile == SingleAttempt` belong to the CAS control plane's conditional writes: a failed attempt there is not the final answer — `CasOperation::writeLoop` resolves the outcome by a read and reissues — yet two upstream sites logged it at Error as if it were terminal: `Client`'s network-error handler and the non-412 `S3Exception` site in `WriteBufferFromS3`. Both now log at Debug when the client carries `SingleAttemptRetryStrategy` or the write carries the `SingleAttempt` profile; an ordinary client configured with zero retries by a user setting (no outer loop resolving it) keeps logging at Error, since for that caller the failure really is final. The neighbouring 412 (`isPreconditionFailedError`) branch drops from Info to Debug for the same reason: a conditional write losing its precondition is the caller's expected answer, not an operator-facing event. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi
… gtests `SnapshotPublisherLatchedAcrossChunks` had two independent, reachable races. With `snapshot_log_count_threshold` at 0 (only reachable in this test), `precommitAdd`'s post-commit trigger dispatches a background publisher whose capture can still be in flight when `promote`, moments later, becomes lane leader and moves the lane to `Writing` — a lost race then backs the publisher off, and this pool's frozen `boot_ms_fn` never advances past that backoff deadline, poisoning every later dispatch on the namespace for the rest of the test. Fixed by driving `precommitAdd`/`promote` directly instead of through the shared `publishEmptyPart` helper, draining and explicitly publishing between the two commits. Separately, the carve hook gated the leader on the publisher reaching its blocked `PUT`, but under contention the dispatch's own scheduling delay could outlast that wait's bound, letting a leader released by timeout (not by the capture it meant to prove) start chunk 2 before the publisher captured — fixed by gating on the publisher's capture instead, which is causally prior to the `PUT`. Verified with 20 isolated `gtest_repeat` iterations and two full `CAS*` gates (2437/2437 each), reproduced only under CPU contention after isolated repeats alone did not reproduce it. Separately: a fatal `ASSERT_*` between launching an `AppendCaller`/`Caller` thread and its explicit `join()` left `TestBody` with the thread still joinable, and `std::thread::~thread()` on a joinable thread calls `std::terminate`, aborting the whole `unit_tests_dbms` binary and discarding every test scheduled after it. Both structs now join in their destructor if still joinable, so a failed assertion costs one test instead of the whole gate. Also: `CASDetachedWork` now stops and drains its detached publisher before the locals its hooks read go out of scope (ASan stack-use-after-return on `fake_boot` via `boot_ms_fn`), the same class of bug as the earlier test-hook lifetime fixes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115C2huxSQJkqDDV24h4JEi Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…esh path captures the slot S3ObjectStorage::client used to be a plain MultiVersion<S3::Client> value member, read via client.get() and written via client.set() from every call site. To let the SingleAttempt read path's credential-refresh lambda outlive this storage safely, the member had become a shared_ptr<MultiVersion<S3::Client>>, which turned all 27 of those call sites into client->get()/client->set(). Restore client as a reference bound to a new private client_slot (the shared_ptr the refresh lambda still captures), so every original client.get()/client.set() call site is textually unchanged. Only the one lambda that must own the slot independently of this object's lifetime captures client_slot directly. The class has no copy/move operations and is never copied (the only constructor call site is the delegating constructor), so a reference member is safe here; MultiVersion's own get()/set() split is preserved by the reference the same way the prior shared_ptr<MultiVersion> did. Also restores the double blank line before the class declaration that a driveby whitespace edit had removed, so that hunk disappears from the diff against upstream. diff --stat vs altinity/antalya-26.6 for the two touched files: S3ObjectStorage.cpp: before 376+/44-, after 355+/23- S3ObjectStorage.h: before 74+/8-, after 73+/7- Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
The shutdown()/getSingleAttemptClient() comments explaining why DisableRequestProcessing does not by itself stop dispatch, and what does, had grown to ~27 lines across the header and the .cpp. Compress each to the facts, one sentence per fact, and make them accurate against contrib/aws: - AWSClient checks IsRequestProcessingEnabled() BEFORE calling ShouldRetry (AWSClient.cpp:324) and breaks out of the loop when it is false; a SingleAttemptRetryStrategy clone lands on the same behaviour through the other branch of that check. The one reissue the flag does still suppress is the SDK's own region redirect of an `AWS_GLOBAL` client after a 301/307/400/403 reply, which precedes the retry strategy. - New open-plane requests after teardown are refused at Pool::teardownBegun() (CasPool.cpp), which CasOperation::readLoop (CasRequests.h) checks before every attempt; the mount and farewell planes stay admitting through this window. - The clone-site and header-field comments point back to shutdown()'s comment instead of repeating it. The `using Aws::S3::S3Client::GetHttpClient;` in S3::Client re-exposes a privately-inherited member; its only external caller is gtest_cas_s3_single_attempt_client.cpp, so it is marked test-only. Production code has no need of it since Client's own methods already reach GetHttpClient through the private inheritance. No behaviour change -- comment text only. diff --stat vs altinity/antalya-26.6 (continuing from the client-member commit): S3ObjectStorage.cpp: before 355+/23-, after 345+/23- S3ObjectStorage.h: before 73+/7-, after 66+/7- Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…ent.cpp gtest_aws_s3_client.cpp is an upstream test file; every fork-added test in it made rebasing conflict. The fork's additions (attempt-seed local-retry test, its TestPocoHTTPSequenceServer helper, and the SingleAttempt/network-error logging tests with their NetworkFailingClient/ScopedS3ClientErrorLogCapture helpers) are self-contained and move into the new fork-owned gtest_cas_aws_s3_client.cpp, under the suite name CASIOTestAwsS3Client so it does not share a suite name with the upstream file's suite. Running the relocated tests in the full battery exposed a pre-existing fragility in every local-HTTP-server helper of these files (TestPocoHTTPServer, TestPocoHTTPStsServer, TestPocoHTTPSequenceServer, ScriptedResponseServer): each constructed its Poco::Net::HTTPServer on Poco::ThreadPool::defaultPool(), one pool shared, unsynchronized, across every live TCPServerDispatcher in the test binary. TCPServerDispatcher's "can we start a thread" check is per-dispatcher against that shared capacity, so once enough other servers saturate the pool the dispatcher's startWithPriority throws and the just-accepted connection is closed without a response (seen via strace as a client-side "Connection reset by peer"). Each helper now owns a private Poco::ThreadPool, binds to and reports 127.0.0.1 explicitly instead of the wildcard bind address, and has a destructor that calls HTTPServer::stopAll(true) then joinAll() so an idle keep-alive worker does not cost PooledThread::release's 10 s join cap per test. This is the one deviation from gtest_aws_s3_client.cpp being byte-identical to altinity/antalya-26.6 (+26/-1). Verified: `unit_tests_dbms --gtest_filter='*S3*:*ReadBuffer*:*WriteBuffer*'` (263 tests, 45 suites) 3/3 green at 13.2-13.5 s, matching the 13.0 s measured before any thread-pool change; `CAS*:*S3*:*ObjectStorage*:*Teardown*` (2699 tests) green, 0 failures; the same set green under ASan. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
gtest_readbuffer_s3.cpp is an upstream test file; every fork-added test in it made rebasing conflict. The fork's additions (the responseIdentityChanged coverage plus BreakingHTTPBasicStreamBuf/rangeStart/makeGetObjectOutcome helpers) do not touch any upstream code and are not used by any upstream test, so they move into the new fork-owned gtest_cas_readbuffer_s3.cpp. The moved tests still need ClientFake/readAndAssert/CountedSession/ StringHTTPBasicStreamBuf and the fixture. The new file carries its own trimmed copies inside an anonymous namespace (internal linkage), and the fixture is renamed CASReadBufferFromS3Test: a same-named external-linkage copy would be a One Definition Rule violation -- the trimmed ClientFake is a different class than the upstream file's, and the fixture references file-local statics (cache_base_path, caches_dir, TEST_LOG_LEVEL) that are distinct entities per translation unit. An early draft with external linkage made the linker keep one ClientFake vtable for both files and sent the upstream ListObjectsV2-based tests through the trimmed override set, where they failed with a real 403 against a real endpoint. `git diff --stat` for gtest_readbuffer_s3.cpp against altinity/antalya-26.6 goes from 412 insertions to 0 (byte-identical to base). Test count for `unit_tests_dbms --gtest_filter='*S3*:*ReadBuffer*:*WriteBuffer*'` is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
gtest_writebuffer_s3.cpp is an upstream test file; every fork-added test in it made rebasing conflict. Unlike the read-side files, the fork's changes here are threaded through MockS3::Client's PutObject/HeadObject/DeleteObject bodies (an attempts_seen recorder) and add two overrides (ListObjectsV2, DeleteObjects) plus an injection struct, all inside the same `namespace MockS3` block the upstream tests use. The upstream file goes back to byte-identical with altinity/antalya-26.6. The fork's tests move into the new fork-owned gtest_cas_writebuffer_s3.cpp, which carries its own private copy of the whole `namespace MockS3` block (with the fork's instrumentation folded in), the writeAsOneBlock/ writeAsPieces helpers and the fixtures, renamed CASWBS3Test/CASSyncAsync with their own INSTANTIATE_TEST_SUITE_P. Everything copied lives in an anonymous namespace: both files link into unit_tests_dbms, and two different `MockS3::Client` definitions with external linkage would be a One Definition Rule violation, while gtest rejects two fixture types under one suite name. Six copied members the fork's tests never call are marked `[[maybe_unused]]`, since internal-linkage members trip -Wunused-member-function under -Weverything. A duplicated mock in a fork-owned file is cheaper than a shared header that turns every upstream change to the mock into a rebase conflict: the upstream file's diff against base goes from 329 insertions to 0. Test names and bodies are unchanged; the fork-only ScopedWriteBufferS3ErrorLogCapture helper stays local to the new file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
… that matters The comment above the response-identity move in ReadBufferFromS3::nextImpl had grown to explain a fixed historical bug (the field used to be copied, not moved) instead of the invariant a future edit actually needs to preserve. Say only that: the identity-baseline update must stay before `next_result` is set, since a throw after that point would exit the retry loop with `impl` left null while the code past the loop still dereferences it. Comment-only change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…e themselves without naming CAS
ObjectStorageControlRequest, ReadSettings::object_storage_attempt_number, and
WriteSettings::object_storage_attempt_number/s3_max_unexpected_write_error_retries_override/
s3_check_objects_after_upload_override are generic per-request knobs any
caller can set, not CAS-specific fields; their comments named CAS as if it
were the only caller. Reword each to describe what the field carries and
why, for any caller. Also drops a dangling internal-RFC citation
("RFC cas-s3-timeout-retry-control") that named a document outside the
branch.
Comment-only change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…load PreconditionFailed and a SingleAttempt write's failed attempt were logged with the exact same LOG_DEBUG call, each behind its own explanatory comment, duplicating the format string and every argument. Combine the two conditions into one branch with one call, keeping both reasons in a single comment. No behaviour change: same log level, same message, same arguments, for the same two conditions. diff --stat vs altinity/antalya-26.6 for this file: before 11+/2-, after 10+/4- (the comment combines two explanations into one paragraph); net line count in the file is 3 lines shorter, confirmed by `git diff --stat` of this commit (6 insertions, 9 deletions). Verified by building WriteBufferFromS3.cpp.o directly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…t CAS delete reuses it
The one-object branch of `removeObjectsIfExistImpl` re-implemented
`deleteFileFromS3` (request, profile event, blob-storage-log event, 404
tolerance, error text) only because the helper could not stamp the
`clickhouse-request` attempt header the CAS request engine requires.
Give `deleteFileFromS3` an optional trailing `attempt_seed` (default 0,
which keeps every existing caller unchanged and sends no header) and call
it from that branch.
Observable differences for that branch: the profile event `S3DeleteObjects`
is now incremented alongside `DiskS3DeleteObjects`, and the success line
`Object with path {} was removed from S3` is logged, exactly as the
ordinary `removeObjectImpl` path already does. The batch path and the
conditional (`If-Match`) delete are unchanged; the latter cannot reuse the
helper because it needs the precondition, the native-conditional mode and
the three-way outcome.
Footprint against upstream: `S3ObjectStorage.cpp` −29 lines,
`deleteFileFromS3.{h,cpp}` +4 lines.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
b5ad8a9 to
2c3c71f
Compare
…tity
`SYSTEM CAS FORGET` (commit 761cc8b8488) decommissions a content-addressed
pool for the lifetime of the server: a later `CREATE TABLE` naming the same
`cas_server_root_id` / `name` / `path` fails with `INVALID_STATE`
("content-addressed pool decommissioned by SYSTEM CAS FORGET"). CI's
`tests/clickhouse-test --repeat-newly-modified-tests` repeats the
highest-numbered tests several times against ONE server, so every repeated
run of a CAS test hit this and failed the CREATE (or, in `.sh` tests where
the failed CREATE isn't fatal, failed later with `UNKNOWN_TABLE`) -- this is
what took down every first-stage stateless lane in CI run 7 of PR #2300.
Every stateless test that creates an inline `cas` disk now derives all three
identity fields -- `cas_server_root_id`, `name` and `path` -- from the
per-run database name (`$CLICKHOUSE_DATABASE` in `.sh` tests), so a repeat
of the same test in the same server never reuses a decommissioned pool's
identity. A few tests already derived part of the identity from
`$CLICKHOUSE_TEST_UNIQUE_NAME`/`$RANDOM` (04290, 04295, 05008, 05020, 05023,
05025); those only needed their remaining static field(s) fixed.
`05024_cas_freeze_two_roots.sh` and `05025_cas_attach_partition_cross_disk.sh`
each mount several named disks (some intentionally sharing one pool path
across two `cas_server_root_id`s) and needed each disk's three fields
threaded through consistently. `05003_cas_freeze.sh`'s `FREEZE ... WITH NAME`
snapshot name also embedded the same static id, so it is now suffixed with
`$CLICKHOUSE_DATABASE` too, with the `.reference` normalizing the database
name out of the printed `backup_name` column.
Thirteen of these tests were `.sql` files: clickhouse-test does no textual
substitution on `.sql` query files, and `disk(...)` settings do not accept
query parameters (`disk(... name = {p:String})` fails to parse), so there is
no way to inject a per-run value into a `.sql` test's `disk(...)` literal.
Each is converted to an equivalent `.sh` test of the same name, feeding the
same statements through `$CLICKHOUSE_CLIENT --multiquery` so the per-run
`$CLICKHOUSE_DATABASE` variable can be substituted; none of them use
`-- { serverError }` / `-- { clientError }` hints, comments and output stay
otherwise unchanged, and every `.reference` file stays byte-identical.
Verified by running all 31 touched tests twice against one local server
(`tests/clickhouse-test --test-runs 2`), matching the repeat mechanism that
exposed the bug: 62/62 passed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…lock CI run 7 of PR #2300 turned CASDecommission.FailedDrainKeepsSlotThenResumes and CASDecommission.ManifestDebrisFailureKeepsSlotThenResumes red under ASan/UBSan: the farewell write on Pool teardown gave up at the lease deadline after zero attempts, so the mount slot was never retired and the resume half of each test refused with "pool member is alive or contended". decommissionPoolMember's drain_now_fn/drain_sleep_fn seam replaces the clock the drain's own request engine paces its retries on, but the opened Pool's mount-lease renewer binds its farewell deadline to a separate clock, PoolConfig::boot_ms_fn, which was left on the real boot clock (CLOCK_BOOTTIME) regardless. On a freshly booted CI VM (real boot time below FakeClock's starting instant) the two clocks disagreed enough that the farewell's own Retry::untilLeaseSafe bound looked already-expired before the first attempt; a long-lived dev box's real uptime dwarfing the fake clock is why this passed locally. Fold drain_now_fn into config.boot_ms_fn whenever the caller left the latter unset, so a test (or any other caller) faking only the request clock does not end up comparing it against an unrelated one. Verified both CasRequests.cpp's bootClockMs and CasServerRoot.cpp's defaultBootMs are CLOCK_BOOTTIME, so production itself already uses one clock for both purposes -- the mismatch was confined to this test seam. Add a regression test that pins the fake clock far beyond any real host's boot time, so the mismatch (and the fix) reproduce deterministically on every machine rather than only a freshly booted one. Unifying the clocks also exposed a second-order timing issue in FailedDrainKeepsSlotThenResumes: its FakeClock fast-forwards through the entire 90 s Retry::standard() window while draining one failing object, and drain_now_fn is now also the admin session's boot clock, so the default 30 s mount lease TTL was no longer enough for the farewell to fit by the time the drain's own retry exhaustion had "elapsed". A real decommission's background renewer keeps the lease fresh over that much real time; nothing in the test advances real time to let it. Widen that one test's admin PoolConfig::mount_lease_ttl_ms to give the farewell comfortable headroom past its own retry-exhaustion budget. Gates: release and ASan unit_tests_dbms --gtest_filter='CASDecommission*' (37/37, including 8x --gtest_repeat --gtest_shuffle), release+ASan --gtest_filter='CASDecommission*:CASEnvelopeWiring*:CASGCBoundedWalk*' (49/49), and the full ASan gate --gtest_filter='CAS*:*S3*:*ObjectStorage*:*Teardown*' (2703/2703, one pre-existing unrelated skip). All green. CI report: ClickHouse#2300 Follow-up folded in: `FakeClock` is thread-safe (the renewal thread now reads it through `boot_ms_fn` while the drain thread advances it), and `PoolConfig::retry_sleep_fn` installs the test sleep together with the test clock so the bootstrap requests of `openForDecommission` never pace real sleeps against a frozen clock; a pacing regression test pins that. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…itizers CI run 7 of PR #2300 turned three CASEnvelopeWiring connect-cap timing assertions red under MSan: EXPECT_LT(capped_elapsed.count(), 1000) failed with 1470-2494 ms (gtest_cas_s3_single_attempt_client.cpp:702, :725, :786), against a base connect timeout of 2000 ms and a single-attempt connect cap of 100 ms, with the uncapped attempt asserted >= 1500 ms. A sanitizer build adds a roughly constant addend to both the capped and uncapped connect attempts, so an absolute upper bound on the capped side alone is not sanitizer-safe. Assert the DIFFERENCE instead: the cap must remove at least half of the connect budget it is capping (default_elapsed - capped_elapsed >= (base_connect_timeout_ms - cap) / 2), keeping the existing EXPECT_GE(default_elapsed, 1500) lower bound on the uncapped side. Applied to all three failing sites; the third (FreezeConnectTimeoutCapReachesTheBackendOverProductionDispatch) reuses the uncapped backend's own measurement at :773 as its baseline instead of a second literal 1500 ms floor. Gates: release and ASan unit_tests_dbms --gtest_filter='CASEnvelopeWiring*' (4/4, including 5x --gtest_repeat), release+ASan --gtest_filter='CASDecommission*:CASEnvelopeWiring*:CASGCBoundedWalk*' (49/49), and the full ASan gate --gtest_filter='CAS*:*S3*:*ObjectStorage*:*Teardown*' (2703/2703, one pre-existing unrelated skip). All green. No MSan build was available locally; the fenced values match the MSan CI log exactly. CI report: ClickHouse#2300 The absolute `< 1000 ms` bound stays on non-sanitizer builds (the difference bound alone is weaker there), `capped < uncapped` holds unconditionally, and the frozen cap is also read back from the backend so sanitizer builds keep a non-timing discriminator. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
CI run 7 of PR #2300 turned CASGCBoundedWalk.ARoundFoldsThroughItsRoundStartTailAndLeavesTheStragglers red under TSan: a data race on ChasingWriterBackend::appending/published/ layout (gtest_cas_gc_bounded_walk.cpp:167), because read() mutates them without synchronization while the GC fold read-ahead (Gc/CasGcReadAhead.h, a ThreadPool in front of one operation) can land several hinted reads on different worker threads at once. The class comment's premise -- "a synchronous hook rather than a thread" -- no longer holds for the reads themselves; it was true only for the single append the hook performs, not for the concurrent reads that can trigger it. Guard the hook's mutable state (layout, ns, published, limit, appending) with a mutex, held across the check-and-publish so the re-entrancy guard keeps meaning across threads, not just within one call. publishAt issues backend calls of its own, so the lock is released before it runs and re-acquired only to record the result -- holding it across publishAt would either self-deadlock on a re-entrant call or serialize every read-ahead worker behind the one doing the append. arm/disarm/ publishedThrough take the same lock. Updated the class comment to state the current concurrency model instead of the no-longer-true synchronous premise. No build_tsan exists in this worktree, so the fix was verified under ASan and release only, with repeats; TSan was not run locally. Gates: release and ASan (5x --gtest_repeat) unit_tests_dbms --gtest_filter='CASGCBoundedWalk*' (8/8), release+ASan --gtest_filter='CASDecommission*:CASEnvelopeWiring*:CASGCBoundedWalk*' (49/49), and the full ASan gate --gtest_filter='CAS*:*S3*:*ObjectStorage*:*Teardown*' (2703/2703, one pre-existing unrelated skip). All green. CI report: ClickHouse#2300 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…rencing test-frame locals A `Pool` can outlive the test body that opened it: a detached task (`Pool::tryDispatchDetached`, `DetachedTaskLease::Completion`) or a background publish holding `shared_from_this()` drops the last `shared_ptr<Pool>` later, and `~Pool` then runs the mount-lease farewell, which calls `PoolConfig::boot_ms_fn` / `wait_sleep_fn` / `retry_sleep_fn`, the event sink and the other config hooks. A hook that captured a test-frame local by reference read a dead stack slot; ASan caught one site (fixed in a726e933419) and MSan the next (`gtest_cas_ref_snapshot_publish_ordering.cpp`, reported from `~Pool` on a detached thread and attributed to a later test). Sweep of the whole class across the CAS gtests: 92 `_fn = [&` sites, 17 `event_sink`/`*_hook_for_test` sites and 47 setter-installed hooks (`setCasRetrySleepForTest`, `setCasRequestNowFnForTest`, `setEventSink`, `setWaitSleepForTest`, ...). Every hook that reaches a real `Pool` now owns its state: `shared_ptr<std::atomic<uint64_t>>` for clocks and counters, a heap-owned `FakeClock`, and two small helpers in `cas_test_helpers.h` (`SharedWaitLog`, `SharedEventLog`: heap-owned, mutex-guarded vectors read through `snapshot()`, since a background renewer or farewell can push from a thread the test never joins). Sites whose receiver provably cannot outlive the frame (the synchronous `claimMountAwaitingExpiry`, a bare `CasRequests` local, `RuntimeUnderTest` whose destructor joins its workers) keep their by-reference captures with a comment saying why. Two real lifetime hazards found on the way are fixed too: a detached publisher's counters in `gtest_cas_detached_work.cpp` and a remount-callback barrier in `gtest_cas_pool.cpp` that an early assertion failure destroyed before the Pool joined the worker. Test-only. Each touched suite ran 5× under ASan; full ASan and release CAS gates green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
`CASEnvelopeWiring` stalled a real TCP connect and measured elapsed time (`EXPECT_LT(capped_elapsed, 1000)`), which failed under MSan at 1.5–2.5 s and would fail on any slow host. Timing bounds, ratios or sanitizer-gated asserts only move the threshold. The tests now dispatch through the production path against an ordinary mock server and assert two clock-free facts: the single-attempt client cache holds exactly the (attempt timeout, frozen cap) key the request must have used (`hasSingleAttemptClientForTest`, a test-only const accessor that inspects the cache and never creates a clone), and that clone's `getClientConfiguration().connectTimeoutMs` equals the cap while the Default client keeps the base value. A wrong dispatch fails immediately. That `PocoHTTPClient` applies `connectTimeoutMs` to the socket is upstream behaviour and is not re-proved here. The connect-stall helper and its `tcp_abort_on_overflow` skip logic are gone; the shared `DelayedResponseServer` fixture disables HTTP keep-alive so a pooled connection cannot outlive the ephemeral-port server that served it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Two MSan failures in CI run 8 had the same root: the module ran with a 1 s lease TTL and a 50 ms attempt budget, so a single lost-response resolve read (1059 ms under MSan) fenced the renewal instead of resolving it, and the hard-restart test asserted the token-stability observation log line right after `start_clickhouse` returned while the CAS disk's `Pool::open` was still probing the mount object. One fixed budget for every build (`mount_lease_ttl_ms` 10000, renew period 2000, attempt timeout 500, connect cap 500, safety margin 500; both `validateCasRequestBudget` inequalities hold with wide margin), every expectation derived from those constants (the observation string is `ttl + ttl/20 + poll`), the hard-restart test waits for `system.cas_mounts` to report `live` before counting the line, and every `_wait_until` probe runs under one shared deadline that is recomputed before each query, HTTP control call and RustFS request (connect/read timeouts on the S3 client), rejecting late results. Assertions keep their meaning; the module runs in ~52 s locally. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
The two publish-visibility tests waited 2 s on a real `std::async` task (the tightest bare future wait in the suite), and a failed assertion before `release_source.set_value` unwound into the future's blocking destructor while the publisher waited forever. Every wait on both sides now has the file's 20 s bound, a scope guard releases the publisher on every exit path, an expired barrier is an explicit test failure instead of a silent release, and the publisher lambda's total runtime is bounded so a stuck publisher costs at most the sum of its internal bounds before the destructor returns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
The test asserts `force_optimize_projection` on the normal projection `p_by_b` of two tables. Under a randomized MergeTree `index_granularity` (703 in the failing CI runs) a merge re-granulates the base part continuously but rebuilds the projection at the source parts' granule boundaries, so the projection ends up with more marks than the table and the planner correctly refuses it with `PROJECTION_NOT_USED`. Not a CAS bug: the same statements fail on a plain `MergeTree` on upstream 26.6.2. Pin `index_granularity = 8192, index_granularity_bytes = 10485760` on both tables, the same guard `04300_cas_projection_multiblock` already carries. Verified with clickhouse-local: `index_granularity = 703` reproduces the 584, the pinned tables use the projection after two inserts and an `OPTIMIZE FINAL`, and `SYSTEM CAS FORGET` still succeeds. Closes: #2325 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
`HTTPServerConnection::run` polls `_stopped` in its loop condition outside `_mutex`, while `onServerStopped` (reached from `HTTPServer::stopAll(true)`) writes it from the stopping thread. A plain `bool` is a data race; TSan reported it from the test servers that abort their connections on teardown (`base/poco/Net/src/HTTPServerConnection.cpp:154` vs `:61`). `std::atomic<bool>` keeps the exact semantics with sequentially consistent loads and stores. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> (cherry picked from commit 5340794a38066ce0d9fce854aaea6c84dd3bc0d4)
…n per request Every test in these files starts its own HTTP server on an ephemeral port and destroys it with the test. With keep-alive on, the process-wide connection pool can hand a later test a pooled connection to a port whose server is already gone, and the request fails with `Connection reset by peer`; reproduced with `--gtest_repeat=3` on `S3BulkDeleteFallback` (three tests failed in iteration 3 only). `http_keep_alive_timeout = 0`, the same setting `gtest_cas_s3_single_attempt_client.cpp` already uses for the same reason. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Only the bulk-delete fallback suite needs it: `makeNetworkFailingClient` in `gtest_cas_aws_s3_client.cpp` creates no mock server, so that file is left as is. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
PR #2300 CI Verification ReportVerdictDo not approve for merge yet. Release / binary CAS insert–select paths look largely healthy (binary CAS stateless green; Fast test green; most integration shards green). Remaining blockers are:
Author triage on the PR (runs 1–3) remains accurate for R7 / T4 / Iceberg / SimSIMD; this report re-checks head Summary table
Funnel (head Job-level fail counts (head SHA)From
Also red on GitHub but not fully represented as test rows: msan/tsan CAS S3 shards hitting the 6h budget; aarch64 CAS alter/LWD 5h timeouts. Database rate comparison (
|
| Test | Branch runs | Branch fails | Branch % | PR runs | PR fails | PR % | Reading |
|---|---|---|---|---|---|---|---|
test_auth_token_profile_events |
8 | 8 | 100 | 16 | 16 | 100 | pre-existing-flaky / broken test on branch |
test_schema_inference[s3-1-True] |
145 | 26 | 17.9 | 21 | 9 | 42.9 | Elevated but known Iceberg UBSan (#2216); not CAS |
test_cas_mount_renewal_retry::…landed_response_lost… |
14 | 0 | 0 | 20 | 6 | 30 | regression |
test_cas_mount_renewal_retry::…hard_restart… |
0 | 0 | — | 8 | 1 | 12.5 | regression (new / PR-only) |
CASRefStateMachine.OwnerTransitionRejectsInvalidCombinations |
7 | 0 | 0 | 11 | 1 | 9.1 | regression (PR-only PRs list = [2300]) |
IOTestAwsS3Client.NativeConditionalModeIsRederivedOnEverySdkAttempt |
7 | 0 | 0 | 11 | 1 | 9.1 | regression (PR-only) |
Cannot start clickhouse-server |
30 | 30 | 100 | 3 | 3 | 100 | infrastructure / known arm_tsan |
03572_export_merge_tree_part… |
129 | 6 | 4.7 | 102 | 2 | 2.0 | pre-existing-flaky |
02477_single_value_data_string_regression |
965 | 5 | 0.5 | 58 | 2 | 3.4 | pre-existing-flaky (labeled) |
00172_early_constant_folding |
946 | 1 | 0.1 | 59 | 1 | 1.7 | weak n=1; labeled reproducible on CAS asan |
00975_move_partition_merge_tree |
961 | 0 | 0 | 60 | 1 | 1.7 | under CAS asan memory pressure |
01461_query_start_time_microseconds |
949 | 0 | 0 | 59 | 1 | 1.7 | labeled flaky |
01883 / 02435 / 03402 / 04105 |
hundreds | ≤2 | ≤0.3 | 72–181 | 3 | ~1.7–4 | CAS asan memory-limit cluster (T4) |
00427 / 02293 / 04266 (msan WasmEdge) |
~960 | ~0–1 | ~0 | ~60 | 1 | ~1.7 | pre-existing-flaky (labeled) |
Other PRs hitting test_auth_token_profile_events in 30d: 0,2251,2290,2294,2300,2305,2309,2315,2318,2320,2330 (42 fails) — clearly not unique to #2300.
Regression suite rates (gh-data.clickhouse_regression_results, 30d)
/selects/final/force/concurrent (parent)
| Kind | On PR #2300? | OK | Fail | Fail % |
|---|---|---|---|---|
| cas | no | 75 | 25 | 25.0 |
| cas | yes | 14 | 2 | 12.5 |
| non_cas | no | 272 | 1 | 0.4 |
| non_cas | yes | 7 | 0 | 0 |
Category: regression (CAS product availability), pre-existing on CAS base — not uniquely worse on #2300 (PR rate is lower than other CAS). Mechanism on d876cda amd Release: Code: 210 NETWORK_ERROR — mount lease not held / TransientNotLive during concurrent FINAL (no injected fault). AArch64 cas_selects and cas_s3_cache_selects passed same SHA. Non-CAS selects green. Do not xfail; fix lease renewal under load.
Note: S3
report.htmlfor x86 selects was overwritten to look green after the job failed — trust the GitHub job log, not the HTML alone.
/alter/attach partition/part 1/replica sanity/parallel add remove sanity (R7)
| Kind | On PR #2300? | OK | Fail |
|---|---|---|---|
| cas | yes | 1 | 3 |
| cas | no | 16 | 15 |
| non_cas | no | 319 | 5 |
| non_cas | yes | 2 | 0 |
Sibling / package split (cas jobs): other PRs 14 fail / 14 ok; release/ref 1 fail / 2 ok.
Category: regression of CAS+replicated partition workload on the base line (R7), not unique to this PR. Leaf assert: expected 500 rows, got 400. Author diagnosis: common pool admission refuses without logging, still increments num_tries, then 300s backoff coincides with SYNC REPLICA budget; first real error often CAS relink-confirm NO_REPLICA_HAS_PART. AArch64 twins time out at 5h.
/lightweight delete/concurrent delete/MergeTree/random delete entire table without overlap
| Kind | On PR #2300? | OK | Fail | Fail % |
|---|---|---|---|---|
| cas | no | 5 | 1 | 16.7 |
| cas | yes | 0 | 2 | 100 |
| non_cas | no | 311 | 2 | 0.6 |
| non_cas | yes | 7 | 0 | 0 |
Category: unknown → lean product race. Mechanism: concurrent non-overlapping LWD left count(*)=1 vs expected 0 (all DELETEs returned 0). Non-CAS on this PR passed; CAS on this PR failed both runs (n=2). Known rare leftover-row race also seen on non-CAS head. Needs more CAS runs or a minimal SQL repro before calling it a #2300 regression.
/tiered storage/with cas/simple replication and moves
On d876cda x86: errno 28 No space left on device writing to jbod1. AArch64 same suite: module OK.
Category: infrastructure.
Root-cause analysis (survivors)
1. Unit MSan — CASRefStateMachine.OwnerTransitionRejectsInvalidCombinations
- Category:
regression - Mechanism: MSan
use-of-uninitialized-valueinstd::functioncall path duringCASRefSnapshotPublishOrdering.NotReadyRefusalBacksOffAndResetsAfterDurablePublish; process exits as the next test starts (mis-attributed name). - DB: 0/7 branch fails; 1/11 on PR; fails only on PR list
[2300]. - Action: Fix uninit in NotReadyRefusal / snapshot publish ordering gtest path under MSan.
2. Unit TSan — IOTestAwsS3Client.NativeConditionalModeIsRederivedOnEverySdkAttempt
- Category:
regression - Mechanism: TSan data race in
Poco::Net::HTTPServerConnection::onServerStoppedwhile the new conditional-mode gtest runs. - DB: 0/7 branch; 1/11 PR; PR-only.
- Action: Serialize / stop server before teardown, or mark race if confirmed false positive in test-only Poco path.
3. test_cas_mount_renewal_retry (msan)
- Category:
regression - Mechanism:
landed_response_lost…waits 120s, last=None(retry_failed);hard_restart…expected observation log count 1, got 0. - DB: landed: branch 0/14, PR 6/20 (30%).
- Action: Revisit msan timing / observation after recent wait-margin commits; still red on
d876cda.
4. T4 — Sanitizer CAS stateless resource pressure
- Category:
regression(CI coverage / resource), not a binary functional break - Mechanism: ASan CAS jobs accumulate memory (
Code: 241 memory limit exceededon many late tests); msan/tsan CAS shards hit 6h. Binary CAS parallel passed (amd + arm). - Evidence: Author T4 (~2400 threads / per-disk pools);
SYSTEM CAS FORGETlanded and is being measured. - Action: Confirm forget helps; shrink sanitizer pool profile if needed.
5. R7 — CAS alter attach replica divergence
- See rate table above. Release blocker for heavy replicated partition ops on CAS until common-pool
num_tries/ pool sizing fix lands.
6. cas_selects concurrent FINAL — mount lease
- See rate table. Designed refusal when not Live; unexpected lease drop under quiet MinIO + concurrent FINAL. Product bug, pre-existing CAS sensitivity; CAS improvements #2300 still owns fixing it for antalya-26.6 CAS quality.
Unrelated (do not block this PR’s CAS claim)
| Item | Category | Evidence |
|---|---|---|
Iceberg test_schema_inference (+ read_in_order cascades) |
cascade / branch bug |
UBSan decimal overflow; #2216; Connection refused after server abort |
test_auth_token_profile_events |
regression of test vs rename (not this PR) |
100% branch + many PRs; counters renamed after #2222 |
Stress arm_tsan Cannot start |
infrastructure |
100% branch; SimSIMD ARM SIGILL probe |
| WasmEdge / azure labeled flaky | pre-existing-flaky |
CI labels + rates |
cas_s3_cache_aggregate_functions_1 module Fail |
unknown / low signal |
1 Fail row at /aggregate functions; coverage text elsewhere says module ok — treat as noise until leaf reproduced |
What is green (signal that CAS core works)
- Builds (all sanitizer/release/debug listed in report)
- Fast test
- Stateless binary CAS S3 (amd + arm)
- Most integration shards (including many CAS tests that failed on earlier commits)
- AArch64
cas_selects;cas_s3_cache_selects(where run) - Grype / Docker / install / compatibility
Recommendations
- Before approve: clear unit MSan + TSan and msan mount-renewal (items 1–3).
- Track explicitly (may ship with known issues): R7 alter attach; concurrent FINAL mount-lease; T4 sanitizer budgets — with release notes if merging.
- Ignore for this PR: Iceberg antalya-26.6: test_schema_inference kills the server on amd_asan_ubsan — upstream Decimal-bounds overflow exposed by #2145 #2216, auth_token counter rename, SimSIMD arm_tsan, labeled flaky.
- Rerun: amd
cas_selects(confirm lease blip rate); CAS LWD concurrent scenario (n=2 is weak). - Do not trust overwritten S3 TestFlows HTML for selects — use job logs / DB.
Approval checklist
| Question | Answer |
|---|---|
| Did this PR uniquely break binary CAS insert/select? | No strong evidence (binary CAS green) |
| Are there PR-only failures that must be fixed? | Yes — unit MSan/TSan; mount-renewal msan |
| Are there CAS product issues that pre-exist on base but block release quality? | Yes — R7; mount-lease under concurrent FINAL; sanitizer CAS T4 |
| Unrelated red paint? | Yes — Iceberg, auth_token, arm_tsan stress |
| Approve now? | No |
Appendix: query snippets used
-- Unique fails on head
SELECT test_name, count() fails
FROM `gh-data`.checks
WHERE pull_request_number = 2300 AND test_status = 'FAIL'
AND commit_sha LIKE 'd876cda%'
GROUP BY test_name ORDER BY fails DESC;
-- Branch vs PR rates
SELECT test_name,
countIf(pull_request_number = 0) branch_runs,
countIf(pull_request_number = 0 AND test_status='FAIL') branch_fails,
countIf(pull_request_number = 2300) pr_runs,
countIf(pull_request_number = 2300 AND test_status='FAIL') pr_fails
FROM `gh-data`.checks
WHERE test_name IN (...) AND (pull_request_number IN (0, 2300))
AND check_start_time > now() - INTERVAL 60 DAY
GROUP BY test_name;
-- Regression CAS vs non-CAS
SELECT multiIf(job_name LIKE '%cas%','cas','non_cas') kind,
clickhouse_package LIKE '%/PRs/2300/%' on_2300,
sum(result='OK') ok, sum(result='Fail') fail
FROM `gh-data`.clickhouse_regression_results
WHERE test_name = '<path>' AND start_time > now() - INTERVAL 30 DAY
GROUP BY kind, on_2300;`stopAll(true)` reaches `HTTPServerConnection::onServerStopped(abortCurrent = true)`, which shuts the connection's socket down without taking the connection mutex so that it can interrupt a handler holding it. A worker that is leaving `run` at the same moment closes that socket from its own thread, and TSan reports the race on `SocketImpl::_sockfd` (CI run 9 of PR #2300, Unit tests (tsan), `IOTestAwsS3Client.NativeConditionalModeIsRederivedOnEverySdkAttempt`). The abort path was only there to unblock workers waiting for the next request on a pooled keep-alive connection before `thread_pool.joinAll`. Close those connections from the client side instead: drop the process-wide `HTTPConnectionPools` cache, which the AWS SDK client uses through `makeHTTPSession`, so every worker sees end of stream and closes its own socket on its own thread; then `stop` the server (accept thread joined, dispatcher stopped, no `serverStopped` notification) and join the pool. Applied to all four fork mock servers: `TestPocoHTTPServer`, `TestPocoHTTPStsServer`, `TestPocoHTTPSequenceServer`, `ScriptedResponseServer`. Production is unaffected: `DB::HTTPServer::stopAll` never used the abort path. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx
This reverts commit 5340794a380, a fork patch to upstream Poco: the only callers of `HTTPServer::stopAll(true)` in this tree were the test mock servers, and they no longer use Poco's abort notification (see the previous commit), so `onServerStopped` never runs concurrently with `run` here and the fork patch to `base/poco` is not needed. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx (cherry picked from commit ca187904e613d3c0755bdf3ac38dc008a127cf4b)
…alya-26.6/CAS-improvements-cicd-fixes Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Closes: #2219
Closes: #2291
Related: #2233
Related: #2159
What this is
Follow-up to #2159 (the initial
casmetadata-storage subsystem): correctness fixes, aperformance/reliability overhaul of the backend request layer, a wire-format key rename, and
three merged features (GC fold read-ahead, forced relink on fetch, GC teardown no longer
blocking on a round), plus a first phase of write-lane serialization for hot control objects
and a GC-cost cut for write-once keys. 22 commits, ~300 files, +38.6k/-19.4k restricted to CAS
paths. This is a curated, squashed reconstruction of the
cas-gc-rebuilddevelopment tree:internal design docs, plans and the soak-harness (
utils/ca-soak) are intentionally not partof this PR (dev-only tooling, no shipped behavior).
How the series is structured
S3_ERROR/NETWORK_ERROR/timeout during a GC round now yieldsAborted(keeping leadership) insteadof an indistinguishable
Failedthat zeroed real counters and droppedi_am_leader; theemulated blob-publication path streams instead of materializing ~1 GiB in memory. (Related CAS: replica HTTP dies on green-path soak after relink NETWORK_ERROR storm #2233)
NO_REPLICA_HAS_PARTinstead ofNETWORK_ERRORfor a relink-confirm refusal — adesigned fail-closed outcome, not a network fault; fixes false Error-level triage and
part_loghygiene checks. (Closes CAS: relink proof refusals are logged at Error with a stack trace despite being an expected outcome #2219)WireKey,EnumWireTablewithset-equality coverage proofs) for every CAS wire-format codec; no bytes change yet.
abbreviated/single-letter keys to descriptive names (
kind,outcome,state, ...).decode), plus fixes that let the full stateless suite run locally.
cas: recommend single-replica merges (doc).ALTER TABLE ... EXPORT PARTITIONnow works from a source on a CAS disk — it readsthrough a sequential source and writes via a sink, nothing is hardlinked on the source
disk, so the CAS refusal was by omission, not by substance. (Closes CAS: EXPORT PARTITION from a CAS source is rejected with SUPPORT_IS_DISABLED #2291)
storage.buckets.getIAM grant turned into a hard outage; now warns and continues. Plusthree live-GCS test-suite corrections found on the first credentialed run.
any mutation of the same namespace, which live-GCS testing showed livelocking two
replicas for 40 minutes under sustained write load; now scoped to the ref actually asked
about.
HEADon a cache hit (a manifest idnames its content, ever); retires the
part_folder_validatesetting that existed only topace that
HEAD.CasRequests/CasOperationrequest engine (core) — replaces the free-formToken(which let an empty value pass as a fenced condition, silently turning a conditional write
into an unconditional overwrite, among 8 other defect classes found across two design
reviews) with a type only the backend can mint, a deadline-bound retry engine, and a
SingleAttemptrequest mode that halves the request cost of a control-object read.matching test-double migration.
Incarnation→Etag/TokenType→Dialectrename, gtest suite onto the engine, deletethe old controller — naming the type by its actual role instead of its wire field;
~120 mechanical test-file migrations; retires the ~1500-line legacy controller.
cas_gc_read_concurrency) — overlaps the fold's small-objectround trips on a bounded pool without moving any decision off the round thread; measured
1.65x on the round overall (2.4x on
fold_ref_intake, which a live-GCS soak measuredtaking 97+ minutes unfinished).
every pool of its storage policy instead of one guess, so a shared-pool fetch never moves
bytes just because a TTL rule or volume order disagreed with the guess.
its next request/retry-sleep/refill via a teardown liveness carried on the open request
plane, recorded as
Stoppedrather thanAborted.instead of once per listed manifest, late/read-ahead manifest reads, and a bulk-delete verb
for owner-removed manifests and ref-object cleanup. Measured on real GCS:
fold_reduce300-380s → 2-5s,manifest_deletes617s → 2s,ref_object_cleanup204s → <1s.for
cas/ref_catalog's conditional writes, replacing same-process write races (measured:DROP TABLEp90 11.9s/max 34.7s, 113PreconditionFailedin 80s from 53 threads) withqueuing and combined commits.
teardown hook, plus a stateless-lane GC-scheduler interval tuning.
SingleAttemptconditional-write attempt is resolved by an outer retry loop, not terminal; logging it at
Error was a false-positive operator signal.
Verification
CAS*gtest gate green throughout (the request-engine migration alone: 2406/2406 at itsfinal checkpoint).
test_cas_replicated_relink,test_cas_gcs,test_cas_gc_sharded,test_cas_gc_bulk_deleteintegration suites.
bodies for exact figures); the hot-key write lane's contention measurements are from ten
parallel stateless CI jobs.
cas-gc-rebuildcommit each squash groupwas cut from is empty by construction (verified file-by-file before opening/updating this PR).
Developed with AI assistance (Claude); every commit carries
Co-Authored-ByandSigned-off-by.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Improvements and fixes to the experimental content-addressed storage (
cas) metadata-storagetype:
ALTER TABLE ... EXPORT PARTITIONnow works from a CAS-disk source; fetch-by-relinkalways lands a same-pool part on that pool's disk instead of occasionally streaming its bytes;
a disk's teardown no longer blocks on an in-flight GC round; several GC-round cost cuts on
real object storage (round-trip counts down by 60-300x on the measured phases); a reliability
overhaul of the CAS-to-object-storage request layer closing several conditional-write edge
cases; and the CAS wire-format's internal JSON keys are renamed from abbreviations to
descriptive names (format generation reset; no compatibility concern, since CAS has no
released, persisted data yet).
Documentation entry for user-facing changes
Updated in this PR:
docs/en/antalya/cas/architecture/{backend,garbage-collection,manifests-and-refs,mounts-and-leases,read-path,replication,storage-layout}.md,docs/en/antalya/cas/{index,configuration,bucket-requirements}.md,docs/en/antalya/cas/operations/{debugging,monitoring,troubleshooting}.md,docs/en/operations/storing-data.md,docs/en/operations/system-tables/{cas_log,cas_gc_log}.md.CI/CD Options
Exclude tests:
Regression jobs to run: