[WIP] CAS draft (adopting to CI/CD, not for review / merge) - #2073
Draft
filimonov wants to merge 4161 commits into
Draft
[WIP] CAS draft (adopting to CI/CD, not for review / merge)#2073filimonov wants to merge 4161 commits into
filimonov wants to merge 4161 commits into
Conversation
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
…ent_addressed stateless lanes) Config Workflow check failed with 'Workflows are outdated' for master.yml, pull_request.yml, pull_request_community.yml, release_builds.yml. Regenerated via 'python3 -m praktika yaml'. The regeneration adds the two CAS stateless jobs to the generated workflows: 'Stateless tests (arm_binary, content_addressed storage, parallel)' and 'Stateless tests (arm_binary, content_addressed s3 storage, parallel)' (the rustfs-backed lane). CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=927ea142c9cb14759623861eb004261d0b4b1c8f&name_0=PR&name_1=Config+Workflow PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
…teless lane The lane's start_rustfs expected a pre-extracted binary at ci/tmp/rustfs and failed on CI runners where nothing provisions it (the workflow wipes ci/tmp on every run). Download the static musl build for the runner architecture from the RustFS GitHub release (1.0.0-beta.9) when the binary is absent, mirroring how setup_minio.sh downloads minio/mc. Validated locally: the beta.9 binary passes the conditional-operation semantics the CA pool requires (second 'If-None-Match: *' PUT -> 412, wrong-etag conditional DELETE -> 412, right-etag DELETE succeeds), and download_rustfs provisions an executable binary end-to-end. PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
Fast test fails at cmake generation: 'Target "dbms" links to ch_contrib::crc32c but the target was not found' — the fast-test job initializes a limited submodule list that does not include contrib/crc32c, so the unconditional add_contrib is skipped while the dbms link line still references the target. The dependency is dead: it was wired in for per-block CRC32C in the early CAS run-file format (5f1272c), which was later replaced by the text record-stream codecs; no source file includes the library today. Restore the pre-CAS state: crc32c is built only for google-cloud-cpp, and dbms does not link it. CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=835251f81cb5af73ad9eaa3a835f50f0c8b678db&name_0=PR&name_1=Fast+test PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
Fast test builds without SSL and failed on the unconditional 'openssl/evp.h' include in CasBlobHashingWriteBuffer.cpp. Wrap the OpenSSL-backed Sha256 hashing write buffer and the one-shot digest in '#if USE_SSL'; on non-SSL builds selecting blob_hash = 'sha256' now fails closed with SUPPORT_IS_DISABLED. CityHash128 and XXH3-128 blob hashes are unaffected. PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 18, 2026
… test regression) CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=aeb13b24394023fa8cd9d310d4cbcbc308380af1&name_0=PR&name_1=Fast+test PR: #2073 A CAS parser commit grouped `RELOAD_DICTIONARY`/`RELOAD_MODEL`/ `RELOAD_FUNCTION` with `CONTENT_ADDRESSED_GARBAGE_COLLECTION` into a format case that prints only the optional disk, dropping the reload targets: `SYSTEM RELOAD MODEL my_model` formatted as `SYSTEM RELOAD MODEL` (failed 04117_parser_system_query_variants and 04124_parser_system_query_extra in Fast test). Fold all four types back into the generic target-printing case (table / target_model / target_function / disk else-if chain) — for the CA GC command the disk branch produces the identical output. Both stateless tests verified locally via clickhouse-local against their references. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 18, 2026
…als (arm_tidy, T13 batch 1) CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=aeb13b24394023fa8cd9d310d4cbcbc308380af1&name_0=PR&name_1=Build+(arm_tidy) PR: #2073 Removes default arguments from all virtual/override methods flagged by `google-default-arguments` (147 sites: `CasBackend.h` interface, `IObjectStorage.h`/`S3ObjectStorage.h`, all backend implementers, test helpers/fixtures) and adds non-virtual convenience overloads on the base classes that forward the previous default values. Derived classes gain `using` declarations to unhide the base overloads. Qualified parent-implementation calls in test fault backends switched to the explicit 3-arg form — the 2-arg form would now route through the base forwarder and re-enter the derived override virtually (double fault injection; caught by the battery). Bulk edits produced by codex (gpt-5.6-luna) per the T13 brief; overload visibility and qualified-call fixes plus verification by Claude. Battery 919/919 green. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 18, 2026
CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=aeb13b24394023fa8cd9d310d4cbcbc308380af1&name_0=PR&name_1=Build+(arm_tidy) PR: #2073 Semantics-preserving conformance for the remaining flagged classes: readability-container-contains, readability-isolate-declaration, google-runtime-int (AWS SDK retry-API overrides keep `long` with targeted NOLINT — the override contract owns the type), readability-duplicate-include, cppcoreguidelines-init-variables, cert-msc, modernize-raw-string-literal, modernize-use-starts-ends-with, bugprone-empty-catch (comments only — no new behavior), googletest naming, bugprone-argument-comment, bugprone-optional-value-conversion, bugprone-misplaced-widening-cast (CasTypes.h site audited: not a real precision bug — the value is range-validated to 0-5; cast made explicit without value change). CasRefCowMap's own `contains` keeps its `find` with NOLINT (self-recursion). Bulk edits by codex (gpt-5.6-luna) per the T13 brief (.superpowers/sdd/task-13-batch2-report.md); one over-removed include (PartFolderAccess.h) restored and verification by Claude. Battery 919/919. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 21, 2026
…n't abort the server (STID 0883) CI PR#2073 (content_addressed storage lanes) crashed with "Too large size (9223372036854775870) passed to allocator" (LOGICAL_ERROR, server abort) running the regression test 04070_no_crash_extreme_compress_block _size. Root cause: an extreme max_compress_block_size (2^63-1) flows into ContentAddressedTransaction::writeFile's buf_size and, unclamped, reaches the CaContentWriteBuffer base-class allocation (Memory::alloc), where checkSize (>= 0x8000000000000000) fires. The ordinary MergeTree writers clamp compress -block sizes to 256 MiB (MergeTreeWriterSettings::MAX_COMPRESS_BLOCK_SIZE) for exactly this reason; the CAS write path received the value unclamped. Fix: clamp buf_size and adaptive_write_buffer_initial_size to 256 MiB at the CAS allocation site (both CaContentWriteBuffer ctors), mirroring the ordinary clamp. New gtest CasContentWriteBuffer.ExtremeBufferSizeIsClampedNotPassedToAllocator reproduces the exact crash number without the clamp (verified RED) and passes with it. CA gtest gate 1057/1057. CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=latest&name_0=PR Related: #2073
filimonov
added a commit
that referenced
this pull request
Jul 21, 2026
…cle (UNMOUNT/FSCK) Design for STID 3982-3b48 (CI PR#2073). Five parts: (1) MountLeaseKeeper's vanished-backing-store case throws FILE_DOESNT_EXIST (not LOGICAL_ERROR) so a background renewal thread never aborts the server + new CasMountLeaseLost counter; (2) SYSTEM CONTENT ADDRESSED UNMOUNT <disk> (shutdown() the pool, auto-remount on next access) as a clean alternative to rm -rf under a live mount; (3) online SYSTEM CONTENT ADDRESSED FSCK <disk> via read-only runFsck(Pool&); (4) rewrite the no-leftovers test teardown to GC RUN -> FSCK -> UNMOUNT -> rm -rf; (5) rename the offline clickhouse-disks fsck applet to ca-fsck for consistency. Auto-teardown-on-DROP stays out of scope. Related: #2073
filimonov
added a commit
that referenced
this pull request
Jul 21, 2026
…ence Task 7 added the SYSTEM_CONTENT_ADDRESSED_FSCK AccessType but only updated the AccessType.h macro list, not the 01271_show_privileges reference — the test enumerates every privilege, so the new row (right after MOUNT, matching the AccessType declaration order) was missing and fasttest failed with a one-line diff. 01271 was not in the local gtest/FSCK-access test runs, so the gap surfaced only in CI. CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=d780fa7473807203916125e96cf73a9774c84464&name_0=PR&name_1=Fast%20test PR: #2073 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKgdqVjZwkpWPxLyHzduPb
filimonov
added a commit
that referenced
this pull request
Jul 24, 2026
Add two DESIRABLE items under §3 (GC correctness/observability) surfaced by triaging the CAS-s3 stateless run (Altinity PR #2073, run 30019911967): - [RECOVERED-INDEGREE-ATTRIBUTION] the "delete_pending recovered in-degree — structurally impossible … investigate" warning is a false alarm (dedup-adopt vs condemn TOCTOU, spared, no data loss). Downgrade the GC log to a ProfileEvent + Debug and move the real adopt-without-resurrect detector to the writer's edge-commit. - [CONDEMN-GRACE-WINDOW] a cool-down before condemning a just-zeroed blob to kill hot-dedup churn (tiny system-log blobs) at the source; flagged higher-risk (condemn-timing/ack-floor, TLA-gated, protocol-veto). Docs-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
filimonov
added a commit
that referenced
this pull request
Jul 24, 2026
…GABRT) CI-confirmed on Altinity PR #2073 (run 30019911967, asan_ubsan CAS-s3 stateless): a mount-lease renewal PUT that times out client-side (ambiguous — may have applied server-side) gets soft-retried with a stale token, mismatches against its own bumped body, and falls through MountLeaseKeeper's classifier (same uuid/epoch, unfenced — none of the 3 cases match) into the base class's generic LOGICAL_ERROR, aborting the server under ASan. A third variant of the STID 3982-3b48 family (parts 1a/1b covered vanished/absent-at-release). Also notes: amd_msan/amd_tsan CAS-s3 stateless jobs in the same run hit the 6h job timeout with zero artifacts — unknown if same crash-loop or a plain hang. Docs-only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
filimonov
added a commit
that referenced
this pull request
Jul 27, 2026
The unsharded CA-s3 sanitizer lanes do not fit the 6h GitHub job timeout, and the kill lands after the test loop but before result upload, so the lane reports NO artifacts at all and praktika's aggregate stays RUNNING forever. In run 30203355812 (sha e2d04bf): tsan finished all 10990/10990 tests at 5h59m and was killed 16s later during teardown; msan was at 5824/10990 (53%, steady progress, no hang) when killed; asan_ubsan passed at 5.4h — too close. This is the full explanation of the "msan/tsan CA-s3 cancelled at exactly 6h with zero artifacts" pattern seen in three consecutive runs. Shard asan_ubsan and tsan 2 ways and msan (the slowest, ~11h projected) 3 ways, following the existing "amd_tsan, s3 storage, parallel, N/M" convention; ci/jobs/functional_tests.py already parses the N/M batch token generically. Workflow YAML regenerated with praktika yaml. Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=e2d04bfe37eec5af58bde837e1cc89d11016ef6f&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
filimonov
added a commit
that referenced
this pull request
Jul 27, 2026
…irectory The refusal in prepareAdoptFromManifest throws LOGICAL_ERROR, which aborts the whole process in debug/sanitizer builds instead of behaving like a catchable exception, so the EXPECT_THROW form killed unit_tests_dbms on all three sanitizer CI lanes (asan_ubsan/tsan/msan, 0s failures with "Logical error: 'Relink target ... does not address a content-addressed part directory of a live table'"). Split it the same way as the CasWiringOpsDeathTest precedent in this file: EXPECT_THROW stays for plain release builds, and a CasWiringExchangeDeathTest EXPECT_DEATH variant proves the same refusals positively abort under DEBUG_OR_SANITIZER_BUILD. Verified: build_asan runs the death test (1 test, OK), build (release) runs the EXPECT_THROW variant (9 tests from 2 suites, all OK). Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=e2d04bfe37eec5af58bde837e1cc89d11016ef6f&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
filimonov
added a commit
that referenced
this pull request
Jul 27, 2026
The pre-scrape cleanup removed only filesystem_caches/*/status (one level), but dynamic cache disks created by tests nest their path — the binary CA-s3 lane died on filesystem_caches/disks/cache_03517/status with "Cannot lock file ... Another server instance in same directory is already running" (StatusFile.cpp flock, EWOULDBLOCK) raised from FileCache::initialize when clickhouse-local initialized the disk map. Make the glob recursive so any depth is covered, same mechanism as before: removing the file lets clickhouse-local create and lock a fresh inode even when the not-fully-stopped server still holds the old one. Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=e2d04bfe37eec5af58bde837e1cc89d11016ef6f&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
filimonov
added a commit
that referenced
this pull request
Jul 27, 2026
…lanes Tag it no-content-addressed-storage: the coverage is the inline content-addressed disk the test creates itself, so it stays meaningful on every ordinary lane. On lanes whose DEFAULT MergeTree storage is content-addressed, system.remote_data_paths (no disk_name pushdown — the applyFilters TODO in StorageSystemRemoteDataPaths.cpp) also walks the huge shared default pool with the whole run's data, and on the S3 (RustFS) variant that walk exceeds the 600s test timeout (recurred in two consecutive runs, plus a "Some queries hung" ride-along). The BACKLOG pushdown item stays open as the real fix; also note the ConnectionGroup Disk-session pressure signal from the same run. Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=e2d04bfe37eec5af58bde837e1cc89d11016ef6f&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
filimonov
added a commit
that referenced
this pull request
Jul 27, 2026
Same class as 5f292e0: PreparedPartWrite's one-shot-terminal rejection throws LOGICAL_ERROR, which aborts the whole process in debug/sanitizer builds (Exception.cpp's handle_error_code) instead of behaving like a catchable exception, so CasPartFolderAccess.PreparedPartWriteRejectsASecondTerminal killed unit_tests_dbms on all three sanitizer CI lanes (0s failures). The file's author knew the class (the NETWORK_ERROR/MEMORY_LIMIT_EXCEEDED choices further down cite it) — these two spots were the leftovers: - PreparedPartWriteRejectsASecondTerminal: split per the CasWiringOpsDeathTest precedent — expectThrowsCode stays for plain release builds, a CasPartFolderAccessDeathTest EXPECT_DEATH twin proves the same rejections positively abort under DEBUG_OR_SANITIZER_BUILD. - PreparedPartWriteMoveTransfersTheTerminalDuty: the single moved-from abort check is guarded in place (EXPECT_DEATH forks, so the rest of the test continues in the parent). A sweep of all CAS gtest files for unguarded LOGICAL_ERROR throw expectations found no other gaps (blob_upload_pool, ref_writer, ref_install_safety, upload_detached, upload_fanout all carry guards). Verified: build_asan 39/39 with both death tests OK; build (release) 30/30 with the EXPECT_THROW variants. Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=8417137aeb842f463066fd68a75b60ffeae30d63&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
filimonov
added a commit
that referenced
this pull request
Jul 28, 2026
CasRefInstallSafetyDeathTest.DenyGuardStopsAnAllocation dispatched on DEBUG_OR_SANITIZER_BUILD, but DENY_ALLOCATIONS_IN_SCOPE itself is gated on MEMORY_TRACKER_DEBUG_CHECKS (MemoryTracker.h: defined only under !NDEBUG). Sanitizer builds define NDEBUG, so there the guard compiles to static_assert(true) and the death test "failed to die" on all three sanitizer CI lanes — visible verbatim in the CI error output. (These lanes only now reached this test at all: the earlier LOGICAL_ERROR aborts in gtest_ca_wiring / gtest_cas_part_folder_access killed the process before it, so each fix un-shadows the next tail failure.) Gate the death test on MEMORY_TRACKER_DEBUG_CHECKS instead, and drop the throw-only EXPECT_ANY_THROW branch as dead code: MEMORY_TRACKER_DEBUG_CHECKS implies !NDEBUG implies DEBUG_OR_SANITIZER_BUILD, so whenever the guard exists its LOGICAL_ERROR aborts — death is the only observable outcome (the old comment claimed the opposite implication). Verified: build_debug runs the death test (OK, guard fires); build_asan compiles it out, remaining CasRefInstallSafety* 21 tests x5 repeats green. Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=07f8398acddff2c676aded5f804f122984845a4a&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
filimonov
added a commit
that referenced
this pull request
Jul 28, 2026
RCA of the fence cascades on the CA-s3 sanitizer lanes (run for 07f8398, msan 2/3 lane): clickhouse-server logged 351 bursts of 'Connection refused' (e.code() = 111) to rustfs at localhost:11121 spread over the whole run — the service stayed up between bursts, so this is fd/accept exhaustion, not a crash. rustfs was launched WITHOUT the open-files-limit raise that start_azurite performs for exactly this failure mode, while the server under parallel sanitizer load holds 10k+ active Disk-group S3 sessions. Most bursts were absorbed by the CAS request retries (max_attempts=16 / 90s deadline); the one at ~02:00-02:01 outlived (a) in-flight writes' 90s budgets (Code 210 UNCERTAIN) and (b) the mount-lease renewal confirm window (TTL 30s), so the mount fence tripped (fail-closed by design) and every writing test failed with Code 668/210 from 02:02:52 until remount completed ~02:05:00, with straggler failures to ~02:10 — ~40 of the run's 66 test failures. Also add rustfs.log to the uploaded artifacts: it was already written to ci/tmp/rustfs.log but never uploaded (azurite/kafka/minio logs are), which was exactly the missing evidence for this class of triage. Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=07f8398acddff2c676aded5f804f122984845a4a&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
ilejn
reviewed
Jul 28, 2026
|
|
||
| ```sql | ||
| SYSTEM CONTENT ADDRESSED GC RUN [ON CLUSTER cluster_name] [disk_name] | ||
| ``` |
filimonov
added a commit
that referenced
this pull request
Jul 30, 2026
… razor RefWriterStalePrecommitSweep.BoundedBatchesAndInterruptionResumeAcrossMounts failed on 5 of 6 sanitizer-lane runs across two CI rounds (asan/tsan/msan, 'refLaneWedgedForTest — Actual: false, Expected: true') while passing every quiet local run. Reproduced locally 14/14 by adding full CPU load with the test otherwise unchanged and filtered alone — so not test-ordering contamination, pure timing. Root cause: the test set attempt_timeout_ms == operation_deadline_ms == 100, which turns the request controller's pre-send gate (putIfAbsentControlled: 'now + attempt_timeout > deadline' returns Unresolved WITHOUT sending) into a zero-width race — it only passes when no millisecond tick elapses between the deadline capture and the gate. This test uniquely burns that window encoding the ~1700-op removal chunk, so on a loaded or sanitizer-slow machine the gate fired first, the injected ambiguous fault was never reached, the sweep failed CLEAN — nothing sent, nothing ambiguous, and the product CORRECTLY did not wedge the lane. The test had over-specified the failure mode it would meet. Ambiguity is guaranteed by max_attempts = 1 alone; widen operation_deadline_ms to 5000 so the PUT is always actually sent and the wedge is deterministic on any machine. Verified: 20/20 green (5 repeats x 4 sweep tests) on ASan under half-core CPU load; the sibling wedge tests keep the tight budget deliberately — their capture-to-gate window is empty. Report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=834c9517f56a83927b0c941ac08d8dfd82460579&name_0=PR PR: #2073 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKmSZa7T87WbRGKkNkSXky
svb-alt
added
antalya
antalya-26.6
roadmap
strtgbb
requested changes
Jul 31, 2026
filimonov
added a commit
that referenced
this pull request
Aug 3, 2026
… (PR 2073 review) Addresses the three review recommendations from strtgbb on #2073 - All 10 CAS `Job.ParamSet` entries move out of `JobConfigs.functional_tests_jobs` into a new `AltinityJobConfigs.cas_functional_tests_jobs`. Every workflow that picked them up implicitly -- including `release_builds` (`binary` filter), `release_branches` and `backport_branches` (`asan` filters) -- now concatenates the two lists in the original order, so the job sets are unchanged. Proven by an empty `diff -r` of the regenerated `.github/workflows` and by an identical dump of `workflow.jobs` names for every `ci/workflows/*.py` module before and after. - The CAS ParamSet comments are cut to the load-bearing constraints: RustFS rather than MinIO OSS for enforced conditional deletes, and sharded sanitizer lanes because an unsharded one exceeds the 6h GitHub job timeout. - The CAS comments in `ci/jobs/scripts/clickhouse_proc.py` are compressed the same way, keeping why the disks are opened read-only, why the substitution is keyed on the `<metadata_type>cas</metadata_type>` marker, and why `grep -R` and `sed --follow-symlinks` are required. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
… Task 14) Persistent agent working guide absorbing INTENT.md, 02-methodology.md and durable cross-session memory items; deletion of the absorbed docs is now gated on its existence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…beat both others Third shape, and on this analysis the strongest: drop the If-Match from the local-source resurrect and write unconditionally, exactly as the S3-staging arm of the same branch already does. What the condition buys turns out to be narrower than it looks. It protects no durable reference -- the part manifest addresses blobs by content hash and contains no token at all, and two racing resurrections write equivalent bodies. It does not provide INV-NO-RETURN either; the fresh incarnation tag does, by changing the bytes and therefore the ETag so GC's queued exact-token delete misses. What it does buy is the economy of adopting a winner's incarnation instead of re-uploading, and the ability to tell a racing displacement apart from a GC delete. The reason it may be best: the single-part cap applies only to CONDITIONAL writes, because that is where GCS drops the precondition. An unconditional streaming resurrect would be size-independent on every backend, and the GCS guard would stop being needed for this path at all. Recorded with what must be measured first -- how often the fan-out actually races the same condemned blob -- rather than adopted on the spot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…l-overwrite design The first design added a conditional streaming overwrite. It worked, but it paid the condition's full cost -- a new seam method with a refusal path, overrides in twelve backends, a `declared_size` parameter, and a GCS guard -- for a benefit that turns out to be one saved re-upload on a lost race. The decisive evidence was already in the tree. `resurrectStaged`, the S3-native staging arm of the SAME branch, writes unconditionally, and its comment states the reasoning verbatim: an `If-Match` on the condemned token "would only save a redundant re-upload on a lost race, never prevent data loss". INV-NO-RETURN comes from the fresh incarnation tag, not from the condition; durable references address blobs by content hash and name no incarnation at all. The method's name also misleads, and the spec says so: `resurrectStaged` performs no server-side copy -- it reads the staging object, streams it through the client, and writes with default settings. The server-side copy is in `promoteStaged`. So the two arms differ in exactly one thing: where the reader comes from. Making the reader a parameter collapses them into one operation, and because an unconditional write needs no precondition, it takes the multipart path on GCS too -- the ceiling this path had stops existing rather than being documented. Order of work puts the revert first so the branch never carries two designs at once, and flags one piece of the reverted work to re-apply on its own merits: the explicit upload abort on a refused conditional write, which `putIfAbsentStream` still needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
Task 1 is the revert, and it lands green on its own so the branch never carries two designs at once. It also calls out the one piece of the reverted work worth re-applying separately: the explicit upload abort on a refused conditional write, which the streaming CREATE still needs and which would otherwise vanish with a design it did not belong to. The middle three tasks are ordered so each is independently testable: the source becomes a reader factory (behaviour-preserving), the seam takes a reader instead of a staging key (behaviour-preserving for the staging arm), and only then does the local arm switch and the materialization disappear. Task 5 pins a property rather than changing behaviour: its test passes on arrival, because it exists to stop a later consistency-minded refactor from routing the resurrect back through conditionalWriteSettings and silently restoring a ceiling that no longer applies. The implementer notes lead with INV-NO-RETURN, because the cheapest-looking shortcuts on this path -- reusing the source header, server-side copying the condemned object -- reproduce the condemned ETag and let an already-queued exact-token delete destroy the live resurrection, which no test without a GC round afterwards would catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…ending the body" This reverts commit 784308d.
This reverts commit 531adee.
…s refused Losing the condition is an ordinary outcome on the streaming create path, not an error: another writer legitimately took the slot. Left to the write buffer's destructor it logs "was neither finished nor aborted" every time -- which any test whose server writes to stderr turns into a failure -- and leaves uploaded parts billable until a lifecycle rule reaps them. Found while building the conditional-overwrite design that was since reverted; it stands on its own, because `putIfAbsentStream` loses conditions too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
Every producer already had a reader behind its callback: the production one opened ReadBufferFromFile and copied, the test ones wrote a retained string. Pushing the copy loop into each producer meant a source could only be consumed by something willing to be written INTO -- which is why the two resurrect arms could not share an implementation even though they write the same bytes. A factory rather than one buffer because this path retries: it re-uploads after the object vanishes and re-decides after a lost race, and each attempt must read from the start. The string-backed sources use ReadBufferFromOwnString, not ReadBufferFromString: the latter only BORROWS the bytes it is handed. Today's captures outlive every call, so the borrowing form would work -- and would be a dangling read the first time someone shortened a lifetime. The owning copy costs nothing here, where this helper exists for inline payloads and tests. Behaviour is unchanged; the touched suites pass 103/103. Gates: release and ASan both green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
The method never performed a server-side copy despite its name: in Native mode it opened the staging object, skipped its envelope header, and streamed it through the client into an unconditional write. The server-side copy is `promoteStaged`, the ordinary write-once create. So the two resurrect arms differed in exactly one thing -- where the reader came from -- and making that a parameter collapses them into one operation. The payload offset moves to the caller, which is what knows the staging envelope's shape. One test assertion had to move rather than be dropped. `CASS3Staging` proved "the resurrect reads the staging object, never the condemned blob key" by inspecting the source ARGUMENT; with the caller opening the reader there is no such argument. It now counts what was actually read via `getStream` and asserts the staging key was read and the condemned key was not -- an assertion about I/O rather than about a parameter the backend was merely told, which is the stronger form. `get` is deliberately not counted: the surrounding assertions use it to inspect bodies. Release gate: 2027/2027. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…ource reader The local arm was the last caller that had to materialize a blob body: putOverwrite took a String, so a body larger than memory could not be resurrected at all. It now streams through the same Backend::resurrect the staging arm uses, and drops the condition for the same reason that arm always did -- an If-Match on the condemned token saves a redundant re-upload on a lost race and prevents no data loss. What protects the resurrection is the fresh incarnation tag, which makes the new body's ETag differ from the condemned one so every already-queued exact-token delete misses. The size check moved rather than disappeared: the whole-body path compared what the source produced against source.size BEFORE writing; the streaming path HEADs the incarnation it just wrote and compares lengths, so a source that lies about its size still cannot publish a body that disagrees with the manifest entry naming it. The new open-count test pins TWO opens for the present-condemned shape and says why each bound matters: open #1 is the ordinary conditional-create attempt (streamed, refused at finalize), open #2 is the resurrect. More would mean a hidden materialization pass crept back in; fewer would mean the create attempt stopped streaming -- a protocol change, not an optimization to make silently. Release gate: 2027/2027. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
It existed because one branch could not stream. That branch streams now, so the aggregate it rationed no longer accumulates. Its guarantee was max(capacity, largest single body) and the second term was unbounded, so it never protected against the case that motivated this work -- one blob larger than memory. The two tests that drove it are deleted rather than ported: they asserted that an aggregate is not exceeded and that an overweight admission does not starve, and neither property exists once nothing is admitted. `cas_condemned_upload_memory_bytes` is deleted with no compatibility shim -- pre-release, no persisted data, and an accepted-and-ignored setting reads as still doing something. One deletion here was initially too greedy, and the full gate caught it: gtest_cas_blob_upload_pool_env.cpp looked like the admission's test scaffolding but is the global gtest listener that (a) brings the blob UPLOAD POOL up before every test -- its getter is fail-loud, so 46 tests across 10 suites threw without it -- and (b) shuts that pool down in OnTestProgramEnd, without which the binary deadlocks at exit: gtest_main joins the GlobalThreadPool whose std::threads are occupied by pool workers that only return when the pool is destroyed. The file is restored with only its admission lines removed. Release gate after the restore: 2027/2027. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…tes the cap binds; prune the ledger The regression test locks in what a consistency-minded refactor would silently break: routing the resurrect through conditionalWriteSettings would reintroduce a single-part ceiling this path no longer has. The three user-facing pages now say which writes the cap actually binds -- the conditional write-once create -- instead of implying it bounds every write on GCS. The displacement-alternatives BACKLOG item is removed as resolved: the question it asked was settled by landing the unconditional design. The GCS-rethink item is narrowed to the one path still capped, the conditional create, with the overwrite discussion marked historical. Both design docs and both plans are deleted -- the code and its comments now carry the reasoning, and a superseded spec left in the tree reads as an open question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
An adversarial review of the increment (verdict REJECT) found two real defects, both invisible to the unit gate because it exercises InMemoryBackend: 1. The size check ran AFTER publication. Moving the source.size comparison from before the write (the old whole-body path counted into the String) to a post-write HEAD left a window where a source truncated after hashing publishes a short body as the CURRENT incarnation; the check then throws, but a racing writer that already resurrected correctly proceeds to flip the meta Clean and promote a tokened dependency without revalidation -- the committed manifest names a truncated body. The old pre-write count let nothing publish at all; the post-write HEAD could also inspect a racing writer's incarnation rather than our own. Fixed by passing `payload_size` into `Backend::resurrect`: the write counts while streaming and, on a mismatch, cancels the upload BEFORE finalize -- nothing becomes current on any backend. 2. Local content-addressed disks could no longer resurrect at all. Local object storage runs `ObjectStorageBackend::Mode::EmulatedSingleProcess`, and the new `resurrect` threw NOT_IMPLEMENTED for every non-Native mode, where the `putOverwrite` it replaced worked. A condemned blob on a local CA disk became unrepairable, and no InMemoryBackend test could notice. Fixed with an emulated implementation serialized by the same `emu_mutex` as every other emulated conditional op. Both regression tests live in the CONTRACT suite, which is parameterized over InMemoryBackend and the real ObjectStorageBackend in emulated mode -- so the mode that hid defect 2 now runs them: `ResurrectWrongSizePublishesNothing` and `ResurrectReplacesBodyAndMintsFreshToken` (the latter also re-pins INV-NO-RETURN through the queued exact-token delete missing the fresh incarnation). Also from the review, the contract prose no longer claims "never overwrites a live blob": two racing resurrections may both run and the loser overwrites the winner's FRESH incarnation -- accepted, not prevented, because payloads are content-identical and durable references name content hashes. The arm comment and the backend doc page now say what is actually guaranteed: a live incarnation of DIFFERENT content is unreachable by the content address itself. Release gate: 2032/2032. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…ERROR test rule at the gate table Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…tyle The three edits carried paragraph-length mechanism recaps into tables whose every other row is one phrase. The mechanism lives in the code comments; the tables state the contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
INV-NO-RETURN / INV-NO-LOSS / INV-NO-DANGLE are development shorthand: to a reader of the public documentation they are unexplained noise. Each occurrence is replaced by the plain statement of what the invariant guarantees, or removed where the sentence already said it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
A source that yields a different byte count than it declared is INPUT -- a staged file truncated after hashing -- not an impossible program state. LOGICAL_ERROR also aborts sanitizer builds, which the ASan gate demonstrated by dying on the guard's own regression test and hiding everything after it. CORRUPTED_DATA states what happened and stays a catchable outcome everywhere. (The change was in the tree when the previous commit's gates ran green; committing it separately was missed and caught by re-review.) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
The second review round caught a guarantee lost in the admission's deletion: the Native resurrect
streams, but the emulated one materializes the whole body, and the upload fan-out runs N resurrect
tasks in parallel -- so a local-disk INSERT resurrecting several large condemned blobs could hold the
SUM of their sizes in RAM. The deleted semaphore bounded exactly that.
Restored as the smallest equivalent: emulated resurrections are serialized by their own process-wide
mutex, so the peak is the largest single body -- the same bound the semaphore's exclusive arm gave.
A dedicated mutex rather than emu_mutex, because the drain may read through the same store. The three
prose sites that contradicted the implementation (the seam promising "never materialized", the header
still claiming NOT_IMPLEMENTED for emulated resurrect, the doc page describing the deleted semaphore)
now say what actually happens.
The proper fix is recorded as debt in BACKLOG {#emulated-resurrect-spill-to-disk}: this backend sits
ON a local disk, so the staging area for a body of any size is a scratch file, not RAM -- spilling
would remove both the materialization and the serialization. Kept as debt rather than done now
because no production deployment runs CAS over local paths.
Release gate: 2030/2030.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
… drop dead setting externs The third review round approved the increment with one prose finding: seven sites still described the pre-serialization world. Two were live code residue -- the extern declarations of the deleted `cas_condemned_upload_memory_bytes` in Server.cpp and LocalServer.cpp (unused symbols, so no linker caught them) and a DisksApp comment describing the deleted admission's derivation. Five were claims that resurrection "streams and never materializes" stated backend-independently; each now says what is true where -- streamed on remote object storage, materialized one body at a time under serialization on the local emulated mode. The review also answered the two safety questions asked of it, with the reasoning worth keeping: the lock graph is acyclic (no path holding `emu_mutex` acquires the resurrect mutex, and emulated `getStream` releases `emu_mutex` before returning), and pool saturation under serialized resurrections is head-of-line blocking, not deadlock -- the mutex owner never waits on queued fan-out work, so its completion always frees both the mutex and a pool slot. Release gate: 2030/2030. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…mmit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
It rode into the CAS branch after a CAS test tripped the class it detects, but it is a generic check in a shared upstream file -- one more line of diff noise in an already loud PR. The check itself is kept aside untracked for a standalone upstream submission. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
Every workflow carried its own splice of upstream lanes plus the CAS lanes -- six copies of the same composition, each a place to forget when the next lane is added. The composition now lives once in AltinityJobConfigs.functional_tests_jobs (a NEW list; altinity_jobs.py is ours, so no upstream file grows CAS knowledge), and each workflow references it with a one-token change. A per-file `JobConfigs.functional_tests_jobs += ...` was considered and rejected with evidence: praktika's mangle loads every workflow module in ONE process, so mutating the shared upstream list would append the CAS lanes once per loaded workflow -- later-loaded workflows would schedule duplicates, and the result would depend on directory iteration order. Equivalence proved, not assumed: the sorted job-name list of every one of the six workflows is byte-identical before and after (loaded standalone, the way praktika loads them); dups=0 in the all-modules-one-process mode as well. pull_request.py's local FUNCTIONAL_TESTS_JOBS list dissolved into the shared one, shrinking its diff against upstream from five edit sites to token swaps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…age-b-remaining) Stage B is complete; the per-plan workspace with its briefs, reports, and evidence bundles is no longer needed on the branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…cessor test Upstream commit bfe5988 ("Iceberg type 'time' support") changed the getSimpleType mapping of the Iceberg `time` type from Int64 to Time64(6), but the test kept the old expectation. The mismatch is inherited from altinity/antalya-26.6 and fails `Unit tests (asan_ubsan)` on every PR against that base. CI: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2159&sha=056488b47a0b3f32ea2ebf83d508b697ff4c0c17&name_0=PR&name_1=Unit%20tests%20%28asan_ubsan%29 PR: #2159 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…on` release at query finish `BlockIO::onFinish` released the whole set of workload resources — including the `MemoryReservation` — before the query pipeline was finalized, while pipeline threads still hold raw pointers to it through `WorkloadResources` in `PipelineExecutor`. Split the release: the query slot is released early (safe, pipeline threads never touch it), the memory reservation only after the pipeline has been finalized and its threads joined. `BlockIO::reset` resets the pipeline before releasing the resources for the same reason. The memory reservation feature (ClickHouse#82414) was backported to 26.6 in `2098704e6d1` on 2026-06-27, but this fix never followed: its cherry-pick ClickHouse#108562 was closed automatically on 2026-07-06 with unresolved conflicts. As a result 26.6 reports the race in `Stress test (amd_tsan)`, for example https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=112727&sha=6120a6fb27efe0d740c8f075aee499215d2f23e1&name_0=BackportPR&name_1=Stress%20test%20%28amd_tsan%29 (cherry picked from commit 9e801ca)
Member
Author
|
TSan: data race + Server died (report) Race on |
…ection stays cheaper The final check asserts the optimizer selects the projection under force_optimize_projection = 1. With randomized MergeTree settings (index_granularity_bytes = 206048, index_granularity = 27075, merge_max_block_size = 5006, enable_block_offset_column = 1 — minimized by the CI randomized-settings diagnosis) the projection needed 202 marks vs 201 for the base table, the optimizer picked the base, and the check threw PROJECTION_NOT_USED. The test is about the multi-block projection rebuild on a content-addressed disk, not the cost model, so pin the default granularity in the CREATE. CI: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=9b5cbc314bc4eeb3d1b3d3017a34cd3816f59e60&name_0=PR&name_1=Stateless+tests+%28amd_debug%2C+parallel%29&name_2=Tests PR: #2073 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
The "proven absent" presence probe and the cold-reader admission both required two whole-catalog reads to be byte-identical (token + content) before trusting an observation. The catalog is pool-global, so under a parallel workload the condition almost never holds and both paths starve: in CI, `01069_database_memory` failed 193 of 194 retries with "catalog changed while probing table-root cleanup completeness" on its `deduplication_logs` namespace. Each catalog read is one token-CAS'd full value, so this namespace's row is what the proof actually needs: - presence probe: row absent in BOTH reads => proven absent (linearized at the second read); a row that appears in between answers present; - cold reader: revalidate THIS row by value; unrelated churn admits, an own-row change still forces a fresh observation. The second cut now runs the ambiguity validation explicitly -- an aliasing incarnation admitted between the reads left the target row byte-identical and was only refused by the whole-catalog comparison it replaces. Two tests pinning whole-catalog stillness updated to the per-row contract; regression tests added for unrelated-churn starvation and the aliasing incarnation. CI: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=95ba719fe763e3d87a3ef4555b9e1b310c768d0c&name_0=PR&name_1=Stateless+tests+%28amd_binary%2C+cas+storage%2C+parallel%29 PR: #2073 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
content addressable storage - draft PR
Documentation entry for user-facing changes
TBD.
Exclude tests:
Regression jobs to run: