Repository navigation
Conversation
4e107a1 to
c7c09a1
Compare
…next commit) --- Original cherry-pick message follows: Merge pull request #2159 from Altinity/feature/antalya-26.6/CAS [experimental] CAS (Content-addressed storage) over shared object storage for antalya-26.6 # Conflicts: # .github/workflows/fast_builds.yml # .github/workflows/master.yml # .github/workflows/pull_request.yml # .github/workflows/pull_request_community.yml # ci/jobs/functional_tests.py # ci/jobs/scripts/clickhouse_proc.py # ci/jobs/scripts/clickhouse_service.py # ci/workflows/pull_request.py # ci/workflows/pull_request_community.py # ci/workflows/release_branches.py # docs/concepts/features/configuration/server-config/storing-data.mdx # programs/disks/DisksApp.cpp # programs/disks/ICommand.h # programs/server/Server.cpp # src/Common/FailPoint.cpp # src/Common/ThreadStatus.h # src/Core/ServerSettings.cpp # src/Disks/DiskObjectStorage/DiskObjectStorage.cpp # src/Disks/DiskObjectStorage/DiskObjectStorage.h # src/Disks/DiskObjectStorage/DiskObjectStorageTransaction.cpp # src/Disks/DiskObjectStorage/MetadataStorages/IMetadataStorage.h # src/Disks/DiskObjectStorage/MetadataStorages/MetadataStorageFactory.cpp # src/Disks/DiskObjectStorage/ObjectStorages/IObjectStorage.h # src/Disks/DiskObjectStorage/ObjectStorages/Local/LocalObjectStorage.cpp # src/Disks/DiskObjectStorage/ObjectStorages/S3/S3ObjectStorage.cpp # src/Disks/DiskObjectStorage/RegisterDiskObjectStorage.cpp # src/Disks/IDiskTransaction.h # src/IO/ReadPipeline.cpp # src/IO/ReadPipeline.h # src/IO/S3/copyS3File.cpp # src/IO/S3/copyS3File.h # src/IO/S3Common.cpp # src/IO/WriteBufferFromS3.cpp # src/IO/tests/gtest_writebuffer_s3.cpp # src/Interpreters/InterpreterSystemQuery.cpp # src/Interpreters/ServerAsynchronousMetrics.cpp # src/Parsers/ASTSystemQuery.h # src/Parsers/tests/gtest_Parser.cpp # src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp # src/Storages/MergeTree/DataPartStorageOnDiskFull.cpp # src/Storages/MergeTree/DataPartsExchange.cpp # src/Storages/MergeTree/IMergeTreeDataPart.cpp # src/Storages/MergeTree/MergeTask.cpp # src/Storages/MergeTree/MergeTreeData.cpp # tests/integration/test_replicated_database/test.py # tests/queries/0_stateless/02253_empty_part_checksums.sh # tests/queries/0_stateless/02254_projection_broken_part.sh # tests/queries/0_stateless/02255_broken_parts_chain_on_start.sh # tests/queries/0_stateless/02369_lost_part_intersecting_merges.sh # tests/queries/0_stateless/02370_lost_part_intersecting_merges.sh # tests/queries/0_stateless/02444_async_broken_outdated_part_loading.sh # tests/queries/0_stateless/04215_replicated_missing_covered_part_on_start.sh # tests/queries/0_stateless/04327_reader_executor_metrics.sql # tests/queries/0_stateless/04328_reader_executor_kpi_async_metric.sql
Kept the antalya-26.8 shape everywhere and layered the CAS changes on top: - `DiskObjectStorageTransaction`: base already routes every metadata effect through `addOperation` (eager vs queued, driven by `appliesOperationsEagerly`), which is the same mechanism as the PR's `dispatch`. Kept `addOperation` at the call sites, made it also honour `transactionIsStagingOverlay`, and dropped the duplicate `dispatch` template. - `copyS3File`: threaded `copy_mode` through the new `copyS3FileImpl`; `copyS3FileRange` passes `ObjectStorageCopyMode::Default`. - `DataPartStorageOnDiskFull`: moved the in-flight read-your-writes checks into the new `*Impl` methods (packed skip-index handling now lives in the base wrappers) and restored `getPackedFileUncompressedSize`, which the auto-merge had mixed up. - `DataPartStorageOnDiskBase::freeze`/`freezeRemote`: clone-transaction cleanup keeps the base `txn_version.txt.tmp` removal and `writeInvalidatedSystemColumnsFile`. - `LocalObjectStorage`: added the directory check to the relocated `tryGetObjectMetadata`. - `ThreadStatus`: base already keeps the parent `ThreadGroup` alive via `ThreadGroup::parent`, so the PR's duplicate `parent_thread_group` member is not added. - `RegisterDiskObjectStorage`: `use_fake_transaction` no longer exists on antalya-26.8 (object storage disks always use real transactions), so the PR's guard is not needed. - `MergeTreeData::removePartsInRangeFromWorkingSet...`: base already commits the empty covering part's disk transaction unconditionally. - `DataPartsExchange`: kept base `..._WITH_INVALIDATED_SYSTEM_COLUMNS = 10` and the PR's `..._WITH_CA_CONFIRM = 11` as the advertised maximum. - CI: added `cas_functional_tests_jobs` to the 26.8 `FUNCTIONAL_TESTS_JOBS` lists, `start_rustfs` into the 26.8 `start` sequence, removed a duplicated `cas_functional_tests_jobs` block from `altinity_jobs.py`, and hand-merged the generated workflow YAMLs (only CAS jobs added). - Removed duplicated `copyObject` counter and merged both `CopyObject` mocks in `gtest_writebuffer_s3.cpp`. - Stateless tests deleted on antalya-26.8 (converted to integration tests) stay deleted. Source-PR: #2159 (#2159)
…next commit) --- Original cherry-pick message follows: Merge pull request #2300 from Altinity/feature/antalya-26.6/CAS-improvements CAS improvements # Conflicts: # src/Common/ErrorCodes.cpp # src/Disks/DiskObjectStorage/ObjectStorages/S3/S3ObjectStorage.cpp # src/Disks/DiskObjectStorage/ObjectStorages/S3/S3ObjectStorage.h # src/IO/ReadBufferFromS3.cpp # src/IO/ReadSettings.h # src/IO/S3/tests/TestPocoHTTPServer.h # src/IO/WriteSettings.h
Kept antalya-26.8's renumbered fork error codes (1016-1018) and added the PR's CAS codes 1037-1038 with END = 1038; dropped the PR's comments that describe the old 1010/1011 numbering, which does not exist on antalya-26.8. Adapted: setClickhouseAttemptNumber -> setClickHouseAttemptNumber (renamed on antalya-26.8 by e8e1e12) Adapted: S3ObjectStorage client_slot merged with antalya-26.8's client_restricts_server_credentials member Adapted: S3IteratorAsync attempt_seed added alongside antalya-26.8's description member Adapted: readObject keeps antalya-26.8's UnknownSize mapping and object.etag, passes the PR's refresh_callback Adapted: TestPocoHTTPServer keeps antalya-26.8's makeMockServerParams with the PR's dedicated thread pool Source-PR: #2300 (#2300)
…next commit) --- Original cherry-pick message follows: Merge PR #2307 (cas/configurable-mount-lease-timing) into feature/antalya-26.6/CAS-improvements-cicd-fixes Brings k-morozov's configurable CAS mount lease TTL / renewal period (#2307) so the heartbeat-model work can land on top of it while keeping the author's commits intact. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx # Conflicts: # src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedMetadataStorage.cpp Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com> # Conflicts: # docs/en/antalya/cas/configuration.md # src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedMetadataStorage.cpp # src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedMetadataStorage.h # src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedSettings.cpp
The changes from #2307 (`mount_lease_ttl_ms` / `mount_renew_period_ms` settings, members, initializers, `PoolConfig` wiring, validation, and the doc rows) are already on the branch. Every conflict was only the "theirs" side removing later work built on top of them (`unsafe_remount_no_delay`, the `attempt_timeout_ms` check, `connect_timeout_cap_ms`, the updated `cas_mount_renew_period_ms` doc text). #2307 does not remove those, so "ours" is kept in every hunk. Source-PR: #2307 (#2307)
…next commit) --- Original cherry-pick message follows: Merge pull request #2349 from Altinity/cas/memworker-split-allocated-source MemoryWorker: use sanitizer info for allocated counter # Conflicts: # src/Common/MemoryWorker.cpp
…next commit) --- Original cherry-pick message follows: Merge pull request #2385 from Altinity/fix/antalya-26.6/lldb-budget-and-sigcont CI: lldb stack budget by build flavor, and keep lldb's JIT breakpoint out of the server # Conflicts: # ci/tests/test_print_stacktraces.py # tests/clickhouse-test
tests/clickhouse-test: applied the PR's lldb budget and JIT-loader flag on top of antalya-26.8's get_stacktraces_from_lldb, which no longer has _ensure_lldb_installed and already attaches via sudo on macOS with keep_output_on_error; kept the base's liveness-check wording in the print_c_stacktraces docstring. ci/tests/test_print_stacktraces.py: kept the deletion from 3a69545 on antalya-26.8 (ci/tests unit tests removed, no automated CI tests); restoring it would revive ~176 pre-existing lines outside the PR's diff. Dropped: ci/tests/test_print_stacktraces.py budget tests — file deleted on antalya-26.8 by 3a69545 Source-PR: #2385 (#2385)
82f8845 to
eff8803
Compare
eff8803 to
e330851
Compare
CI triage for #2461 (combined CAS port,
|
| Check | Verdict |
|---|---|
| Stateless tests (amd_debug, parallel) | PR-related |
| Stateless tests (arm_binary, parallel) | PR-related |
| Stateless tests (arm_binary, sequential) | PR-related |
| GrypeScanKeeper / Server / Server(-alpine) | Not PR-related (pre-existing base-image CVEs) |
All of the stateless failures are in the feature this PR introduces (cas tests are brand-new here, so they can't be "pre-existing"), plus one pre-existing test (04812) that this PR regresses. None of them look like generic flakiness — the two that ran the randomized-settings diagnosis both came back reproducible. Evidence and concrete fixes below.
Report: result_pr.json for e330851.
1. 04812_clickhouse_disks_move_directory — PR-caused regression, clean fix ✅
(arm_binary parallel; deterministic — the diagnosis re-ran it 18× and it failed 18/18, "fails without randomization too")
Reason in the report is return code: 83, not a reference diff — the test script exits nonzero.
Root cause: this PR (cherry-pick of #2159) adds exit-code propagation to programs/disks/DisksApp.cpp:
if (query.has_value() && last_command_exit_code != 0)
return last_command_exit_code;last_command_exit_code does not exist on antalya-26.8 — before this PR, clickhouse-disks --query always returned EXIT_OK (0). The 04812 test explicitly depends on that old contract — its own comment says so:
# clickhouse-disks always exits 0, so assert the disk state.
…and it deliberately issues commands that error (e.g. move src aa/bb/cc into a missing parent fails with ENOENT on a local disk; remove -r can't empty a dir holding a virtual child on plain_rewritable). With the new behavior those surface as a nonzero process exit (83) and fail the test. The test asserts disk state via list, it does not care about exit codes.
Suggested fix (restore the contract for this test; stdout/reference are unchanged):
function disks()
{
- clickhouse-disks -C "$config" --disk "$disk" --query "$1" 2>/dev/null
+ clickhouse-disks -C "$config" --disk "$disk" --query "$1" 2>/dev/null || true
}Alternative if you'd rather not touch a test owned by #2379: scope the new exit-code propagation to the cas-* commands only (the stated motivation was CI-gating cas-fsck), rather than making it global for every clickhouse-disks --query.
2. 04295_cas_mutation_no_leftovers — PR-related; points at a real CAS + packed-parts gap ⚠️
(arm_binary sequential; diagnosis: passes 16/16 without randomization, culprit --min_bytes_for_full_part_storage 399407315)
-grew_above_baseline 1
+grew_above_baseline 0 (baseline=0 after_insert=0)
The test inserts 4×100k rows and asserts the CAS pool directory grew (find …/ca/{blobs,packs,trees}). With a high min_bytes_for_full_part_storage, parts are written as packed parts (DataPartStorageOnDiskPacked) and no objects land in the counted pool subdirs — see finding #3, which is a crash in exactly that packed path. A one-line test fix (SETTINGS … min_bytes_for_full_part_storage = 0) would turn it green, but that would mask the packed-part behavior rather than fix it. I'd recommend confirming whether CAS is meant to support packed parts before pinning the setting.
3. Server logical error (crash) — most serious, PR-related 🔴
(amd_debug parallel — this is why that suite also shows "Server liveness check failed": the server aborts)
Logical error: 'Uncommitted shared transaction already exists'.
src/Storages/MergeTree/DataPartStorageOnDiskPacked.cpp:520: DB::DataPartStorageOnDiskPacked::beginTransaction()
src/Storages/MergeTree/MergeTask.cpp:630: ExecuteAndFinalizeHorizontalPart::prepare()
… DB::StorageMergeTree::optimize(...) ← triggered by OPTIMIZE / a merge
beginTransaction() throws when transaction || writer is already set and has_shared_transaction is true — i.e. a packed part on a CAS disk gets beginTransaction() called while a shared transaction is already open, during a merge. has_shared_transaction is a CAS addition, so this path is new in this PR. In a debug build the LOGICAL_ERROR aborts the server, which cascades into the liveness failure and masks the rest of the amd_debug run.
This one needs the CAS authors — it can't be verified/fixed statically here (no local build/run in this environment). Same DataPartStorageOnDiskPacked packed path as #2, so I'd treat #2 and #3 as one underlying issue: CAS × packed-part merges.
4. 05008_cas_gc_snapshot_prune, 05009_cas_event_log, 05023_cas_dropns_leaked_namespace — PR-related 🔴
(failed in both amd_debug and arm_binary parallel — reproducible across arches, and arm had no crash, so these are genuine behavior diffs, not just crash fallout)
All three are GC/blob-accounting reference diffs in the new feature:
05009:has_blob_put 1→0(no blob-put recorded)05023:captured_rows_absent_after_gc_fixpoint 1→005008:-1 1→0 1
These are the CAS GC/event-log oracles disagreeing with the ported runtime. Like #2/#3 the recurring theme is blob/pool objects not appearing as the tests expect — plausibly the same packed/compact-part storage interaction. Needs the feature authors to confirm whether it's a port regression or a test-robustness gap; I can't settle it without running the server.
5. Grype scans (keeper / server / server-alpine) — NOT PR-related
fail = "image contains N high/critical CVEs" from the base OS/library layers (keeper: 4, server-alpine: 1, server: 0-but-policy-fail). This PR changes only C++/tests/CI config — no docker/, contrib/, or requirements.txt changes — so it introduces none of these, and they fail branch-wide. Safe to ignore for this PR / handle separately.
What I'd do next
- Update README.md #1 is a ready, safe fix — happy to apply it.
- Update README.md #2–Update README.md #4 are in the CAS runtime (packed parts + GC) and need @filimonov / @k-morozov; Update README.md #2/Update README.md #3 look like one bug (CAS × packed-part merge), Update README.md #4 like the GC oracles.
I have not pushed anything. Want me to open a blau/* PR with the 04812 fix (and, if you confirm it's acceptable to mask, the 04295 setting pin), or commit the 04812 fix directly to feature/antalya-26.8/pr-2159?
I can't build or run ClickHouse in this environment — findings are from the praktika S3 report + static reading; any fix is validated by CI on the PR.
|
waiting from input from @filimonov on test fails related to new packed parts |
|
Superseded: #2159 is now ported as part of group |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add experimental content-addressed storage (
cas) for object-storage disks, which stores MergeTree parts as deduplicated blobs in a shared S3/GCS pool that multiple servers can mount, replicates by relinking existing blobs instead of copying them, and comes with lease-coordinated garbage collection, online integrity checks,SYSTEM CAScommands, system tables,clickhouse-diskstools,ALTER TABLE ... EXPORT PARTITIONsupport and configurable mount lease timing. Also stop object-storage listings from parsing every listed key as a JSON command, which removes one thrown and caught exception per key on every object storage.Add experimental content-addressed storage (
cas) for object-storage disks, which stores MergeTree parts as deduplicated blobs in a shared S3/GCS pool that multiple servers can mount, replicates by relinking existing blobs instead of copying them, and comes with lease-coordinated garbage collection, online integrity checks,SYSTEM CAScommands, system tables,clickhouse-diskstools,ALTER TABLE ... EXPORT PARTITIONsupport and configurable mount lease timing. Also stop object-storage listings from parsing every listed key as a JSON command, which removes one thrown and caught exception per key on every object storage (#2159 by @filimonov, #2300 by @filimonov, #2307 by @k-morozov, #2312 by @k-morozov, #2326 by @k-morozov, #2349 by @k-morozov, #2360 by @filimonov, #2384 by @filimonov, #2385 by @filimonov, #2393 by @filimonov).CI/CD Options
Exclude tests:
Regression jobs to run:
Combined port of 10 PR(s) (group
pr-2159). Cherry-picked from #2159, #2300, #2307, #2312, #2326, #2349, #2360, #2384, #2385, #2393.