26.8 CAS avoid manifest get on promote by k-morozov · Pull Request #2794 · Altinity/ClickHouse

k-morozov · 2026-11-05T12:02:55Z

Related: #2528

Changelog category (leave one):

Performance Improvement

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

Reduced CAS object storage requests by reusing the proof recorded when a manifest is staged.

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

Regression jobs to run:

(cherry picked from commit 5a44354) Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Keep blob dependency checks on the staged-manifest path and verify that foreign manifests still require a body read and identity validation. Cover invalid entry paths before the body write and preserve independent decoder coverage with forged input. Port the promote dependency case from 9f9c13b and path validation cases from 7d7fcc9, 470fb0e and 17dc11c. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

(cherry picked from commit ff7d99f) Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Review: https://gist.github.com/k-morozov/7b34b402f18b051ced88f457c95d5df4 (cherry picked from commit 657c521) Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Write the expected `LOGICAL_ERROR` message to `stderr` in the child. Keep `EXPECT_DEATH` so debug and sanitizer builds test the same failure path. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Missing dependency proofs are already covered by `MissingDependencyProofFailsClosed`, `PromoteRevalidatesBlobPresenceFailClosed` and `WDepSetCrossAlgoSatisfactionFailsClosed`. These tests also use the staged-manifest path. Keep the mixed-entry test focused on successful publication without a manifest `GET`. Remove its duplicate `EXPECT_DEATH` check and the logger setup needed to capture the error message. Rename the tests to state whether the manifest has no entries or has both `Inline` and `Blob` entries. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

All current production paths call `stageManifest` and `promote` on the same `PartWriteTxn`. This includes normal writes, repoint, republish and interserver relink. The private `staged_manifest_ids` map has no eviction, and `stageManifest` records the manifest before returning its id. A missing record therefore means a broken invariant. Reading the body from storage hides that error and allows one build to promote another build's manifest, which the production paths do not need. Remove the manifest `GET` path and raise `LOGICAL_ERROR` before the ref-log append when local staging evidence is missing. Reject foreign ids even if they already name the current committed manifest. Keep idempotent re-promotion on the same build. Replace the foreign-manifest read test with invariant checks for both a live precommit and an already committed manifest. Check the exception, zero body reads and unchanged ref-log and binding state in release; check termination in debug and sanitizer builds. Update the idempotency test to reuse the original build and document the stricter contract. Validation: the new invariant test failed before the change. The focused release build and all 101 tests passed after the change. Debug and sanitizer builds were not run. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Rename `id` to `manifest_id` in `promote` and `precommitAdd` so the parameter names identify the manifest explicitly. Update declarations, uses, lambda captures and existing parameter references. This is a naming refactor with no behavior change. Validation: `git diff --check` passed. The function changes were checked to be exactly reversible by renaming the identifier back. Build and tests were not run for this mechanical rename. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Rename `blob_dependencies` to `blob_refs` to make it clear that the record stores blob references. Use `manifest_blob_refs` in `promote`, `blob_refs` in its lambda capture and `blob_ref` for each loop item. Store `const StagedManifest` values in `staged_manifest_ids` so a staged record cannot be changed after insertion. This is a refactor with no change to publication behavior. Validation: the focused build compiled all nine translation units that depend on the header, including the const map value type. All 101 focused release tests passed. `git diff --check` passed. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

github-actions · 2026-11-05T12:04:39Z

Workflow [PR], commit [7bc6028]

Keep the useful explanations removed during the manifest read change: closure capture lifetimes, atomic ownership checks in `precommitAdd`, the `promote` contract and the lifetime of local staging records. Update them to describe `blob_refs` and the requirement that the same build staged the manifest. Remove the stale claim that `promote` validates the stored body. Clarify that a successful staging record does not prove current body existence or GC protection. A manifest decode cache hit can delay detection of a missing body; a backend read and `fsck` expose it. Only comments and documentation change. The C++ token sequences are unchanged, and `git diff --check` passed. Build and tests were not run. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Check encoded text lines in `encodePartManifest` against the decoder's line cap before any manifest body is written. Measure escaped bytes without the line terminator, and leave raw inline payload outside this limit. Cover entry and descriptor boundaries, JSON escaping and refusal before the body write. Build total-size cap fixtures from many valid records. Validation: focused release build and all 105 CAS tests passed. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

…-get-on-promote

filimonov · 2026-11-06T16:12:24Z

-        throwCasWriteRetryLater(fmt::format("promote: ManifestNamespaceMatches failed for {}", manifest_key));
+    const auto staged = staged_manifest_ids.find(manifest_id);
+    if (staged == staged_manifest_ids.end())
+        throw Exception(ErrorCodes::LOGICAL_ERROR,


promote throws LOGICAL_ERROR for a foreign id that is already the committed manifest. precommitAdd (the closure above, committed-match check) returns {} for the same id. PromoteRejectsManifestNotStagedByThisTransaction with already_committed = true shows the mismatch.

Make them agree: in precommitAdd move the committed-match no-op under id_staged_by_this_txn, and drop "or already the current committed manifest" from the header. Nothing uses that branch anymore.

Also: these two errors carry only the manifest ref. Add target_ns, final_ref_name and u128ToHex(promote_build_id), like precommitAdd does.

filimonov · 2026-11-06T16:12:24Z

    ///
-    /// A3 mint-tightening: `id` must either be one THIS transaction minted via `stageManifest`, or
+    /// A3 mint-tightening: `manifest_id` must either be one THIS transaction minted via `stageManifest`, or
    /// already be `final_ref_name`'s current committed manifest (the idempotent re-drive -- see the


See the comment on promote: promote no longer accepts this case. Either drop this clause and the branch in precommitAdd, or make promote accept it too.

filimonov · 2026-11-06T16:12:24Z

+            e.object_hash = manifestRefDebugString(manifest_id.ref);
            e.token = u128ToHex(promote_build_id);
            e.outcome = "promoted";
            e.reason = "promote: atomic owner move precommit(build_id) -> ref(final_ref_name) after fail-closed reval";


"fail-closed reval" no longer exists. Suggested: promote: atomic owner move precommit -> committed.

The same stale "body-absent precommit" text is still in ContentAddressedExchange.h:94,221, ContentAddressedMetadataStorage.cpp:2339,2348,2456,2468, CasPartWriteTxn.h:317 ("an absent body") and docs/en/antalya/cas/architecture/replication.md:72,105 (row 7 contradicts manifests-and-refs.md). Please remove all of them in this PR.

filimonov · 2026-11-06T16:12:24Z


+    struct StagedManifest
+    {
+        Etag incarnation;


incarnation is dead. The only check compares its key with manifestKey(manifest_id), which is the same key it was minted for. Drop the field, the check and the CasEtag.h include. Map value becomes std::vector<BlobRef>.

filimonov · 2026-11-06T16:12:24Z

-        expectThrowsCode(DB::ErrorCodes::CORRUPTED_DATA, [&] { decodePartManifest(encoded); });
+        String forged = valid;
+        forged.replace(pos, needle.size(), "\"path\":\"" + String(path) + "\"");
+        expectThrowsCode(DB::ErrorCodes::CORRUPTED_DATA, [&] { decodePartManifest(forged); });


The forged text keeps the old payload_digest, so the digest check also throws CORRUPTED_DATA. Remove the path check and the test still passes. Assert the message invalid entry path, or fix the digest in the forged text.

filimonov · 2026-11-06T16:12:24Z

+        const size_t gets_before = counting->getCountFor(manifest_key);
+        const size_t tail_before = store->tailSinceSnapshotCountForTest(ns);
+#if defined(DEBUG_OR_SANITIZER_BUILD)
+        EXPECT_DEATH(publisher->promote(ns, "part", publisher->buildId(), id), "");


EXPECT_DEATH(..., "") only proves the process died. Use the same regex as the release arm: was not staged by this transaction.

filimonov · 2026-11-06T16:12:24Z

+void validateManifestEntryPath(std::string_view path)
+{
+    if (path.empty() || path.front() == '/')
+        throw Exception(ErrorCodes::CORRUPTED_DATA, "CAS part manifest: invalid entry path '{}'", path);


On encode a bad path is a caller bug, not corrupt data. Keep CORRUPTED_DATA for decode, use LOGICAL_ERROR for encode. Same for the duplicate-path check below.

filimonov · 2026-11-06T16:12:24Z

+
    CasJsonWriter out(256);
    writeHeaderLine(out, FormatId::PartManifest);
+    check_line_bytes(out.size() - 1, "header");


Optional. Header and trailer lines cannot reach 64 KiB; this check and the trailer one never fire. kMaxManifestEncodedBytes in CasPartWriteTxn.cpp:57 duplicates traitsFor(PartManifest).object_cap. Use fitsObjectCap here like CasFoldSealFormat.cpp does, or move the encode-time hardening to a separate PR.

filimonov · 2026-11-06T16:12:25Z

+        auto build = startBuildFor(store, ns, "part");
+
+        ManifestEntry entry;
+        entry.path = String(path_char == 'a' ? 65536 : path_char == '"' ? 32768 : 10923, path_char);


Optional. 65536 / 32768 / 10923 are magic (10923 * 6 = 65538 for the escaped quote). Derive from traitsFor(FormatId::PartManifest).line_cap or add a comment.

k-morozov added 9 commits November 5, 2026 11:40

Add test for redundant manifest read during promote

3bf2a9c

(cherry picked from commit 5a44354) Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Avoid rereading staged manifests during promote

d8984d1

(cherry picked from commit ff7d99f) Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Validate CAS manifest entry paths before encoding

ff3b531

Review: https://gist.github.com/k-morozov/7b34b402f18b051ced88f457c95d5df4 (cherry picked from commit 657c521) Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

Set up logging for the promote dependency death test

755b5d0

Write the expected `LOGICAL_ERROR` message to `stderr` in the child. Keep `EXPECT_DEATH` so debug and sanitizer builds test the same failure path. Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>

k-morozov changed the title ~~Perf/antalya 26.8/cas avoid manifest get on promote~~ 26.8 CAS avoid manifest get on promote- #2528 Nov 5, 2026

k-morozov added antalya CAS antalya-26.8 Session label (releasy session config) labels Nov 5, 2026

k-morozov mentioned this pull request Nov 5, 2026

CAS avoid manifest get on promote #2528

Closed

10 of 30 tasks

k-morozov changed the title ~~26.8 CAS avoid manifest get on promote- #2528~~ 26.8 CAS avoid manifest get on promote Nov 5, 2026

k-morozov and others added 3 commits November 5, 2026 13:51

Merge branch 'antalya-26.8' into perf/antalya-26.8/cas-avoid-manifest…

dd41b85

…-get-on-promote

k-morozov marked this pull request as ready for review November 5, 2026 18:25

k-morozov requested a review from filimonov November 5, 2026 18:25

This was referenced Nov 5, 2026

Fix DataLakeCatalog table creation issues #2792

Merged

Antalya 26.8 Backport of #128865 - Fix wrong ORDER BY results and pushed bucket twice with offset based parallel replicas #2799

Merged

filimonov mentioned this pull request Nov 6, 2026

Antalya 26.6: port the CAS fixes merged into antalya-26.8 (PR 2730, 2773) #2806

Open

11 of 31 tasks

Merge branch 'antalya-26.8' into perf/antalya-26.8/cas-avoid-manifest…

7bc6028

…-get-on-promote

filimonov requested changes Nov 6, 2026

View reviewed changes

Navigation Menu

Repository navigation

26.8 CAS avoid manifest get on promote - #2794

26.8 CAS avoid manifest get on promote#2794
k-morozov wants to merge 13 commits into
antalya-26.8from
perf/antalya-26.8/cas-avoid-manifest-get-on-promote

k-morozov commented Nov 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 5, 2026 •

edited

Loading

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

filimonov Nov 6, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

k-morozov commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Documentation entry for user-facing changes

CI/CD Options

Exclude tests:

Regression jobs to run:

Uh oh!

github-actions Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

k-morozov commented Nov 5, 2026 •

edited

Loading

github-actions Bot commented Nov 5, 2026 •

edited

Loading