Repository navigation
Retry GCS rate-limit errors when deleting and copying snapshots - #2143
Merged
Max Smythe (maxsmythe) merged 1 commit intoOct 21, 2026
Merged
Max Smythe (maxsmythe) merged 1 commit into
Max Smythe (maxsmythe) merged 1 commit into
Conversation
Sairaj Pokale (sairajp-rewind)
force-pushed
the
fix/gcs-write-retries
branch
from
October 7, 2026 08:42
7ec9f36 to
5d1746d
Compare
Collaborator
|
LGTM, thanks Sairaj for the updates and the extra test cases! |
Collaborator
|
Julian Gutierrez Oschmann (@juli4n) could you help approve this PR as well? Thanks. |
Chenyi Wang (chw120)
requested a review
from Julian Gutierrez Oschmann (juli4n)
October 8, 2026 23:26
Collaborator
|
LGTM. Let's see if Max Smythe (@maxsmythe) has any other suggestions. Thanks. |
Max Smythe (maxsmythe)
enabled auto-merge
October 17, 2026 03:58
auto-merge was automatically disabled
October 19, 2026 17:00
Head branch was pushed to by a user without write access
Sairaj Pokale (sairajp-rewind)
force-pushed
the
fix/gcs-write-retries
branch
from
October 19, 2026 17:00
1578354 to
99616c8
Compare
Collaborator
Author
|
Rebased onto main. The e2e CI wouldn't trigger and the module rename needed a one-line import fix in the new test (99616c8); nothing else changed. Max Smythe (@maxsmythe) could you re-approve? |
Sairaj Pokale (sairajp-rewind)
force-pushed
the
fix/gcs-write-retries
branch
2 times, most recently
from
October 19, 2026 22:46
0788d0c to
661c6dd
Compare
GCS answers 429 while it scales a cold bucket up to a burst of suspends. ateapi's GCS store used the default retry policy, which never retries a delete or copy without a precondition, so a single 429 failed a snapshot release, an actor delete, a revert or a tag copy. Give every ateapi GCS call RetryAlways with a backoff from 250ms up to 5s and at most 7 attempts. atelet: drop the 5-attempt cap, which never applied to uploads, and back off from 250ms up to 5s, so calls retry until their context ends. Scratch cleanup ran on context.WithoutCancel with no deadline, deleted one object at a time, and failed an upload that had already succeeded when a delete failed. It now deletes 8 objects at a time within a 30s timeout and only logs what it could not delete. Note in setRetry that an upload the SDK sends as resumable also stops once a chunk has been retrying for ChunkRetryDeadline.
Sairaj Pokale (sairajp-rewind)
force-pushed
the
fix/gcs-write-retries
branch
from
October 20, 2026 01:05
661c6dd to
dc7d790
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does and why we need it ?
GCS answers 429 while it scales a cold bucket up to a burst of suspends. ateapi
used the default retry policy, which never retries a delete or copy without a
precondition, so a single 429 failed a snapshot release or a tag copy. List did
retry, but with no cap, until the caller's context ended.
This change:
RetryAlways, backing off from 250ms up to 5s, at most 7 attempts per request
(at most 12.75s of sleep). The settings sit on the bucket handle, so objects
inherit them. That includes the copy's destination, which is where the SDK
reads them.
WithMaxAttempts(5), which never capped uploads, and backsoff from 250ms up to 5s. On transient errors, compose, delete, copy and reads
now retry until their context ends instead of stopping after 5 attempts.
removeScratch, for upload parts and composeintermediates) ran on
context.WithoutCancel(ctx)with no deadline, sowithout the cap it could retry forever. Each cleanup now deletes its objects 8
at a time within a 30s timeout. It is best-effort: objects it can't delete are
logged and left next to the object, instead of failing an upload that already
succeeded.
RetryAlways is safe here. A retried delete that already landed gets a 404,
which Delete already treats as success. A retried copy writes the same
unchanging source to the same destination.
Behavior change
and tag create and delete. Delete and copy used to fail on the first 429, so
for them this is only more retrying. List used to retry until the caller's
context ended. In a long GCS outage, these calls now fail after at most 12.75s
of sleep:
actor is deleted.
ateapi starts over gRPC, that is its deadline of at most 10 minutes. The
golden-actor template reconciler is the exception: it calls
SuspendActor,ResumeActorandDeleteActorin-process with no deadline, so the ateletcalls it causes have none. Uploads already ignored the cap there. A follow-up
can bound those calls.
Writer's
ChunkSize) also gives up once a chunk has been retrying for 32s,the Writer's default
ChunkRetryDeadline.Testing
internal/objectstore/gcs_retry_test.go: an httptest fake GCS answers ascripted number of errors per call. The tests check attempt counts, not
timing (they back off 1ms), and run in
make testwithout an emulator.pkg/objectstorage/gcs_retry_test.go, against the same kind of fake:upload's context is already canceled.
WithMaxAttempts(5)in atelet, no cleanup timeout, cleanup on the upload'scontext, deleting one object at a time, or returning the cleanup error.
-race.