Skip to content

Implemented the poll chain to await the download future that serialize polling - #169

Merged
sspaink merged 8 commits into
open-policy-agent:mainfrom
polachandu:fix/serialize-bundle-polling
Aug 12, 2026
Merged

Implemented the poll chain to await the download future that serialize polling#169
sspaink merged 8 commits into
open-policy-agent:mainfrom
polachandu:fix/serialize-bundle-polling

Conversation

@polachandu

@polachandu polachandu commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Re-serialize by chaining each poll off the previous download's completion instead of a fixed timer — the same "single serial loop" guarantee that Go-OPA has. This makes bundle (and discovery) polling strictly serial, so overlapping polls can no longer run activateBundle() — and thus mutate the shared Store — concurrently.

Added a test confirming that downloads remain serialized even when activation exceeds the poll interval, so overlapping downloads cannot trigger concurrent activateBundle() calls.

Closes #113

@polachandu
polachandu force-pushed the fix/serialize-bundle-polling branch from dad3896 to 9cf6dfe Compare July 24, 2026 05:46
@polachandu
polachandu marked this pull request as ready for review July 24, 2026 06:27
@polachandu
polachandu requested a review from a team as a code owner July 24, 2026 06:27
@polachandu

Copy link
Copy Markdown
Contributor Author

Hi @sspaink, please review this PR. Please post your comments/suggestions if any, happy to incorporate.

delay,
() ->
downloadBundle()
.whenComplete(

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.

The request has no .timeout(...) and the clients only set connectTimeout. Java's HttpClient has no default response timeout, so a server that accepts and never replies leaves sendAsync pending forever: whenComplete never fires, scheduleDownload is never called again, and polling stops permanently with nothing logged.

One line next to requestBuilder.build() fixes it:

requestBuilder.timeout(Duration.ofSeconds(/* e.g. the service's responseHeaderTimeoutSeconds */));

The timeout fails the future, handleHttpResponse logs it, and the chain keeps polling. (Prefer this over .orTimeout(...), which doesn't cancel the request — the stalled exchange could still land later and call activateBundle alongside a fresh poll.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looking into it.

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.

Ah, my bad — requestBuilder.timeout() covers the whole exchange, not just headers, so wiring it to a 10s value would've killed any download slower than that. Real problem, and my suggestion walked you into it.

I'd rather not add download_timeout_seconds though. OPA doesn't have a bundle download deadline at all, and response_header_timeout_seconds is headers-only — "does not include the time to read the response body." The catch is the buffering body handler, not the timeout: switch to BodyHandlers.ofInputStream() and the future completes as soon as headers land, so the deadline only covers headers and a slow body just keeps streaming. No 300s default needed. Bonus, cancel the original future on timeout and my orTimeout gripe goes away too.

@polachandu
polachandu force-pushed the fix/serialize-bundle-polling branch from ee3e36b to c34be21 Compare August 3, 2026 18:59
@polachandu
polachandu requested a review from sspaink August 3, 2026 19:03
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
@polachandu
polachandu force-pushed the fix/serialize-bundle-polling branch from cc3ba06 to ff6c4ab Compare August 12, 2026 02:35
Signed-off-by: Chandrahas Reddy Pola <polachandrahas@gmail.com>
@sspaink
sspaink merged commit ecb5997 into open-policy-agent:main Aug 12, 2026
20 checks passed
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.

BundleDownloader: async download allows overlapping polls → concurrent activateBundle/Store mutation

2 participants