Skip to content

BACKUP of CAS tables to S3 on the same endpoint - #2415

Merged
filimonov merged 32 commits into
antalya-26.6from
cas/backup-native-copy-bug
Oct 6, 2026
Merged

filimonov merged 32 commits into
antalya-26.6from
cas/backup-native-copy-bug

Conversation

@k-morozov

@k-morozov k-morozov commented Sep 22, 2026 •

Copy link
Copy Markdown

BACKUP of a table on a CAS to an S3 destination that shares the pool's endpoint failed outright, and the code path that failed would have silently corrupted the backup had it not failed.

BackupWriterS3::copyFileFromDisk asks S3 to copy the source object server-side. That assumes the file is that object, whole, from byte 0. On a CAS disk neither half holds:

  • A small per-part file (checksums.txt, count.txt, columns.txt, ...) is an inline manifest entry with no object of its own. getStorageObjects returns a sized placeholder with an empty remote key, deliberately poisoned so a reader
    that bypasses the read path fails loudly rather than reading someone else's bytes. The copy passed that empty key to CopyObject, so every BACKUP died with Invalid argument - these files exist in every part.
  • A blob object is [envelope][payload] and the payload is the file. BlobLocation carries the payload offset, but getStorageObjects returns only key and length and getBlobPath keeps only the key, so the copy started at byte 0 and pulled the envelope into the backup. RESTORE then failed.

The first failure masked the second: checksums.txt aborted the backup before any blob corruption could surface. Fixing only the empty key would have turned a loud failure into backups that report success and cannot be restored.

The same bug was on the BACKUP ... TO Disk(...) path, for an s3/s3_plain disk on the pool's endpoint.

RESTORE onto CAS is not affected: it writes each part through one CAS transaction.

The fix

A CAS file is a byte window inside a shared object, so a whole-object server-side copy cannot express it. The change makes that a property of the data source instead of a special case in each caller.

  • DataSourceDescription gets files_are_whole_objects, and canUseNativeCopyWith requires it on both sides. The default is false, so a source that does not state the property gets the correct slow path rather than a wrong fast one.
    DiskObjectStorage derives it from metadata_storage->isContentAddressed; every other disk and the S3/Azure backup endpoints set it to true, so no non-CAS configuration loses its server-side copy.
  • BACKUP ... TO S3(...) from a CAS disk on the same endpoint copies a blob's payload with a ranged UploadPartCopy, taking the window from getBlobViewPlan. A payload larger than one upload part takes several requests, each carrying
    its own absolute range. Only UploadPartCopy can name a range, so ClickHouse falls back to reading and writing the payload itself when s3_allow_multipart_copy = 0, when the store is recognized as GCS, or when UploadPartCopy is
    refused with AccessDenied; any other refusal fails the BACKUP.
  • BACKUP ... TO Disk(...) reaches DiskObjectStorage::copyFile, which now declines the object-storage transaction for a CAS source and copies through buffers.

Other changes

  • copyS3File no longer issues CopyObject when the copy starts at a non-zero offset, because CopyObject cannot express a range and silently copied the whole object instead. No caller outside this change passes a non-zero offset
    today, so this is the contract the new ranged copy relies on rather than a user-visible fix

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Fixed BACKUP of a table on a CAS disk to an S3(...) or Disk(...) destination on the same S3 endpoint as the pool.

Documentation entry for user-facing changes

Adds docs/en/antalya/cas/operations/backup.md describing how BACKUP/RESTORE behave on a content-addressed disk, and links it from the CAS index and roadmap. Adds the ranged same-store copy to docs/en/antalya/cas/bucket-requirements.md.

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)

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

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [90dbe76]

Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
@k-morozov
k-morozov marked this pull request as ready for review September 24, 2026 15:50
@k-morozov
k-morozov marked this pull request as draft September 25, 2026 16:26
@k-morozov k-morozov changed the title CAS: backup native copy CAS: S3/Disk backup Sep 25, 2026
@k-morozov k-morozov changed the title CAS: S3/Disk backup BACKUP of CAS tables to S3 on the same endpoint Sep 25, 2026
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
@filimonov filimonov mentioned this pull request Sep 27, 2026
68 tasks
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
@k-morozov
k-morozov marked this pull request as ready for review September 29, 2026 06:51

@filimonov filimonov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things I would change before merge, one is shape and one is a bug.

Shape: the generic API should not learn about envelopes. The PR touches 23 files outside ContentAddressed/ (IDisk, IMetadataStorage, IObjectStorage, S3ObjectStorage, copyS3File, four disk wrappers, ObjectStorageQueue). That is a rebase cost on every release, and the new offset is a defaulted argument that future upstream callers of copyObjectToAnotherObjectStorage / getBlobPath will not know about, so blobs they copy will carry the envelope silently once inline files stop failing first.

The root cause is that DataSourceDescription::operator== and sameKind compare only type, object storage type and endpoint, so a CAS disk looks identical to a plain s3 disk on the same endpoint. Fixing it there keeps everything inside code we already patch:

  • In the DiskObjectStorage constructor (DiskObjectStorage.cpp:124), mark the description for a content-addressed metadata storage (suffix on description, CAS-gated). Then DiskObjectStorage::copyFile falls into IDisk::copyThroughBuffers, and all four backup writers fail sameKind and copy through buffers. Correct bytes, no generic diff. This also covers MOVE out of CAS (CAS-254).
  • copyS3File::performCopy has a real upstream bug: src_offset is ignored when the single-operation CopyObject is chosen. A 3-line fix (never single-op when offset != 0) is independent of CAS and can go upstream on its own.
  • If we want server-side copy for S3(...) destinations, one CAS-gated branch in BackupWriterS3::copyFileFromDisk can call copyS3File with the payload offset as the existing src_offset and a raw readObject fallback reader. No src_object_offset, no getObjectPayloadOffset, no wrapper changes.

Bug: zero-byte blob files. A wide part with an all-empty Array or all-NULL Nullable column has 0-byte arr.bin / n.bin (reproduced with the PR binary). partFileMustStayBlob keeps them blobs, every blob copy is ranged, and calculatePartSize(0) throws LOGICAL_ERROR. BACKUP skips empty files, IDisk::copyFile does not. Needs a size == 0 branch and a test.

Smaller items:

  • getObjectPayloadOffset is only called on the non-CAS branches and always returns 0 there; readInlineDataToString on CAS has no caller (the description says copyFileImpl uses it, it uses getContentAddressedFileCopySource).
  • The blob source is validated in the producer and again in both consumers; src_blob != copy_source.object and payload_size != object.bytes_size cannot be true by construction.
  • No gtest for the ranged copy, although S3ObjectStorageConditionalOpsTest in gtest_writebuffer_s3.cpp already has the mock and a LocalObjectStorage destination (the base IObjectStorage seek path is otherwise untested).
  • GCS: supportsMultiPartCopy is false there, so every blob falls back to the buffered path. Correct, but logged only at TRACE and not tested; worth a line in backend.md.
  • Cost: every blob is now Create + UploadPartCopy + Complete, even a 100-byte primary.idx.

k-morozov and others added 9 commits September 30, 2026 11:46
A column of all-empty `Array` values produces a zero-size `.bin`. Placement
keys on the file name, so `partFileMustStayBlob` keeps it a blob, and a ranged
server-side copy of a blob reaches `calculatePartSize(0)`, which throws.

`BACKUP` never meets such a file when files are deduplicated, but
`IDisk::copyFile` does, so `MOVE PARTITION` out of a CAS disk hits it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`DataSourceDescription::operator==` and `sameKind` compare the storage kind and
the endpoint, so a content-addressed disk looks identical to a plain s3 disk on
the same endpoint. Every server-side copy path then assumes a file is its whole
object starting at byte 0, which is false on a CAS disk: a blob-backed file is a
payload window behind a fixed-size envelope, and an inline file has no object of
its own.

Add `files_are_whole_objects` to `DataSourceDescription` and a predicate
`canUseNativeCopyWith` that requires it from BOTH sides. A precondition holds on
each side separately, so it is a conjunction rather than a comparison: two disks
that both lack the property are not thereby able to use it. `operator==` and
`sameKind` keep their meaning and stay reflexive, so callers that ask about disk
identity are unaffected.

The field is last in the struct because four call sites brace-initialize
`DataSourceDescription` positionally.

`BackupIO_AzureBlobStorage` does not use `sameKind` - it compares
`object_storage_type` directly - so both of its conditions check the capability
explicitly. This closes a reachable configuration rather than only future code: a
read-only CAS mount over Azure is constructible, because `Pool::open` skips the
conditional-write capability probe when the pool is opened read-only, and the
Azure native path copies a whole blob with no range.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Ten tests for the paths the capability gate touches, most of which no test
exercised before.

Three of them guard non-CAS behaviour, because the gate sits on a path every
object-storage disk takes:

- an ordinary s3 disk against an `encrypted` disk over the same s3. `sameKind`
  ignores `is_encrypted` and `DiskEncrypted` reports its delegate's
  description, so a predicate that replaced `operator==` rather than joining it
  would let the pair through and the cast to `DiskObjectStorage` would throw.
- two ordinary s3 disks keep their server-side copy. The capability defaults to
  false, so any description that forgets to claim it loses the fast path in
  silence.
- `BACKUP TO File(...)` keeps using `fs::copy`. Both sides take their
  description from one function, so a forgotten claim there gives false on both
  sides.

The rest cover CAS: a cache disk over CAS stays out of the server-side copy, a
zero-size file survives a backup when files are not deduplicated, an
incremental backup exercises a non-zero `start_pos`, and a backup written into
a CAS disk now succeeds where it used to be refused.

The mechanism test now also asserts that something went through buffers, so it
cannot pass when a part happens to hold no in-manifest entries.

The unit test pins the predicate's truth table and, separately, that
`operator==` stayed reflexive - the property that breaks if the conjunction is
folded into it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`BACKUP` to an `S3(...)` destination on the pool's own endpoint copies a blob's
payload without its header, and only `UploadPartCopy` can name a byte range. A
store without it is still supported - the blob goes through the ClickHouse
server - but the distinction belongs in the capability table next to the other
store requirements, where an operator choosing a backend will look for it.

`GCS` is the store this applies to today: its current XML API has no
`UploadPartCopy`, so `Client::supportsMultiPartCopy` reports `false`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_backup_to_s3_with_empty_array_column` and
`test_incremental_backup_of_a_cas_table` repeat what
`test_backup_to_s3_with_empty_arrays` and `test_incremental_backup_to_s3`
already do, and the existing pair is stronger: the empty-file one asserts that
a zero-size file actually reached the backup writer, rather than only checking
the data after a restore.

Two more cluster round-trips for no new coverage is time every CI run pays.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three fixes from the whole-branch review.

`BackupIO_AzureBlobStorage` went back to checking the capability flags directly
instead of `canUseNativeCopyWith`. The predicate carries `sameKind`, which
compares the description, and the two Azure sides build that string
differently: a disk reports `Endpoint::getServiceEndpoint`, while the backup
reports `ConnectionParams::getConnectionURL`, which for a `connection_string`
disk parses that string and returns the service URL instead. Requiring them
equal would have taken Azure's native copy away from a working configuration
with no error and no log line. The capability is the only dimension this work
argued about, so it is the only one the Azure conditions gained.

`ReadOnlyDiskWrapper` now forwards `getS3StorageClient` and
`tryGetS3StorageClient`. It already forwards `isContentAddressed` and
`getMetadataStorage`, so a CAS disk behind the wrapper reaches the new backup
branch, which then asks the disk for its client and got `NOT_IMPLEMENTED` from
the base class - an exception that escapes the copy rather than falling back to
buffers.

`test_backup_to_file_keeps_fs_copy` asserted that the backup read zero bytes
through a file descriptor. It cannot: `BackupFileInfo` checksums every entry
without a precalculated hash, and `checksums.txt`, `columns.txt`, `count.txt`
and the codec and version files have none. The assertion now compares what was
read against the part's size on disk, which is what "the data did not go
through the server" actually means.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
k-morozov and others added 13 commits September 30, 2026 21:54
`BACKUP TABLE t TO Disk('<cas disk>', ...)` does not work, and the test that
claimed it does was wrong. A backup's own layout mirrors the table's data
directory - `.../data/<db>/<table>/all_1_1_0/<file>` - and
`Cas::isPartFilePath` looks for a part-directory component anywhere in the
path, so a backup file counts as a part file and the autocommit refusal
applies. The test now asserts the refusal, and the documented limitation says a
`CAS` disk cannot be a backup destination rather than claiming the opposite.

`test_backup_to_file_keeps_fs_copy` had its bound the wrong way round. Every
entry pays one checksum pass through `ReadBufferFromFileDescriptor`, so one
pass over the data is what a working `fs::copy` looks like, and the run that
failed - 6832069 bytes read against 6830303 on disk - was the fast path doing
its job. A second pass is what a lost `fs::copy` would cost, so the assertion
is now against one and a half passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_backup_to_s3_with_empty_arrays` counted objects in the backup with
`_size = 0` through the `s3` table function, which cannot report them:
`s3_skip_empty_files` defaults to true, and the listing drops a zero-size
object before the `WHERE` ever sees it. The assertion would have failed even
with the file sitting in the backup.

The query now turns that setting off.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
@DimensionWieldr

Copy link
Copy Markdown
Collaborator

PR #2415 CI Triage

Nothing in this run is a regression from the CAS backup change. The 22 red checks are 9 regression jobs, 2 Grype scans, and status contexts that repeat those same results.

Of 49 failing scenario rows in the regression reports, 29 are parent nodes that failed because a child failed. The 20 leaves are four groups.

Summary

Category Count What
regression 0
cascade 29 Parent scenarios of the leaves below
pre-existing-flaky 20 leaves uniqApacheHLL, CAS selects, concurrent lightweight delete, Azure invalid disk
infrastructure 2 jobs Grype on the keeper image and the alpine server image

Compared across other open and merged antalya-26.6 PRs (check rollups, plus the reports for #2420, #2452, #2455, and #2456). The CI database was not queried.

uniqApacheHLL — pre-existing-flaky

/aggregate functions/part 1/uniqApacheHLL, .../finalizeAggregation/uniqApacheHLL_finalizeAggregation_Merge, .../state/uniqApacheHLLState, .../merge/uniqApacheHLLMerge, on both CAS and CAS S3 cache (8 leaves).

AggregateFunctionUniq.cpp on antalya-26.6 and on this PR's head (90dbe764) registers uniq, uniqHLL12, uniqExact, and uniqTheta. It does not register uniqApacheHLL. The server answers UNKNOWN_FUNCTION / UNKNOWN_AGGREGATE_FUNCTION. This PR does not touch that file.

The suite started calling the function on 1 Oct 2026 (57cff3f90 in clickhouse-regression). #2456, an Iceberg backport, fails the non-CAS aggregate_functions_1 job with the same error. #2455 passed those jobs on 30 Sep, before the test existed. The function is added by #2398, which is still open.

CAS selects — pre-existing-flaky

Eight /selects/final/force/concurrent/* leaves, all Code: 210 content-addressed disk 'cas_disk' -- mount lease not held.

The same leaves and the same error are on merged #2452. The release cas_selects job also failed on #2468, #2463, #2456, #2455, #2437, #2440, and #2420. #2468's aarch64 run passed, so the rate is high and not 100%. This PR does not change lease renewal. #2474 is the in-flight fix for the renewal giving up.

Concurrent lightweight delete — pre-existing-flaky

/lightweight delete/concurrent delete/MergeTree/random delete 75 percent of the table without overlap left 255 rows against an expected 250. The half-table scenario left 502 against 500. The scenario runs the same delete list twice in parallel.

The shard fails on other PRs with a different leaf: #2455 failed random delete entire table without overlap. #2440 passed the shard. #2456's CAS shard passed and its S3-cache shard failed. The non-CAS shard passed on #2456, #2440, and #2420.

Azure invalid disk — pre-existing-flaky

/s3/azure/part 1/invalid disk/access default and access failed. The test in s3/tests/disk_invalid.py requires the log to contain Server failed to authenticate. ConfigReloader logged Azure 403 This request is not authorized to perform this operation.

The same two leaves and the same 403 are on #2420 (29 Sep). s3_azure_1 also failed on #2440 (two runs) and #2456; s3_azure_2 passed on those runs. This PR's Azure edit is the backup reader/writer setting files_are_whole_objects. The failure is the disk access check at config load.

Grype — infrastructure

Keeper: High CVE-2026-85091, plus Alpine 3.21 CVE-2026-84782, CVE-2026-84784, CVE-2026-72897. Alpine server: High CVE-2026-85091. The same two jobs fail on #2444 (docs) and #2471 (unrelated server change). The image scan will stay red until the base image is rebuilt.

Recommendations

  1. These jobs do not block BACKUP of CAS tables to S3 on the same endpoint #2415. A rerun will not clear uniqApacheHLL or the Azure invalid-disk assertion, and the CAS selects and lightweight-delete jobs fail on unrelated PRs.
  2. uniqApacheHLL goes green when Antalya 26.6: add UniqApacheHLL #2398 registers the function, or when the suite stops calling it before that.
  3. The Azure assertion should accept the 403 authorization text the server logs now.
  4. CAS selects wait on the mount-lease renewal fix (CAS: retry the mount lease renewal until the store answers #2474).

@Slach

Slach commented Oct 2, 2026

Copy link
Copy Markdown

@k-morozov will BACKUP support server side copy for TO AzureBlobStorage(...) clause? from azblob CAS?

@k-morozov

Copy link
Copy Markdown
Author

@k-morozov will BACKUP support server side copy for TO AzureBlobStorage(...) clause? from azblob CAS?

CAS does not support Azure as a backend. If this support is added, we will separately investigate safe Azure server-side copy in addition to the current buffered copy.

@filimonov filimonov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@filimonov

Copy link
Copy Markdown
Member

some extra AI feedback (nothing blocks the merge):

  • getBlobPath on a CAS disk still returns [key, bucket]. The gate is opt-in per call site, and sameKind still treats CAS as equal to plain s3 (BackupIO_Disk, BackupIO_File use it; safe today through the copyFile gate and the type mismatch). A future upstream caller that pairs sameKind with getBlobPath would copy envelopes. Returning an empty vector from DiskObjectStorage::getBlobPath for a content-addressed disk makes that fail closed; no current caller reaches it for CAS.
  • Initializer churn. The four DataSourceDescription initializers in BackupIO_S3.cpp and BackupIO_AzureBlobStorage.cpp could stay positional with , true appended. The field is last for that reason, and it keeps the fork patch to one line each.
  • Unrelated hunks. The prepareRead LOGICAL_ERROR, the view -> manifest_view rename (which also dropped the comment explaining bytes_size), and the blank line in DiskObjectStorage.h are not part of this fix.
  • Range check error code. start_pos/length in tryNativeCopyFromContentAddressedDisk come from base-backup metadata, so BAD_ARGUMENTS fits better than LOGICAL_ERROR.
  • Description nit. "No caller outside this change passes a non-zero offset" is not literal: BackupImpl passes base_size. It is zero for immutable part files in practice.

@filimonov
filimonov merged commit f3c650a into antalya-26.6 Oct 6, 2026
551 of 585 checks passed
@DimensionWieldr DimensionWieldr added verified Approved for release verified-with-issues Verified by QA and issues found. labels Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.6 CAS verified Approved for release verified-with-issues Verified by QA and issues found.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants