Skip to content

CAS: packed parts - #2579

Open
k-morozov wants to merge 73 commits into
antalya-26.8from
feature/cas-packed-parts
Open

k-morozov wants to merge 73 commits into
antalya-26.8from
feature/cas-packed-parts

Conversation

@k-morozov

@k-morozov k-morozov commented Oct 15, 2026 •

Copy link
Copy Markdown

Changelog category (leave one):

  • New Feature

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

Added basic support for Packed parts on CAS disks.

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Unit tests
  • Performance tests
  • Aarch64 tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

mkmkme and others added 30 commits October 8, 2026 15:46
…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
Source PR: #2159

- `.github/workflows/*.yml` are generated: kept at the base version; the
  "Check Workflows" job regenerates them from `ci/workflows/*.py`
  (`python3 -m praktika yaml`), which now include the CAS lanes.
- Stateless tests 02253/02254/02255/02369/02370/02444/04215 stay deleted:
  `antalya-26.8` converted them to integration tests (7f7a30e).
- `ci/defs/altinity_jobs.py`: dropped the duplicated
  `cas_functional_tests_jobs` (already on base from 5aaf212).

Adapted: DiskObjectStorageTransaction - PR's `dispatch` mapped onto base `addOperation` (made eager also for `transactionIsStagingOverlay`), keeps `appliesOperationsEagerly` for in-memory metadata
Adapted: RegisterDiskObjectStorage - `use_fake_transaction` selection dropped, fake transactions were removed on base (c6254c3)
Adapted: copyS3File - `copy_mode` threaded through base `copyS3FileImpl`; ranged copies use `ObjectStorageCopyMode::Default`
Adapted: DataPartStorageOnDiskFull - in-flight reads moved into the base `*Impl` overrides (packed skip-index overlay lives in the base wrapper)
Adapted: MergeTask/IMergeTreeDataPart - `use_parent_transaction` passed to base `getProjectionNoInitialize`
Adapted: DataPartsExchange - kept base `REPLICATION_PROTOCOL_VERSION_WITH_INVALIDATED_SYSTEM_COLUMNS = 10`, negotiation tops out at `WITH_CA_CONFIRM = 11`
Adapted: MergeTreeData - empty covering part already commits its disk transaction on base, PR's CAS-only commit dropped as redundant
Adapted: functional_tests.py - RustFS start folded into base SeaweedFS-based `start`
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
- `ErrorCodes.cpp`: kept antalya-26.8 numbering (1009-1018) and the
  `CLICKHOUSE_PARSER_MINIMAL_BUILD` guard; added the PR's
  `CAS_WRITE_UNATTRIBUTED` (1037) and `CAS_DELETE_MARKER` (1038), END = 1038.
  Dropped the PR's comments describing the antalya-26.6 1009-1011 layout,
  which does not exist on antalya-26.8.
- `S3ObjectStorage`: combined the PR's `client_slot`/`client` reference with
  antalya-26.8's `client_restricts_server_credentials`, the iterator's
  `description` with the PR's `attempt_seed`, and antalya-26.8's
  `UnknownSize`-aware size and `object.etag` args with the PR's
  `refresh_callback`.
- `TestPocoHTTPServer.h`: kept `makeMockServerParams` and added the PR's
  dedicated thread pool and destructor.
- Adapted the PR's `S3::setClickhouseAttemptNumber` calls to
  `S3::setClickHouseAttemptNumber`, renamed on antalya-26.8 by e8e1e12.

Adapted: setClickhouseAttemptNumber -> setClickHouseAttemptNumber (renamed by e8e1e12 on antalya-26.8)
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
All changes from #2307 (`mount_lease_ttl_ms` / `mount_renew_period_ms` settings, members, wiring into `PoolConfig`, validation, docs) are already present on the base. The conflicting hunks keep the base side, which has later additions on top of them (`unsafe_remount_no_delay`, the `attempt_timeout_ms` check, `connect_timeout_cap_ms`, and the refined `cas_mount_renew_period_ms` doc text).

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
Kept the speculative RSS reservation block present on antalya-26.8 and
switched its reads of the former `resident` local to `memory_usage.resident`,
since `getMemoryUsage` now returns `MemoryUsage`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Source-PR: #2349 (#2349)
…ed-variable

Fix the coverage build: unused variable in gtest_cas_throttling_gate.cpp

Source-PR: #2360 (#2360)
…time-knobs

CI: cap the RustFS blocking-thread pool on the CAS-S3 lanes and collect its log
Source-PR: #2384 (#2384)
…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 the antalya-26.8 shape of get_stacktraces_from_lldb (sudo/shutil.which
lldb path and keep_output_on_error kept; no _ensure_lldb_installed, which
antalya-26.8 does not have). Docstring keeps the 'liveness check' call site
that exists on antalya-26.8.

ci/tests/test_print_stacktraces.py: kept deleted, as on antalya-26.8, where
3a69545 removed all of ci/tests/. The PR's test changes to that file are
therefore not carried over.

Source-PR: #2385 (#2385)
…-on-listing-keys

Temporary workaround: do not probe object-storage listing keys as JSON

Source-PR: #2393 (#2393)
…next commit)

---
Original cherry-pick message follows:

Merge pull request #2292 from Altinity/enable-cas-s3-stress

Enable cas s3 stress
# Conflicts:
#	.github/workflows/master.yml
#	.github/workflows/pull_request.yml
#	ci/tests/test_stress_job.py
Kept the antalya-26.8 `needs` lists of `finish_workflow` in `master.yml` and
`pull_request.yml` and added only the four `stress_test_amd_*_cas_s3_storage`
jobs introduced by the source PR. Kept both `ensure_ripgrep_installed` and
`get_additional_envs` imports in `ci/tests/test_stress_job.py`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Source-PR: #2292 (#2292)
…-probes-no-list

CAS: answer directory probes inside a part from the part manifest, no LIST per part file
Source-PR: #2440 (#2440)
…val-one-drop-pr

CAS: remove a part with one ref drop
Source-PR: #2452 (#2452)
…meta-delete

CAS GC: delete a condemn marker only for the condemnation that owns it

Source-PR: #2463 (#2463)
…next commit)

---
Original cherry-pick message follows:

Merge pull request #2444 from Altinity/docs/antalya-26.6/metrics-and-events

Document Antalya-only profile events and metrics.
# Conflicts:
#	src/Common/ProfileEvents.cpp
…strictness-pr

CAS: drop KeyStrictness, readers skip unknown ordinary keys
Source-PR: #2455 (#2455)
…ete-key-limit

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

Source-PR: #2484 (#2484)
…ng-after-read

Date a GC mount-token sighting after the read that returned it

Source-PR: #2509 (#2509)
…tinel-boundary

Return the last Indeterminate when a CAS probe pause ends on the deadline

Source-PR: #2510 (#2510)
…next commit)

---
Original cherry-pick message follows:

Merge pull request #2511 from Altinity/fix/antalya-26.6/cas-renewal-memory-guard

Keep the CAS mount lease renewal alive over the server memory limit
# Conflicts:
#	src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasMountRuntime.cpp
Kept both includes: `Common/ProfileEvents.h` from antalya-26.8 and `Common/LockMemoryExceptionInThread.h` from the source PR.

Source-PR: #2511 (#2511)
…r-time-budget

CAS GC: batch the namespace janitor's deletes under a soft time budget

Source-PR: #2483 (#2483)
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [267672c]

zvonand and others added 2 commits October 16, 2026 16:41
`AlterDropPartitionExecutor` (#2533) called
`getManifestFileEntriesHandle` and `generateManifestList` without the `ExternalStorageCache`
argument added by #2551. Pass the cache of
`IcebergMetadata` to the executor.
Add CAS disk to existing backup tests

Source-PR: #2437 (#2437)
@k-morozov
k-morozov changed the base branch from antalya-26.8 to feature/antalya-26.8/auto-grp-pr-2159 October 19, 2026 11:22
@k-morozov
k-morozov changed the base branch from feature/antalya-26.8/auto-grp-pr-2159 to antalya-26.8 October 19, 2026 11:33
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

# Conflicts:
#	programs/local/LocalServer.cpp
#	programs/server/Server.cpp
#	src/Common/ErrorCodes.cpp
#	src/Common/FailPoint.cpp
#	src/Core/ServerSettings.cpp
#	src/Interpreters/InterpreterSystemQuery.cpp
#	src/Storages/ObjectStorage/DataLakes/Iceberg/AlterDropPartitionExecutor.cpp
#	src/Storages/ObjectStorage/DataLakes/Iceberg/AlterDropPartitionExecutor.h
#	src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp
…acked-parts

Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

# Conflicts:
#	.github/workflows/master.yml
#	.github/workflows/pull_request.yml
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
@k-morozov
k-morozov marked this pull request as ready for review October 22, 2026 20:22
@zvonand

zvonand commented Oct 23, 2026

Copy link
Copy Markdown
Member

@k-morozov does it supersede #2540 ? or this is a standalone work? maybe I exclude that PR from the scope?

@k-morozov

Copy link
Copy Markdown
Author

standalone work

Yes, this is standalone work and will be merged after #2540.

@alsugiliazova

Copy link
Copy Markdown
Member

PR #2579 CI Verification

Verdict

No failure is attributable to this PR. Every one of the PR's own tests passes.

CI does not block approval of #2579, but it does not clear the stack either: six of
the nine failures are deterministic breaks on antalya-26.8 itself or on its CAS port,
and they fail the same way on an unrelated sibling PR.

The bigger problem is not a red check. The ten cas storage / cas s3 storage stateless
lanes were skipped on every commit of this PR, and the PR deletes the four
Stress test (…, cas s3 storage) lanes that exist on its base.
Those are exactly the
lanes that would exercise the change with the widest blast radius in the diff —
cas_allow_packed_parts = 1 in the two default-MergeTree-policy configs, which makes the
entire stateless suite run on packed parts over CAS. That configuration was never run.

Category Count Where
regression (not caused by #2579 — see attribution below) 6 /oauth/access control, /oauth/groups, /swarms/…/wrong cluster name, /swarms/…/cpu overload all swarm nodes, test_storage_gcp_auth::test_gcp_auth_ordinary_contract, test_cas_gcs::test_ordinary_goog4_traffic_keeps_upstream_semantics
pre-existing-flaky 3 /swarms/…/object storage cluster profile events with overloaded node, and delete most of the table in each of the two CAS lightweight-delete lanes
infrastructure 1 mechanism, 225 rows A missing Docker image fails setup in integration shards 1/8 and 6/8
cascade 0 —
unknown 0 —

DCO is ACTION_REQUIRED: commits need a sign-off. The PR is also marked conflicting
with its base.

How the list was reduced

Eight jobs are red. 227 individual rows.

1. Setup errors, 225 rows, one cause. Shards 1/8 (91 errors) and 6/8 (134 errors) are
red almost entirely from ERROR at setup, not from test failures — 6/8 reports
971 passed, 36 skipped, 134 errors and zero failures. Every one of them is the same
docker compose … pull returning 18:

node_old Error manifest for clickhouse/clickhouse-server:23.3.19.33.altinitystable not found: manifest unknown

That image is the old-version node used by backward-compatibility fixtures, and it is no
longer in the registry. It takes out test_storage_s3 in 1/8 and test_storage_delta plus
test_database_backup in 6/8. infrastructure.

2. Three real integration failures, two distinct assertions. After the setup errors,
what is left is test_storage_gcp_auth::test_gcp_auth_ordinary_contract (1/8 and targeted)
and test_cas_gcs::test_ordinary_goog4_traffic_keeps_upstream_semantics (2/8 and targeted),
plus test_cas_gcs::test_a_first_attempt_timeout_is_reissued_as_attempt_two in 2/8's first
pytest session.

3. Four regression-suite jobs. oauth, swarms, cas_lightweight_delete_3,
cas_s3_cache_lightweight_delete_3 — nine leaf scenarios between them.

Findings

/oauth/access control and /oauth/groups — regression, owner antalya-26.8

Nine of 112 oauth scenarios fail. The reported assertion is an expectation mismatch, not a
timeout:

Expected HTTP status code 500, but got 403. Response body: Code: 497.
DB::Exception: a_c74925: Not enough privileges. To execute this query, it's necessary
to have the grant CREATE TEMPORARY TABLE ON *.*. (ACCESS_DENIED)

Split by release line, the rate is not ambiguous:

Feature On 26.8 Everything else
/oauth/access control 64 fails / 64 runs (100 %) 0 / 473
/oauth/groups 64 fails / 64 runs (100 %) 0 / 473

It fails on the branch itself (REFs/antalya-26.8/ab9c4003d72ad373f88f9695b) and on
unrelated PRs #2545 and
#2588. antalya-26.8 broke it; #2579
inherits it. Someone needs to own it, but not in this PR.

/swarms/…/wrong cluster name and /swarms/…/cpu overload all swarm nodes — regression, owner antalya-26.8

Same shape. Both are 109 fails / 109 runs (100 %) on 26.8, and 0 / 584 everywhere
else
, across 10 distinct commits. The error is

Code: 279. DB::Exception: Cannot connect to any replica for query execution.
(ALL_CONNECTION_TRIES_FAILED)
(query: SELECT hostName(), count() FROM s3Cluster('swarm', 'http://minio:9000/warehouse/…'))

Nothing in this PR touches cluster discovery, s3Cluster, or swarm node startup.

test_storage_gcp_auth::test_gcp_auth_ordinary_contract and test_cas_gcs::test_ordinary_goog4_traffic_keeps_upstream_semantics — regression, owner the antalya-26.8 CAS port

These two are the ones worth reading carefully, because the naive rate comparison points
straight at this PR and is wrong.

Test Branch runs (pr = 0) Branch fails #2579 runs #2579 fails
test_gcp_auth_ordinary_contract 86 4 7 7
test_ordinary_goog4_traffic_keeps_upstream_semantics 85 0 7 7

Never-fails-on-branch against 7-of-7-here is the textbook PR-caused pattern. It does not
hold here, for two reasons.

The baseline is the wrong branch. Every pull_request_number = 0 run of these two tests
is on antalya-26.6 or a v26.6.* tag. There is no antalya-26.8 branch run for either
test, because the CAS code that ships them has not landed on antalya-26.8 — it lives on
the stacked branch this PR sits on. The baseline measures a branch that cannot run the code
under test.

A sibling PR fails identically.
#2647 (CAS: ref-log owner Move for part renames, feature/antalya-26.8/cas-disk-transactions → antalya-26.8) fails both tests on
2026-10-23, and fails the same 26 test_database_backup rows. #2647 shares nothing with
#2579 except the base.

The mechanism has no path from this diff. Both tests fail on the same assertion — an
If-Match header present on an ordinary, non-CAS object-store request:

assert 'if-match' not in headers
E   AssertionError: {'headers': {..., 'authorization': 'GOOG4-HMAC-SHA256 …'}, 'key': 'ordinary/ordinary.txt'}

Whatever adds that header lives in S3ObjectStorage or under src/IO/S3/. This PR's own
commits touch neither; its only I/O changes are ReadBufferFromFileView and
PackedFilesReader, and neither sets HTTP headers.

/lightweight delete/immediate removal/…/delete most of the table — pre-existing-flaky

One scenario in each CAS lane: VersionedCollapsingMergeTree in cas_lightweight_delete_3,
MergeTree in cas_s3_cache_lightweight_delete_3. The failing line is a wall-clock budget,
not a correctness check — the data assertion one line above it passed:

r = node.query(f"SELECT count(*) FROM {table_name} FORMAT TabSeparated")
assert r.output == output, error()
assert execution_time < 20, error()  # todo rewrite value

The scenario ran for about 40 s against 100 partitions × 100 000 rows.

Scenario Lane Runs Fails Rate
MergeTree/delete most of the table cas_s3_cache_lightweight_delete_3 179 6 3.4 %
VersionedCollapsingMergeTree/delete most of the table cas_s3_cache_lightweight_delete_3 179 5 2.8 %
VersionedCollapsingMergeTree/delete most of the table cas_lightweight_delete_3 181 3 1.7 %
MergeTree/delete most of the table cas_lightweight_delete_3 181 2 1.1 %
every engine suite (non-CAS) 275 1–5 each 0.4–1.8 %

It fails on the plain non-CAS lane too. Two lanes each tripping one of seven engines is what
a ~2 % per-scenario rate predicts. A deterministic code defect would hit the same engine in
both lanes; these hit different ones.

/swarms/…/object storage cluster profile events with overloaded node — pre-existing-flaky

6.6 % on 26.8 (7 of 106) against 5.4 % elsewhere (31 of 575), over 13 distinct commits. The
rates are the same on both sides.

What passed, and what was never run

The feature's own tests all pass, on both the debug and the ASan/UBSan stateless lanes:

Test Result
05217_cas_packed_parts OK
05218_cas_packed_fetch_partition OK
05219_cas_packed_backup_restore OK
05220_cas_unique_key_mutation_keeps_full OK
05301_cas_mutation_preserves_source_storage OK
05385_cas_packed_merge_disabled_destination OK
test_cas_s3/test.py::test_cas_s3_packed_parts OK
Unit tests (asan_ubsan) (23 631 rows, incl. the new CAS and file-view gtests) success

The full CAS regression suite ran and was green apart from the two flaky lightweight-delete
scenarios above: cas, cas_s3_cache, cas_aggregate_functions_*, cas_alter_*,
cas_atomic_insert, cas_engines, and the rest.

Two gaps, and they are the reason not to read the green as full coverage.

The CAS stateless lanes were skipped. All ten are SKIPPED on this PR — amd_binary, cas storage, parallel; amd_binary, cas s3 storage, parallel; arm_binary, cas s3 storage, parallel; and the amd_asan_ubsan, amd_tsan, amd_msan batches. The database has zero
rows for any of them on any commit of #2579, while other antalya-26.8 CAS PRs
(#2644,
#2623,
#2614) run them with tens of thousands of
rows each. This PR sets cas_allow_packed_parts = 1 in
tests/config/config.d/cas_storage_policy_for_merge_tree_by_default.xml and
cas_s3_storage_policy_for_merge_tree_by_default.xml — making the whole stateless suite use
packed MergeTree parts on CAS — and that configuration was never executed.

The PR removes the CAS stress lanes. Commit 762c37ae ("Revert port of #2292") deletes
Stress test (amd_debug | amd_asan_ubsan | amd_tsan | amd_msan, cas s3 storage) from
ci/defs/job_configs.py, along with the USE_CAS_S3_STORAGE_FOR_MERGE_TREE plumbing in
ci/jobs/stress_job.py and its unit tests. Those four lanes exist on the base branch and run
on #2644, #2623 and #2614. The PR description does not mention the removal.

The CAS regression lanes do not cover packed parts. cas_allow_packed_parts appears
nowhere in the clickhouse-regression repository, so every green Regression release cas_*
job ran with packed parts off. Those jobs are still meaningful — they exercise the refactored
*InTransactionOrDisk read helpers on Full parts over CAS, which is a live behaviour change
in this PR — but they say nothing about the packed path.

Recommendations

  1. Un-skip the CAS stateless lanes and re-run, or state explicitly why packed-on-CAS
    across the whole stateless suite does not need to run before merge. This is the one item
    that would change the verification result.
  2. Explain or revert the deletion of the four CAS stress lanes, which is unrelated to
    packed parts and silently reduces coverage on the base branch.
  3. Resolve the merge conflict with antalya-26.8 and add the DCO sign-off.
  4. Rerun integration shards 1/8 and 6/8 once the clickhouse-server:23.3.19.33.altinitystable
    image is back, or pin the compatibility fixtures to an image that still exists. This is a
    branch-wide problem, not a CAS: packed parts #2579 problem.
  5. The /oauth, /swarms and GCS If-Match breaks need owners on antalya-26.8 and its CAS
    port. They are 100 % reproducible there and will block every PR on that branch until fixed.

@alsugiliazova

Copy link
Copy Markdown
Member

PR #2579 Audit Review

Verdict

No High or Medium distributed-systems defect in reviewed scope.

Four Low findings, all in the resource and configuration/observability classes. The change that
mattered most for correctness — whether a per-replica choice of Packed vs Full can make two
replicas of the same part disagree — holds, and the reason is given below.

One coverage gap worth closing before merge: nothing exercises two servers with two separate CAS
pools exchanging a packed part.

What the change does

A CAS disk previously forced every new MergeTree part to Full storage. This PR makes Packed
available behind a per-disk opt-in and makes the packed read path work on a part that is still
being assembled inside an open CAS transaction.

1. Opt-in capability. New setting cas_allow_packed_parts, default false
(ContentAddressedSettings.cpp), surfaced as IDisk::allowsPackedParts() and forwarded through
DiskEncrypted and ReadOnlyDiskWrapper.

2. The gate moved from the policy to the destination disk. choosePartFormat no longer scans
every disk of the storage policy and downgrades if any one of them forbids Packed. Instead a
static MergeTreeData::normalizePartFormatForDisk(format, disk) is applied once the actual
destination is known: in MergeTreeDataWriter::writeTempPartImpl for inserts, in
FutureMergedMutatedPart::updatePath for merges and mutations, and as a backstop in
MergeTreeDataPartBuilder::withPartStorageType for PartDirIntent::CreateFresh.

3. Transaction-aware reads, shared between the two part storages. The read-your-writes
overlay that DataPartStorageOnDiskFull had grown inline (existsFile, existsDirectory,
iterate, getFileSize, getRemotePaths, prepareRead, readFileIfExists, each consulting
transaction->tryGetInFlight*) is lifted into six protected helpers on
DataPartStorageOnDiskBase and DataPartStorageOnDiskPacked is converted to use them. Every
volume->getDisk()->... call in the packed storage becomes a ...InTransactionOrDisk call.

4. Reading the archive through a buffer instead of a disk path. PackedFilesReader gains a
constructor from ReadBufferFromFileBase & and a readFile(buffer, name) overload, and
patchSettings becomes public. DataPartStorageOnDiskPacked gains private readArchive /
readArchiveFile, which obtain the data.packed buffer from the transaction overlay. This is
what lets a packed part be read before its CAS commit.

5. Borrowed transactions. A packed projection sub-part that rides the parent's transaction
used to throw LOGICAL_ERROR on commit and undo. Now beginTransaction is a no-op,
commitTransaction finalizes the writer (so the archive lands in the parent's transaction) and
returns before committing, and undoTransaction returns without touching someone else's
transaction. This mirrors DataPartStorageOnDiskFull::commitTransaction.

6. data.packed stays a blob. partFileMustStayBlob now matches the basename data.packed
and data.packed.tmp, including under a *.proj/ prefix, so an archive is never inlined into the
CAS manifest.

7. ReadBufferFromFileView::count() fix. A view's count() previously drifted whenever the
inner buffer was manipulated through executeWithOriginalBuffer. The new
restoreViewWorkingBuffer(view_position) re-establishes bytes + offset() == view_position after
every such operation. This matters here because a packed read on CAS is now a view over a view:
the member file view sits on the archive buffer, which may itself be a view over a pool blob.

Model

Actors The server assembling a part (insert, merge, mutation), peer replicas fetching parts over DataPartsExchange, CAS GC and the orphan-manifest sweep in every server sharing the pool, and the object store
Channels The CAS blob / manifest / ref protocol over conditional object-store operations; replica-to-replica HTTP part transfer; Keeper log entries, which name a part but carry no storage format
Authoritative state The committed CAS manifest for a part. An open disk transaction is a per-process staging overlay: no peer, and no second reader in the same process that does not hold the transaction, can see it
Failure model assumed by the code A part is published by exactly one commit. A crash before that commit leaves uploaded-but-unreferenced blobs for the orphan sweep, not a partial part
Safe-failure criterion An aborted build leaves no reachable partial part; a packed part reads back the bytes its writer wrote on any replica; two replicas of the same part never disagree about its contents

Why replicas cannot diverge, despite choosing the format independently

normalizePartFormatForDisk runs per replica against that replica's destination disk. Two
replicas executing the same MERGE_PARTS log entry can therefore store the same part name as
Packed on one and Full on the other. That is safe, for two reasons that are already in the
code.

A part's checksums are keyed by logical file (.bin, .mrk*, primary.idx, checksums.txt),
not by data.packed. The archive is a container, not a checksummed member. So the
checkPartChecksumsAndAddCommitOps agreement in Keeper compares the same values on both sides
regardless of format.

On the fetch path the two formats are handled separately and deliberately
(DataPartsExchange.cpp, around the isFullPartStorage branch). The wire checksums
(data_checksums) for a packed part are keyed by data.packed, so checkEqual against
part->checksums is skipped and the receiver instead recomputes from the archive with
checkDataPart, failing the fetch when the recompute returns empty. The receiver then builds the
part with withPartFormatFromDisk, so the format follows the bytes that arrived rather than the
receiving disk's capability — a replica with cas_allow_packed_parts = false accepts a packed
part and reads it correctly.

The remaining question is ordering inside a single build: a borrowed packed projection must get
its archive into the parent's transaction before the parent commits. commitTransaction runs
finalizeWriter() before the has_shared_transaction early return, and finalizeWriter ends
with reader.emplace(plan.index), so the projection is both staged and readable afterwards. The
new gtest ProjectionBorrowedTransaction.CASPackedStorageReadsStagedReplacement pins exactly this,
including that a second storage opened on the same path still sees the old payload until the
parent commits.

Findings

Backpressure & resource limits

Low: a packed part on CAS is built entirely in memory and published by one commit, and mutating
one re-uploads the whole archive.

  • Impact: peak memory during a packed part build is the size of the part, and a mutation of a
    packed part costs a full re-upload of every column, defeating the per-column carry-forward that
    is the CAS write-path optimization for Wide parts.
  • Anchor: DataPartStorageOnDiskPacked::checkpointTransaction (returns early when
    isContentAddressed()); DataPartStorageOnDiskPacked::finalizeWriter, whose own comment states
    "The writer keeps the whole part in memory"; MutateTask.cpp lines 2492 and 2506, where
    entries_to_hardlink and the is_full_wide_part carry-forward are gated on
    is_full_part_storage.
  • Trigger: ALTER TABLE ... UPDATE touching one column of a packed part on a CAS disk.
  • Why defect: it is a real cost, not a preference — but it is bounded. Packed is selected
    only below min_bytes_for_full_part_storage, so the amplification is capped by that threshold,
    and the no-checkpoint behaviour deliberately mirrors DataPartStorageOnDiskFull, where a
    mid-build commit would publish a partial part. That is why this is Low and not Medium.
  • Fix direction: document the interaction between cas_allow_packed_parts and
    min_bytes_for_full_part_storage as a sizing rule, so an operator who raises the threshold
    knows they are also raising per-mutation upload cost and per-build memory.
  • Regression test direction: assert that a single-column UPDATE on a packed CAS part does
    not grow pool bytes beyond the archive size, and that the old archive blob is reclaimed by GC.

Configuration & version skew

Low: the capability gates creation only, so a move imports packed parts onto a disk configured
to refuse them.

  • Impact: an operator who sets cas_allow_packed_parts = false on one disk of a multi-volume
    policy still ends up with Packed parts on it. The setting is a policy about new parts, not a
    property of the disk, and nothing reports the discrepancy.
  • Anchor: normalizePartFormatForDisk is called from exactly two places
    (MergeTreeDataWriter::writeTempPartImpl, FutureMergedMutatedPart::updatePath), and
    MergeTreeDataPartBuilder::withPartStorageType gates only PartDirIntent::CreateFresh. The
    move path rebuilds the part with withPartFormatFromDisk, which reads the format off the files.
  • Trigger: ALTER TABLE ... MOVE PART ... TO VOLUME 'full' on the cas_packed_mixed policy,
    or the same move driven by a TTL rule.
  • Why defect: the PR's own test
    tests/queries/0_stateless/05385_cas_packed_merge_disabled_destination.sh performs this move and
    asserts packed_before_merge ... Packed ... 2, so the behaviour is intended. The gap is in the
    documentation: docs/en/antalya/cas/configuration.md says the setting "controls creation, not
    readability. Existing Packed parts can still be opened, attached, fetched, and read" — which
    does not tell the reader that a move or a TTL rule can newly place packed parts on that disk.
    The rolling-upgrade and downgrade procedures in the same section are otherwise complete and
    correct.
  • Fix direction: add the move and TTL cases to the sentence about what the setting does not
    cover.
  • Regression test direction: the coverage already exists in 05385; only the doc line is
    missing.

Observability & diagnosability

Low: an in-flight file with no blob yet is reported as a file with no remote paths.

  • Impact: introspection cannot distinguish "this part has no remote storage" from "the upload
    has not happened yet".
  • Anchor: DataPartStorageOnDiskBase::getStorageObjectsInTransactionOrDisk returns {} when
    tryGetInFlightStorageObjects is nullopt but tryGetInFlightFileSize has a value;
    DataPartStorageOnDiskPacked::getRemotePaths turns that into an empty vector.
  • Trigger: read system.remote_data_paths while a packed part is being assembled.
  • Why defect: the empty return is deliberate and correct — the comment explains that falling
    back to a committed object at the same path would be worse, and the gtest
    CASPackedStorageReadsStagedReplacement asserts getRemotePaths is empty after a staged
    rewrite. The sibling helper prepareReadInTransactionOrDisk handles the same state by
    synthesizing a placeholder object, so the two helpers answer differently about one path. No
    correctness impact: zero-copy replication is already off for CAS
    (DiskObjectStorage::supportZeroCopyReplication).
  • Fix direction: none needed in code; a one-line comment on getRemotePaths stating that an
    empty result during an open transaction means "not yet uploaded" would close it.

Low: clickhouse-disks reports failure through its exit code only for cas- commands.

  • Impact: a script that checks the exit code of clickhouse-disks copy or remove sees
    success on failure; a mistyped CAS command also exits 0.
  • Anchor: programs/disks/DisksApp.cpp, processQueryText — all three catch blocks set
    last_command_exit_code only when command && command->command_name.starts_with("cas-"). When
    getCommandByName itself throws, command is still null, so even cas-fcsk (a typo) exits 0.
  • Trigger: clickhouse-disks --query "cas-fsck nonexistent-disk" versus
    clickhouse-disks --query "cas-fcsk some-disk".
  • Why defect: measured against antalya-26.8 this is not a regression — upstream always
    exits 0, and the commit "Fix clickhouse-disks exit codes for regular commands" restores that for
    non-CAS commands after an earlier CAS-branch change had made every command propagate its code.
    What remains is that the discriminator is a name prefix rather than a property of the command,
    and it excludes the case where the command never resolves.
  • Fix direction: set the exit code when the failing command is a CAS command or when no
    command could be resolved from a cas--prefixed token.

Coverage summary

Fault class Status Note
1. Network / transport N/A No new RPC, timeout, or retry logic; all object-store I/O goes through unchanged CAS request machinery
2. Partition / availability Reviewed No new quorum, leader, or lease logic. cas_allow_packed_parts does not participate in mount leases or fencing
3. Timing / ordering Reviewed One new ordering rule (borrowed projection finalizes before the parent commits); enforced by the placement of finalizeWriter() before the shared-transaction early return
4. Partial / gray failure Reviewed Commit ambiguity is unchanged; the new part_storage_fail_commit_transaction failpoint on packed commit matches the one DataPartStorageOnDiskFull already had
5. Idempotency & duplicates Reviewed The archive is content-addressed, so a retried upload of identical bytes resolves to the same blob key
6. Backpressure & resource limits Reviewed One Low finding
7. Recovery & rollback Reviewed A crash mid-build leaves data.packed/data.packed.tmp blobs unreferenced; both basenames are forced to blob placement and are reclaimed by the existing orphan-manifest sweep. undoTransaction on a borrowed storage correctly leaves the parent's rollback to the parent
8. Configuration & version skew Reviewed One Low finding. The documented rolling-upgrade and downgrade procedure is complete, including that an older binary rejects the unknown key even when it is false
9. Security / trust across nodes N/A No change to peer authentication, authorization, or credential handling
10. Observability & diagnosability Reviewed Two Low findings

Deferred: none.

Assumptions and limits: static review of the PR's own commits at head, with no build and no
execution. Base-branch CAS behaviour (ref ledger, GC, manifests, the fetch wire protocol) was
treated as given.

Coverage gap to close before merge

No test exercises two servers with two separate CAS pools exchanging a packed part.
05218_cas_packed_fetch_partition.sh is tagged no-shared-merge-tree, no-replicated-database and
defines both the source and the destination table with the same inline disk(...) block — same
name, same path, same cas_server_root_id — so source and destination share one disk object
and one pool. The HTTP fetch path and the checkDataPart recompute on the receiving side are
genuinely covered, but the part of the protocol that matters most for CAS (the receiving mount
uploading the data.packed blob into its pool and installing its own manifest and ref) is not.
tests/integration/test_cas_s3/test.py::test_cas_s3_packed_parts is also single-node.

A two-node integration test — insert on node 1, let node 2 fetch, CHECK TABLE on both, then run
GC on both pools — would cover it.

@alsugiliazova alsugiliazova added the verified Approved for release label Oct 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.8 Session label (releasy session config) CAS forwardport This is a frontport of code that existed in previous Antalya versions verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants