[WIP] CAS draft (adopting to CI/CD, not for review / merge) - #2073
Draft
filimonov wants to merge 4158 commits into
Draft
[WIP] CAS draft (adopting to CI/CD, not for review / merge)#2073filimonov wants to merge 4158 commits into
filimonov wants to merge 4158 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
…it-message claim; undecided short keys) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159e4eaPFKDFgxG83d5VL73
# Conflicts: # .github/workflows/pull_request.yml # src/Disks/DiskObjectStorage/DiskObjectStorageTransaction.cpp
…cannot work Second shape considered for displacing a condemned blob: leave the body alone and rewrite its per-hash meta -- back to Clean, or at a token that does not exist -- so the queued delete matches nothing. It fails structurally, which is worth writing down because it is the shape reached for first. The blob delete site issues deleteExact with the token from GC's OWN retired pipeline, fixed at condemnation; the meta is never consulted there. Rewriting it cancels nothing and leaves the meta asserting health for a body that is about to disappear -- the dangling state the exact-token protocol prevents. The design already states the only sanctioned defence: a stale delete is safe because the object is absent or a writer CHANGED ITS INCARNATION TOKEN. A token is an ETag, a function of the bytes, and a copy onto itself reproduces both -- so a new token cannot be minted without writing content. Token change and body write are one operation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
The conditional overwrite had only a whole-`String` form, so its one blob-body caller had to materialize the body to write it. The condition was never the obstacle: it rides on the write buffer, `WriteBufferFromS3` already applies `If-Match` on both `PutObject` and `CompleteMultipartUpload`, and the existing whole-`String` path already goes through the same `writeObject`. Only the caller's input shape was. `declared_size` is required rather than advisory because a generation-token store cannot condition a multipart completion at all and must refuse from the size before sending anything; the guard itself lands next. Two things this touched that the shape of the change made unavoidable: A refused conditional write now cancels its upload explicitly instead of leaving it to the write buffer's destructor. For the streaming CREATE that was tolerable -- losing a create race is rare. For a conditional OVERWRITE it is not: the caller races a displacement it fully expects to lose sometimes, and the destructor path logs "was neither finished nor aborted" on every occurrence, which any test whose server writes to stderr turns into a failure, while the uploaded parts stay billable. Adding a pure virtual made every `Backend` implementation abstract at once -- not three, but twelve, nine of them test doubles across five files. They delegate; two classes that derive from `InMemoryBackend` rather than `Backend` correctly needed nothing. Gates: release 1993/1993, ASan 1998/1998. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…he body The limit is neither new nor ours to lift here. GCS honours no precondition on multipart completion -- it completes the upload and drops the condition, turning a write that had to be refused into a silent overwrite -- so conditional writes on a generation-token store are forced into one part, and one part is bounded. A body above that bound cannot be conditionally written at all. What changes is when the caller finds out: from the declared size, before the first byte, instead of from a storage error after the whole body has crossed the network. The message names the size, the budget, the setting, and the reason, because an operator meeting this needs to know it is a property of the backend rather than a misconfiguration they can tune away. The two tests that pass WITHOUT the guard are deliberate: exactly at the cap must still be allowed, and an ETag store must keep having no ceiling at all -- the cap exists only because of the generation dialect's multipart gap. The refusal test asserts the setting name appears in the message, not merely that something threw, so the text cannot decay into an unattributable "bad argument". Gates: release 1993/1993, ASan 1998/1998. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1r6FXkwCGRmniKKVfs2ZN
…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
Member
Author
|
TSan: data race + Server died (report) Race on |
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
5 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: