Skip to content

enable gRPC client retries with jitter by default#549

Closed
k4leung4 wants to merge 3 commits into
chainguard-dev:mainfrom
k4leung4:normalize-retry-config
Closed

enable gRPC client retries with jitter by default#549
k4leung4 wants to merge 3 commits into
chainguard-dev:mainfrom
k4leung4:normalize-retry-config

Conversation

@k4leung4

@k4leung4 k4leung4 commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Change the default gRPC client retry configuration to provide baseline
resilience against transient Unavailable errors (connection failures,
load balancer overloads, Cloud Run cold starts).

  • Default GRPC_CLIENT_MAX_RETRY from 0 → 2
  • Change backoff from BackoffExponential(100ms) to
    BackoffExponentialWithJitter(100ms, 0.2) to desynchronize retriers
    and prevent thundering herd on recovery
  • Explicitly retry only codes.Unavailable (request never reached server)

Motivation

Addresses CUS-172 /
chainguard-dev/internal-dev#18988,
a follow-up from INC-44 where internal usage spikes overwhelmed the
shared load balancer and caused cascading dial timeouts.

Current state in mono (11 gRPC clients from the API server)

Client Retry max Backoff Codes
IAM datastore 0 (default)
Registry datastore 2 Linear 50ms Unavailable, Internal
Ecosystems datastore 2 Linear 50ms Unavailable, Internal
Packages datastore 2 Linear 50ms Unavailable, Internal
Policy gates 2 Linear 50ms Unavailable, Internal
Apkoaas 2 Linear 50ms Unavailable, Internal
Vulnerabilities datastore 0 (default)
OIDC 0 (default)
Image recommender 0 (default)
Workqueue 0 (default)
GitHub identity 0 (default)

Problems:

  • 6/11 clients have zero retries — transient connection failures surface
    as user-visible errors immediately
  • The 5 clients with retries use linear 50ms backoff with no jitter,
    causing synchronized retry waves (thundering herd)
  • No client has keepalive configured except apkoaas (6 min), so idle
    connections are silently dropped by Cloud Run after ~10 min, causing
    reconnection storms under load

Design decisions

Why codes.Unavailable only (not codes.Internal):
Unavailable means the request never reached the server — always safe
to retry. Internal means the server received and potentially processed
the request. Retrying non-idempotent mutations (e.g., datastore Create
RPCs) after an Internal error could produce AlreadyExists instead of
success if the first attempt committed but the response was lost. The 5
per-client overrides in mono that add codes.Internal made an explicit
choice that this tradeoff is acceptable for those services.

Why default 2 (not higher):
Matches the retry count already used by the 5 production clients that
have been tuned. With exponential backoff (100ms base + 20% jitter),
2 retries complete within ~300ms, keeping tail latency reasonable.

Override: Services can set GRPC_CLIENT_MAX_RETRY=0 to disable
retries entirely if needed.

Follow-up in mono

After this lands and mono bumps go-grpc-kit, the per-client retry
overrides in mono can be simplified:

  • Registry, ecosystems, packages, policygates, apkoaas: Remove the
    grpc_retry.WithBackoff(BackoffLinear(50ms)) and
    grpc_retry.WithMax(2) — they now get 2 retries with jitter from the
    default. They only need to keep grpc_retry.WithCodes(Unavailable, Internal)
    to retain the Internal retry behavior.
  • IAM, vulnerabilities, OIDC, recommender, workqueue, GitHub identity:
    Automatically gain 2 retries on Unavailable with no code changes.

Test plan

  • cd pkg/options && go test ./...
  • Deploy to staging, verify retry metrics in Prometheus
    (grpc_client_handled_total with retry labels)
  • Confirm no increase in duplicate mutations

🤖 Generated with Claude Code

for
https://linear.app/chainguard/issue/CUS-234/enable-grpc-client-retries-with-jitter-by-default
https://linear.app/chainguard/issue/CUS-235/add-keepalive-to-all-grpc-clients

Signed-off-by: Kenny Leung <kleung@chainguard.dev>
@k4leung4 k4leung4 marked this pull request as ready for review March 16, 2026 19:11
Kenny Leung and others added 2 commits March 16, 2026 12:18
Signed-off-by: Kenny Leung <kleung@chainguard.dev>
Verify the server was called exactly once, confirming retries were
actually suppressed for codes.Internal (not just that the error code
was correct).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

🤖: Retry-with-jitter is well-implemented — backoff config, code filtering, and WaitForReady are all correct. Two concerns.

Comment thread pkg/options/options.go
EnableClientStreamReceiveTimeHistogram bool `envconfig:"ENABLE_CLIENT_STREAM_RECEIVE_TIME_HISTOGRAM" default:"true"`
EnableClientStreamSendTimeHistogram bool `envconfig:"ENABLE_CLIENT_STREAM_SEND_TIME_HISTOGRAM" default:"true"`
GrpcClientMaxRetry uint `envconfig:"GRPC_CLIENT_MAX_RETRY" default:"0"`
GrpcClientMaxRetry uint `envconfig:"GRPC_CLIENT_MAX_RETRY" default:"2"`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖: Flipping default:"0"default:"2" silently activates retries for every consumer of GRPCDialOptions(). During outages this adds ~300ms latency and up to 3x RPC traffic before errors surface. Worth calling out in the PR description and/or a CHANGELOG entry so teams can opt out with GRPC_CLIENT_MAX_RETRY=0 if needed.

return &healthpb.HealthCheckResponse{Status: healthpb.HealthCheckResponse_SERVING}, nil
}

func TestGRPCDialOptions_RetriesUnavailable(t *testing.T) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖: sync.OnceValue means state() is locked to MaxRetry=42 from TestGetEnv before this test runs. The headline invariant — retries capped at 2 by default — has no test coverage. Consider adding a test that calls envconfig.Process("", &env) on a fresh struct and asserts env.GrpcClientMaxRetry == 2.

@k4leung4 k4leung4 closed this Jun 18, 2026
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