Bound flush() retries so a persistently-retriable upload can't hang teardown#289
Merged
Conversation
…eardown The synchronous `flush()` on the shutdown path retried a `.retry` upload result in an unbounded loop. A persistently retriable server (503/500/ 408/429) or a downed network — both surface as `needsRetry` — would loop forever, hanging process teardown. Cap flush retries at 3 (1 initial attempt + 3 retries). On giving up, report failure and leave the undelivered batch on disk for a later run. The background worker's indefinite cross-tick retry behaviour is unchanged; only the bounded shutdown flush is affected. Adds a regression test that a persistently-retriable upload returns failure instead of hanging, attempts exactly 4 uploads, and leaves the batch on disk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![datadog-prod-us1-6[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Retry bounding logic is correct: persistently-retriable uploads now give up after 4 total attempts (1 initial + 3 retries), exit gracefully instead of hanging, and leave the undelivered batch on disk for a later run. Off-by-one semantics, batch preservation, and return value semantics are all sound. No regressions to existing tests.
📊 Validated against 4 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit a37f691 · What is Autotest? · Any feedback? Reach out in #autotest
`testFlushGivesUpAfterMaxRetriesInsteadOfHanging` flushed while the worker was still live, so its scheduled background tick fired right after flush released the queue and added a stray 5th upload (expected 4, got 5). Stop the worker before flushing, matching the production shutdown order in `Feature.stop()` (stop periodic uploads, seal writer, then final flush) so `flush()` is the only thing uploading the batch. Deterministic 4 uploads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gnufede
approved these changes
Jul 15, 2026
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
2 participants
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
The synchronous
flush()on the shutdown path retried a.retryupload result in an unbounded loop. A persistently retriable server (503/500/408/429) or a downed network — both surface asneedsRetry— would loop forever, hanging process teardown.Change
maxFlushRetries.flush()is affected.Test
Adds
testFlushGivesUpAfterMaxRetriesInsteadOfHanging: a persistently-retriable upload returns failure (not a hang), performs exactly 4 upload attempts, and leaves the batch on disk.🤖 Generated with Claude Code