refactor(BA-5862): Retry once on stale aiodocker connections after dockerd restart#11235
Open
rapsealk wants to merge 3 commits into
Open
refactor(BA-5862): Retry once on stale aiodocker connections after dockerd restart#11235rapsealk wants to merge 3 commits into
rapsealk wants to merge 3 commits into
Conversation
…erd restart The shared aiodocker client (introduced in #11226) pools keepalive sockets in its aiohttp.ClientSession. After a dockerd restart, the first post- restart call can pick a stale socket and fail with ClientConnectionError or ServerDisconnectedError; aiohttp reconnects on the next call. A thin once-retry wrapper absorbs that one-shot failure so user-visible operations (check_image, scan_images, purge_images, etc.) don't spuriously fail after dockerd bounces. - Wraps retry-safe aiodocker calls via _retry_on_stale_connection. - Leaves monitor_docker_events (own reconnect loop) and the DockerStatsStreamer (bounded backoff) untouched. - Non-idempotent / retry-risky calls (commit, etc.) are explicitly not wrapped. Closes #11233 Refs #11216 Refs #11226 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size:L
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous catch tuple (ClientConnectionError, ServerDisconnectedError) subsumed ServerTimeoutError via ClientConnectionError. A long-running images.push / images.pull that blew its timeout would be silently retried and emit a misleading "stale aiodocker connection" warning. Narrow to (ServerDisconnectedError, ClientOSError), which covers the real pooled-socket-dead cases (dockerd restart / keepalive reset) without catching legitimate timeouts. ClientConnectorError (dockerd down) and ClientSSLError also correctly fall through. Add a regression test asserting ServerTimeoutError is NOT retried, and a test exercising container.create's retry path. Refs #11233 Refs #11235 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
rapsealk
changed the title
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
1 participant
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.
Closes #11233 (BA-5862)
Refs #11216
Summary
_retry_on_stale_connectionhelper that catches(ServerDisconnectedError, ClientOSError)once, logs a warning, and retries.ServerTimeoutError— even though it inherits fromClientConnectionError, it represents response-too-slow, not a stale socket. Legitimate long-runningimages.push/images.pulltimeouts now propagate normally.DockerAgent(image / container / network inspect / list / delete / create / start / stop — 26 sites across 13 methods).monitor_docker_events(own reconnect loop),DockerStatsStreamer(bounded backoff),__ainit__bootstrap probes (boot-failures should be loud), streaming log readers,commitand other non-idempotent verbs.Why
After
systemctl restart docker, the shared aiohttp session's pooled keepalive sockets are stale. aiohttp reconnects on the next call, but the first one-shot failure surfaces as spurious user-facing errors (purge_images,check_image, etc.). See #11233.Stacked on
#11226 (
perf/11218-docker-client-pool). Rebase to main once #11226 merges.Test plan
pants test tests/component/agent/docker::passes — 5 tests: retry-once-then-succeeds, propagates persistent failure, non-matching errors not retried,ServerTimeoutErrorNOT retried (regression guard),container.createretry path viaresolve_image_distro.docker restart; runbai admin image listfrom the CLI; verify it succeeds on the first call post-restart (would fail pre-this-PR).images.pullpast its timeout; verify it propagates without silent retry.