Skip to content

GCS: retry transient upload and token failures - #37

Draft
achudnovskij wants to merge 1 commit into
mainfrom
storage/gcs-retry
Draft

achudnovskij wants to merge 1 commit into
mainfrom
storage/gcs-retry

Conversation

@achudnovskij

Copy link
Copy Markdown
Contributor

Why

On GCS, a single transient error fails an upload outright; on S3 the same error is retried. Found while moving wal-listener to GCS (ClickHouse/wal-listener#25). There, a failed DR-tail PUT during failover cuts the tail short, and a failed fallback-archive PUT stalls that pass.

Two gaps, both GCS-only:

  1. Uploads were a single streamed attempt. GcsStorage::put / put_if_absent streamed the body once. S3 buffers bodies up to its 32 MiB single-PUT threshold and retries in place. GCS only got RetryingStorage's retries, which require size_hint <= 8 MiB. Compressed or encrypted WAL is pushed with size_hint = None, so in practice no GCS WAL upload was retried. That covers walrus wal-push and every library user.
  2. Token-endpoint failures were never retried. Any non-2xx from the metadata server or oauth2.googleapis.com became StorageError::Auth, which is_transient() always rejects. The S3 IMDS path instead returns Http{status}, so its 5xx is retried. With node identity (GCS: Support Node/VM identity auth. #33) the metadata server is called on every hourly token refresh, so a brief 503 or 429 there failed every GCS operation in that window, reads included.

What changes

  • put and put_if_absent buffer bodies up to BUFFERED_UPLOAD_LIMIT (32 MiB, the same as S3's threshold) and retry transient errors with with_retry. With no size hint, it reads one byte past the cap to detect overflow, the same way S3's put does. An overflowing body is chained back together and streamed once, as before. A resumable upload would let those retry too, but it's a larger change and WAL never needs it.
  • The retry policy is now passed in from config (GcsStorage::with_retry_policy, as S3Storage does). GcsStorage::new keeps its signature and uses the default policy.
  • The token is fetched inside each upload attempt, so the retry also covers a token-refresh blip.
  • Token-endpoint errors: 408/425/429/5xx return StorageError::Http (transient); other 4xx stay Auth. A bad key, missing scope, or missing service account still fails immediately.
  • put_if_absent semantics are unchanged. If an attempt succeeded but its response was lost, the retry gets 412 and reports AlreadyExists, the same as S3.

Worth a look

  • Nested retries. For bodies ≤ 8 MiB with a known size, RetryingStorage retries around the backend's own retries. S3 already behaves this way, so I kept the two backends consistent rather than fixing it here.
  • Memory. Each upload ≤ 32 MiB now holds one buffered copy while in flight. S3 already does the same.

Tests

The new tests run against the in-process mock (test_http::serve):

test covers
put_retries_transient_failures_for_unknown_size_bodies 503×2 then 200 with size_hint=None → 3 attempts, body replayed intact; 403 not retried; attempts exhausted → last 503 returned
put_streams_oversized_bodies_in_one_attempt a body over 32 MiB is stitched back together correctly (None hint) and not retried (Some hint)
put_if_absent_retries_then_maps_outcome 503 then 200 → Created; 503 then 412 → AlreadyExists; ifGenerationMatch=0 sent on every attempt
token_endpoint_errors_are_transient_only_for_throttling_and_5xx 503/429 transient, 403/404 Auth; end to end, a metadata server that fails once no longer fails the put

Mutation check: I reintroduced each bug separately. Forcing the old single streamed attempt fails 3 of the new tests; forcing token errors back to Auth fails the token test.

Local gates (rust:1-bookworm): cargo fmt --check ✅, cargo clippy --all-targets --locked -D warnings ✅, cargo test --locked ✅ (486 lib tests + integration). Not run locally: pg-compat against fake-gcs-server; CI runs it.

🤖 Generated with Claude Code

GCS uploads were a single streamed attempt, so one 503, 429 or connection
reset failed the put. S3 buffers bodies under its 32 MiB single-PUT
threshold and retries in place; GCS only got RetryingStorage's retries,
which need a known size_hint <= 8 MiB. Compressed or encrypted WAL is
pushed with no size hint, so in practice no GCS WAL upload was retried.

Buffer put and put_if_absent bodies up to 32 MiB (the S3 threshold; every
WAL segment fits) and retry transients with the storage retry policy, now
threaded through from config like S3's. Bodies over the limit, or that
overflow it with no size hint, still stream once: they can't be replayed
without buffering the whole thing, and a resumable upload is a larger
change. A retried put_if_absent whose earlier attempt landed but lost its
response gets 412 and reports AlreadyExists, the same as S3.

Also classify 429/5xx from the token endpoint (metadata server or
oauth2) as transient Http errors instead of Auth, which is never
retried. With node identity the metadata server is hit on every hourly
refresh, so a brief blip there failed every operation in that window.
The token is fetched inside each upload attempt, so the retry covers it.
4xx stays Auth.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@serprex serprex 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.

Thinking we should use backon if we're going to keep adding retry logic, it's what walshadow uses, but that can be followup

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants