Skip to content

Never resend a CAS blob's object-creating S3 request - #2555

Open
filimonov wants to merge 13 commits into
antalya-26.6-20261008from
fix/antalya-26.6/cas-361-single-attempt-blob-creation-split
Open

filimonov wants to merge 13 commits into
antalya-26.6-20261008from
fix/antalya-26.6/cas-361-single-attempt-blob-creation-split

Conversation

@filimonov

@filimonov filimonov commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

On S3-compatible stores the ETag of a single PUT or CopyObject is derived from the bytes. When the SDK resent a CAS blob publication after the first attempt had already landed, and GC had condemned and deleted that incarnation in between, the resend brought back the old ETag. A delayed exact-ETag DELETE from GC then removed a blob that a newer writer had just reused, leaving a committed reference without data. Blob publication now sends each object-creating request at most once; the CAS publish loop retries with a fresh envelope instead.

Changes

  • S3ObjectStorage::copyObject honours the write settings' retry override, as writeObject already does.
  • New WriteSettings::object_storage_single_attempt_object_creation: PutObject, CopyObject and CompleteMultipartUpload go out once, with adaptive first-attempt timeouts off (there is no second attempt). CreateMultipartUpload, UploadPart and UploadPartCopy keep their retries, so large multipart blobs still survive throttling. The flag also bounds the NO_SUCH_KEY reissue loops of WriteBufferFromS3 and copyS3File to one request (applyWriteErrorRetryOverride). A copy with the flag must use ObjectStorageCopyMode::NativeOnly; copyObject refuses anything else with BAD_ARGUMENTS.
  • Client::CompleteMultipartUpload on a single-attempt client no longer reports NoSuchUpload as success when some object exists at the key, and skips the GCS compose (a second creating request; off by default).
  • A failed single-attempt creating request is not logged at Error: WriteBufferFromS3 logs it at Debug, copyS3File logs its multipart failure at Debug, and an abort that finds no upload after a lost completion is not an error on either path. A NO_SUCH_KEY loop no longer announces "will retry" on its last attempt.
  • CAS blob publication (ObjectStorageBackend::publish) sets the flag. PartWriteTxn::ensureBlobPresent logs each failed attempt at Warning (rate-limited) and counts it in CASBlobPublishAttemptFailed; CASBlobPublishGaveUp counts publications that ended with an exception after a failed attempt, at the attempt cap or the deadline.

Commits 1–3 and 6 contain no CAS code; commits 4 and 7 wire the flag into CAS; commit 5 reworks the tests; commit 8 updates the CAS docs; commits 9 and 10 address review follow-ups (abort log on the copy path, the give-up counter, test trims, wording).

Relation to the earlier retry work

  • d528693 (2026-09-05), on top of the adc0fa7 engine, forbade the CAS engine from reissuing a blob publication and recorded this hazard. The transport below stayed on the retrying client: the SDK retry strategy, Client::doRequestWithRetryNetworkErrors and the NO_SUCH_KEY loops.
  • The only single-attempt tool then was the control-plane write settings: the SingleAttempt retry profile plus conditionalWriteSettings, with a 5 s attempt budget and a single PUT on GCS, which 940b168 had just removed for blobs. The profile alone swaps the client for the whole buffer, so one lost UploadPart loses the whole multipart body.
  • This PR adds the missing per-verb split: creating requests go out once, parts keep their retries. It completes that work rather than fixing a mistake in it.
  • The "fresh envelope" argument of 940b168 holds per engine attempt, not per wire request: an SDK resend carries the same bytes.
  • The CaBlobPublishCore model had no resend below the engine until the 2026-10-12 audit added SdkResend.

Risks / notes

  • Retries are not removed, they move up one level. Before: the S3 SDK and Client::doRequestWithRetryNetworkErrors resent the same request (same bytes, same ETag). Now a failed or ambiguous creating request fails the attempt, and PartWriteTxn::ensureBlobPresent retries it. Each attempt first re-observes the key (HEAD of the body, then the marker) and, if the blob is still needed, publishes a new envelope with a fresh incarnation_tag, so it can never recreate a condemned ETag. A failed CopyObject of a staged blob is retried as a streaming upload (the staged copy is used once). Definite refusals (e.g. access denied) are not retried, as before.
  • Retry budget: a creating request runs under the disk's request_timeout_ms, with its connect timeout capped like the control plane's. The loop makes up to 8 attempts and starts no new one after the 90 s deadline (Retry::standard), then the insert fails with a retryable error. An attempt already started can run past the deadline. A failed CompleteMultipartUpload or CopyObject re-uploads the whole body.
  • Mixed versions: the fix applies per writer node. A node without it can still resend a blob PUT it publishes.
  • The CompleteMultipartUpload change reaches every single-attempt clone, including the CAS control-plane conditional writes. It is more conservative there: NoSuchUpload becomes an S3 error, the write classifies as unresolved, and the engine settles it with a read. Reporting another writer's object as this upload's success was wrong for them too.
  • Residuals: CreateMultipartUpload, UploadPart and AbortMultipartUpload on the failure path keep the disk client's retries (about 500 by default), so one engine iteration can block much longer than 90 s; a follow-up measures and bounds that tail. A CompleteMultipartUpload slower than 30 s on RustFS/MinIO was previously rescued by a retry plus the NoSuchUpload → HEAD check; it now costs a re-upload. Neither is measured.
  • Part requests (CreateMultipartUpload, UploadPart, UploadPartCopy) keep their SDK retries unchanged.
  • Resends after a definite refusal (credentials refresh, region redirect, HTTP 307) are kept; they cannot duplicate an applied request.
  • Generation-token stores (GCS) were not affected by the loss, only by the NoSuchUpload rewrite.

Testing

  • New suites, red before their fix: S3ObjectStorageCopyObject, S3SingleAttemptObjectCreation (lost responses and NO_SUCH_KEY per creating verb, no Error log for a lost response or for the abort after a lost completion (write and copy), no "will retry" on the last NO_SUCH_KEY attempt, a broken response body, part/create/part-copy retries kept, EntityTooLarge → one multipart copy, adaptive timeouts off on the creation clone plus one slow PUT, default writers still resend), S3ClientSingleAttemptCompletion.
  • CAS: CASBlobPublishTransport.LostCompleteIsOneWireRequest drives the backend directly, and two composition tests run the real S3ObjectStorage under the CAS publication loop against a storing S3 stub: a lost PUT is republished after a new HEAD with a different incarnation_tag; a lost staged CopyObject is republished by streaming. CASBackend.ConditionalMultipartNoSuchUploadIsUnresolved pins the control-plane effect.
  • Gates on the final tree: release CAS* 2627/2627, ASan CAS* 2631/2631; S3ObjectStorageCopyObject*:S3SingleAttempt*:S3ClientSingleAttempt* release 33/33, ASan 34/34 (includes the death test).
  • TLA+: CaBlobPublishCore reproduces the loss with an SDK resend of an identical envelope on content-ETag stores and is green with single-attempt creation; generation stores are green either way.

Changelog category (leave one):

  • Critical Bug Fix (crash, data loss, RBAC)

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

Fixes a rare data loss on CAS (content-addressed storage) disks backed by S3 stores with content-derived ETags (GCS is unaffected): a retried blob upload could bring back a blob that garbage collection had already scheduled for deletion, and the delayed deletion then removed a blob still in use. Blob uploads no longer resend the request that creates the object; CAS retries with a new upload instead. Under a store outage an insert may now fail with a retryable error after 8 attempts or once the 90-second deadline for starting attempts passes.

Documentation entry for user-facing changes

No user-facing setting; the new WriteSettings field is internal. The CAS docs (blob-protocol.md, backend.md, correctness.md, configuration.md, troubleshooting.md) and the Antalya profile events page describe the new behaviour and counters.

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)

filimonov and others added 4 commits October 12, 2026 23:41
`copyObject` passed the disk's request settings to `copyS3File` unchanged,
so `s3_max_unexpected_write_error_retries_override` bounded the `NO_SUCH_KEY`
loop of `writeObject` but not the identical `CopyObject` loop. Apply it to a
local copy of the request settings, as `writeObject` does.

Add a counting S3 test server and an `S3ObjectStorage` test factory for
per-verb wire counts.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A resent `PutObject`, `CopyObject` or `CompleteMultipartUpload` that the
store already applied can add a version on a versioned bucket, emit a
second `ObjectCreated` event, or land after a newer write of the key and
overwrite it. A writer that cannot accept that sets
`WriteSettings::object_storage_single_attempt_object_creation`.

- `S3ObjectStorage::writeObject` and `copyObject` then pass a second client
  to `WriteBufferFromS3` and `copyS3File`: a single-attempt clone with the
  configured timeouts. Adaptive first-attempt timeouts assume a second
  attempt, which this client never makes.
- Only the creating requests use it. `CreateMultipartUpload`, `UploadPart`
  and `UploadPartCopy` keep the retrying client.
- A copy must use `ObjectStorageCopyMode::NativeOnly`; `copyS3File` throws
  `LOGICAL_ERROR` otherwise, because the read-write fallback would write
  through the retrying client.
- `getSingleAttemptClient` takes `configured_timeouts` as part of its cache
  key, so existing single-attempt callers keep their clones unchanged.

Writes without the flag are unchanged.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`Client::CompleteMultipartUpload` turns `NoSuchUpload` plus a successful
`HEAD` of the key into success, on the theory that an earlier attempt of
the same request completed the upload and its response was lost. A
single-attempt client sent the request once, so the object at the key
may come from another writer. Return the original error for such a
client.

Skip the optional GCS self-compose for it too: the compose rewrites the
object as a new generation, a second creating request.

Clients with the retrying strategy are unchanged. The change also applies
to existing single-attempt callers.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Blob publication ran with default `WriteSettings`, so the S3 client and the
`NO_SUCH_KEY` loops could resend an identical envelope. On a content-ETag
store a resend that lands after GC deleted that incarnation restores its
ETag, and a late exact-token delete then removes a body another writer
republished.

`ObjectStorageBackend::publish` now sets
`object_storage_single_attempt_object_creation` and a retry override of 1
for both the streaming upload and the staged copy. `ensureBlobPresent`
already retries with a fresh envelope after re-observing the key.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [fc72b78]

filimonov and others added 8 commits October 15, 2026 14:00
- `CountingS3Server.h` gets `faulting`, `dropping` and `failing` responders; the
  per-verb lost-response, `NoSuchKey` and `NoSuchUpload` cases become `TEST_P`
  tables instead of copied bodies.
- Exception codes are checked instead of `EXPECT_ANY_THROW`, and the broken
  response body test no longer swallows the exception.
- Two of the three slow timeout tests become an assertion on the clone's
  `s3_use_adaptive_timeouts`; one 3.5 s `PUT` test stays end to end.
- Default-write negative controls cover `CompleteMultipartUpload` too.
- The test client factories keep `RemoteHostFilter` alive: the client
  configuration holds a reference to it.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The caller of a single-attempt object-creating request resolves the outcome
and retries, so one failed attempt is not an operator error:

- `WriteBufferFromS3::makeSinglepartUpload` logs it at `Debug` when the
  creation client is single-attempt, as it already did for the
  `SingleAttempt` profile.
- An abort that finds no upload after a lost single-attempt completion logs
  at `Information`, and its 404 is expected.
- `copyS3File` logs a failed multipart upload at `Debug` on that path.
- A `NO_SUCH_KEY` loop says "will retry" only when it will.

`object_storage_single_attempt_object_creation` now also bounds the
`NO_SUCH_KEY` reissue loops to one request (`applyWriteErrorRetryOverride`,
shared by `writeObject` and `copyObject`), and `copyObject` refuses the flag
without `NativeOnly` with `BAD_ARGUMENTS`; `copyS3File` keeps the check as a
`chassert`. `configured_timeouts` is renamed `disable_adaptive_timeouts`.

`CountingS3Server.h` records request paths and bodies and gains an in-memory
`ObjectStore`.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`PartWriteTxn::ensureBlobPresent` retried a failed publication silently.
Each failed attempt now logs a rate-limited `Warning` (key, transport,
attempt n of N, error) and counts `CASBlobPublishAttemptFailed`; running out
of attempts counts `CASBlobPublishGaveUp`.

Blob publication no longer sets the `NO_SUCH_KEY` override itself: the
single-attempt creation flag implies it.

Tests:
- Two composition tests run the real `S3ObjectStorage` under the CAS
  publication loop against a storing S3 stub: a lost `PUT` is republished
  after a new `HEAD` with a different `incarnation_tag`; a lost staged
  `CopyObject` is republished by streaming.
- A conditional multipart write whose completion answers `NoSuchUpload`
  stays unresolved and settles as a conflict with another writer's object.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `blob-protocol.md`: the creating request goes out once; retries restart at
  `HEAD`, up to 8 attempts in one 90-second window, then a retryable error.
- `troubleshooting.md`: the attempt ceiling applies to blob uploads, with the
  counters to watch.
- `backend.md`, `configuration.md`: blob creation runs under the disk's
  `request_timeout_ms` with the connect cap, outside `cas_attempt_timeout_ms`.
- `correctness.md`: the `SdkResend` counterexample.
- `metrics_and_events.md`: the new counters and the reworded
  `S3SingleAttemptRetryConsultations`.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `copyS3File`'s abort after a lost single-attempt completion no longer logs
  the 404 at `Error`, as `WriteBufferFromS3` already does; the no-`Error` test
  covers both the write and the copy path.
- `CASBlobPublishGaveUp` now also counts an exit at the loop's deadline: any
  exception out of `ensureBlobPresent` after a failed attempt counts once.
- Tests: the per-verb `CASBlobPublishTransportFault` table is gone (the S3
  suites cover it); one lost-`Complete` test stays in the `CAS*` gate. The
  unused `CopyObject` apply in `ObjectStore` is removed.
- Comments: `WriteSettings.h` keeps the "after an ambiguous outcome"
  qualifier; a long line in `S3ObjectStorage.h` is wrapped.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Blob creation is never resent after an ambiguous outcome; resends after
  a definite refusal stay.
- `CASBlobPublishGaveUp` counts exits at the attempt cap or the deadline.
- `correctness.md` describes the SDK-resend case in plain words and drops
  the stale sabotage count.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ture test

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@filimonov
filimonov marked this pull request as ready for review October 16, 2026 12:42

@k-morozov k-morozov left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Model found:
The documentation promises a 90-second wall-clock bound that the code does not enforce.

return result;
}

void S3ObjectStorage::copyObjectToAnotherObjectStorage( // NOLINT

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems here also need to override settings + objectCreationClient.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in fc72b78: the S3-to-S3 shortcut in copyObjectToAnotherObjectStorage now applies the write-error retry override and the single-attempt creation client, and requires NativeOnly like copyObject. A non-S3 destination is rejected when the flag is set (BAD_ARGUMENTS without NativeOnly, NOT_IMPLEMENTED with it). A new test checks that a lost CopyObject or NoSuchKey is one wire request.

The S3-to-S3 shortcut ignored the write settings: it neither applied the
unexpected-write-error retry override nor used the single-attempt creation
client, so a lost CopyObject was resent. It now mirrors copyObject, including
the NativeOnly requirement. A non-S3 destination is rejected under the flag
(NOT_IMPLEMENTED in NativeOnly mode).

Docs: the 90-second figure is a deadline for starting attempts, not a
wall-clock bound; a started attempt can run longer.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@filimonov

Copy link
Copy Markdown
Member Author

The 90 s is a deadline for starting attempts, not a wall-clock bound: no new attempt starts after it, but an attempt already started can run longer (each creating request waits up to request_timeout_ms, and part uploads keep their own retries). The docs and the PR description are corrected in fc72b78. A follow-up measures and bounds the longer tail.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants