fix(control_plane): retry transient errors on startup POST to backend (404/connect-refused) - #293
Merged
Merged
Conversation
… (404/connect-refused) In the multinode harness (asap arm from ASAPCollector PR #394), the controller and backend start concurrently. The backend logs `HTTP server listening on port 9091` before its `/api/v1/streaming-config` route is actually registered, and the controller's PR #290 startup `Replanner::replan_all()` tick fires before the backend's HTTP server has finished accepting connections. Observed symptom: all seven startup `POST /api/v1/streaming-config` calls returned 404; convergence relied entirely on the `OpampServer::on_connect` re-fire when the first agent connected. This patch adds retry-with-backoff for transient failures in `emit::backend_push::post_typed_backend_for_role` (the single helper PR #290 unified both call sites through). Transient: 404, 5xx, connection refused / DNS / connect timeout, reqwest `IsTimeout` / `IsConnect`. Permanent (no retry): 4xx other than 404. Policy: 5 attempts, exponential 100ms -> 300ms -> 900ms -> 2.7s (capped) with full jitter, total span ~5-8s. Lock-discipline unchanged: the `backend_routing_cache` lock is dropped before any HTTP call, so retries never stall sibling replan cycles. `BackendClient`'s existing public methods are untouched (new typed variants `_typed` returning `BackendPostError` sit alongside); `OpampServer::on_connect` remains as the secondary convergence mechanism. Unit tests: 4 new tests in `emit::backend_push::tests` cover recovery-after-404, no-retry-on-permanent, exhaustion-reports- attempts, no-retry-on-first-success; 4 new tests in `backend_client::tests` cover the typed classifier (404/500 transient, 400 permanent, connection-refused transient). All 748 `cargo test -p control_plane --lib` pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Race we are patching
In the multinode harness (asap arm from ASAPCollector PR #394), the controller and backend start concurrently. We observed:
Convergence today depends on
OpampServer::on_connect(added in PR #290) re-firingreplan_all()when the first agent connects. That's a workaround — the first POST is wasted, and any deployment without OpAMP connects (or where the controller restarts before agents reconnect) hangs.Fix
Add retry-with-backoff for transient errors in
control_plane/src/emit/backend_push.rs::post_typed_backend_for_role(the single helper PR #290 unified both call sites through).Transient (retry): 404 (route not yet registered), 5xx, TCP connection-refused / DNS / connect timeout, reqwest
IsTimeout/IsConnect/IsRequest.Permanent (no retry): 4xx other than 404.
Policy: 5 attempts, exponential 100ms -> 300ms -> 900ms -> 2.7s (capped) with full jitter, total span ~5-8s. Bounded by
RETRY_MAX_ATTEMPTS=5andRETRY_DELAY_CAP=2.7s. The lock onbackend_routing_cacheis dropped before any HTTP call (preserved from PR #290's pattern), so retries never stall sibling replan cycles.Public-interface preservation:
BackendClient's existing methods are untouched. New typed variantspost_streaming_config_json_typed/post_storage_routing_json_typedreturningResult<(), BackendPostError>sit alongside; the retry layer uses only those.OpampServer::on_connectre-fire is preserved as the secondary convergence mechanism.Tests
cargo build -p control_plane— cleancargo test -p control_plane --lib— 748 passed, 0 failedemit::backend_push::tests:retry_transient_recovers_after_first_404— closure returns 404 then 200; asserts attempts > 1 and final Ok (the explicit ask in the task)retry_transient_does_not_retry_permanent_errors— closure returns 400; asserts attempts == 1retry_transient_exhausts_and_reports_attempts— closure returns connection-refused 5x; asserts attempts == 5 and final Transient Errretry_transient_no_retry_on_first_success— protects the smoke-test happy-path invariantbackoff_delay_respects_cap— every attempt's delay <= capbackend_client::tests:typed_streaming_config_404_is_transienttyped_streaming_config_500_is_transienttyped_streaming_config_400_is_permanenttyped_connection_refused_is_transienttyped_streaming_config_success_is_okAll five retry-loop tests use
tokio::test(start_paused = true)so they execute in microseconds despite the multi-second nominal backoff window.Test plan