transfer,transport: detect backend degradation and stop leaking bandwidth during control-API outages - #1
Open
full-bars wants to merge 4 commits into
Open
transfer,transport: detect backend degradation and stop leaking bandwidth during control-API outages#1full-bars wants to merge 4 commits into
full-bars wants to merge 4 commits into
Conversation
…idth during control-API outages When the control API is unreachable, providers keep creating contracts, expanding client windows and retransmitting against an endpoint that cannot authorize anything. The work has nowhere to go, so it is spent bandwidth. On metered links a sustained outage can consume a large share of a monthly allowance while serving no client. Add a shared degradation signal driven by the two round-trips that actually touch the backend (platform auth and contract OOB). It requires backendDegradedFailThreshold consecutive failures with no intervening success, and the last failure must be within backendDegradedWindow, so isolated timeouts on a busy provider never trip it. Any successful round-trip clears it immediately, so recovery is not on a timer. While degraded: - skip CreateContract; every request is an OOB round-trip that cannot succeed - start contract retries at the backed-off interval instead of the fast first retry, composing with the existing nextCreateContractRetryInterval backoff - do not expand the multi-client window; each added client needs its own contract Also rate-limit the four log lines that flood under the same fault ([t]auth error, [contract]oob err, [ts]->error, [r]drop) to one per minute with a suppressed count, via a small shared logThrottle. These are emitted per sequence per retry, so during an outage they are the dominant log volume and can push out the lines needed to diagnose it. Each falls back to -v=1 so no detail is lost when the level is raised.
📝 WalkthroughWalkthroughThe change adds atomic backend degradation tracking and recovery, throttles repeated error logs, and adjusts contract creation and client window expansion while the backend remains degraded. Tests cover thresholds, stale failures, recovery, interleaving, and concurrent access. ChangesBackend resilience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant H1H3 as H1/H3 connection loops
participant BackendState as backend degradation state
participant ContractCreation as contract creation
participant ClientWindow as client window expansion
H1H3->>BackendState: Record authentication failure or success
BackendState-->>ContractCreation: Return degraded status
ContractCreation->>BackendState: Check backend health
BackendState-->>ContractCreation: Skip or resume contract request
ClientWindow->>BackendState: Check backend health
BackendState-->>ClientWindow: Block or allow window expansion
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)❌ Error committing Unit Tests locally. ❌ Error committing Unit Tests locally.
Comment |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transfer.go`:
- Around line 3282-3295: Guard the queued-contract prefetch CreateContract call
in nextContract with isBackendDegraded(), or centralize the same check inside
ContractManager.CreateContract so all callers are covered. Preserve normal
prefetch behavior when the backend is healthy, and add a regression test
covering a queued contract while the backend is degraded.
In `@transport.go`:
- Around line 154-158: The noteBackendFailure state update must discard an
expired failure streak before counting the new failure. Synchronize the
stale-age check, timestamp refresh, and counter reset/increment as one
transition, preserving the existing backendDegradedWindow behavior; add a test
covering a stale threshold-sized streak followed by one failure and assert
isBackendDegraded remains false.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b9b2b22-b9ac-432a-b3d4-65df3c4c2b8d
📒 Files selected for processing (7)
backend_degraded_test.goip_remote_multi_client.golog_throttle.golog_throttle_test.gotransfer.gotransfer_contract_manager.gotransport.go
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @full-bars. The following files were modified: * `log_throttle.go` * `transfer.go` * `transfer_contract_manager.go` * `transport.go` These files were ignored: * `backend_degraded_test.go` * `log_throttle_test.go`
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
…fetch Two issues from review. noteBackendFailure kept the count from a streak that had already aged out of backendDegradedWindow. isBackendDegraded ignores stale failures, but the counter did not, so an idle provider that saw a few failures long ago and stopped retrying would carry the old count forward: one new failure pushed the total past the threshold with a fresh timestamp, and the backend read as degraded on the strength of a single recent failure. The stale check, counter adjustment and timestamp refresh are now one serialized transition, and an aged-out streak restarts at 1 instead of resuming. The gate in createContract's retry loop did not cover the CreateContract call in nextContract. After TakeContract returns an already-queued contract, that path prefetches the next one. Sequences still holding queued contracts would therefore keep issuing OOB requests during an outage, which is exactly the traffic the gate exists to stop. Gated at the call site: both callers are in this one function, so centralizing the check inside ContractManager would put transport policy in the contract manager for no additional coverage. Adds regression tests for both, including that a gap shorter than the window still accumulates so a slow outage is not missed.
|
Request timed out after 900000ms (requestId=4294791d-51f0-48ac-8065-aad405a12102) |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Request timed out after 900000ms (requestId=4006c77d-aafb-418a-8a75-592dff9e5ad1) |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Request timed out after 900000ms (requestId=9ad34922-edbe-4519-ba28-d58ec9d76350) |
The gates live inside SendSequence.updateContract and multiClientWindow.resize, which need a live Client and ContractManager to reach, so these assert the decision each gate makes rather than standing up the surrounding machinery. Each test names its call site and mirrors that site's expression, so an edit to one and not the other shows up in review. Covers: contract creation suppressed only once the threshold is reached and re-enabled on the first success; window expansion requiring both room and a healthy backend; contract retry starting already backed off while degraded. Also covers nextCreateContractRetryInterval, the existing upstream backoff this change composes with. It had no tests, and starting at the maximum while degraded is only correct if repeated application actually converges there and never overshoots.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend_degraded_gate_test.go (1)
17-43: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftTest the production gate paths.
These tests reproduce the predicates locally. They do not call the production gates. A removed or inverted gate in
SendSequence.updateContractormultiClientWindow.resizewill still pass.
backend_degraded_gate_test.go#L17-L43: ExerciseSendSequence.updateContractwith a controlledContractManager. Assert that degraded state suppressesCreateContract.backend_degraded_gate_test.go#L48-L74: ExercisemultiClientWindow.resize. Assert that degraded state prevents client expansion.backend_degraded_gate_test.go#L79-L104: ExerciseSendSequence.updateContract. Assert that degraded state passesCreateContractRetryMaxIntervalas the first retry timeout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend_degraded_gate_test.go` around lines 17 - 43, Replace the local predicate-only test at backend_degraded_gate_test.go:17-43 with an integration-style test that invokes SendSequence.updateContract using a controlled ContractManager and verifies degraded state suppresses CreateContract. Extend backend_degraded_gate_test.go:48-74 to call multiClientWindow.resize and verify degraded state prevents client expansion. Extend backend_degraded_gate_test.go:79-104 to exercise SendSequence.updateContract and assert degraded state supplies CreateContractRetryMaxInterval as the first retry timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend_degraded_gate_test.go`:
- Around line 17-43: Replace the local predicate-only test at
backend_degraded_gate_test.go:17-43 with an integration-style test that invokes
SendSequence.updateContract using a controlled ContractManager and verifies
degraded state suppresses CreateContract. Extend
backend_degraded_gate_test.go:48-74 to call multiClientWindow.resize and verify
degraded state prevents client expansion. Extend
backend_degraded_gate_test.go:79-104 to exercise SendSequence.updateContract and
assert degraded state supplies CreateContractRetryMaxInterval as the first retry
timeout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f3913b0-c78b-4272-a4fa-bea9c1b6bc55
📒 Files selected for processing (6)
backend_degraded_gate_test.gobackend_degraded_test.golog_throttle.gotransfer.gotransfer_contract_manager.gotransport.go
🚧 Files skipped from review as they are similar to previous changes (4)
- log_throttle.go
- transfer_contract_manager.go
- transport.go
- transfer.go
|
Request timed out after 900000ms (requestId=773f0feb-ea91-46db-930a-c28e879a2f76) |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
14 UNAVAILABLE: Connection dropped |
This was referenced Aug 5, 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
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.
Stop leaking bandwidth when the control API is unreachable
When the control API is unreachable, a provider keeps requesting contracts against an
endpoint that cannot authorize anything. Every sequence retries on its own schedule, so the
requests never stop, and none of them can succeed until the API returns. On metered links,
which is the common case for proxy providers, a sustained outage can burn a large share of a
monthly allowance while serving no traffic.
The root problem is that nothing tells the provider the backend is down. This PR adds that
signal and uses it.
The signal
isBackendDegraded()is true only when both hold: at leastbackendDegradedFailThreshold(3) consecutive failures with no intervening success, and themost recent failure is within
backendDegradedWindow(2 minutes). It is fed by the only tworound-trips that actually touch the backend: platform transport auth, and the contract OOB
request.
The consecutive-failure requirement is what separates a real outage from normal churn.
Isolated timeouts happen constantly on a busy provider, but they are interleaved with
successes, and any success resets the counter. A real outage fails every attempt with
nothing to reset it. The recency window then stops a stale count, left by an old blip on an
idle provider, from reading as a live outage.
Note
The threshold is deliberately conservative. Reacting after three consecutive failures is
slower than reacting to the first, and that is the intended trade: a false positive stalls
contract creation for a provider whose backend is fine, which is worse than a few extra
seconds of leak at the start of a real outage.
What is gated while degraded
CreateContractis skippednextCreateContractRetryIntervalbackoff rather than replacing it.Each gate is a skip, not a teardown. Nothing is discarded, and sequences resume on their
normal path as soon as a successful round-trip clears the state.
Log throttling for the same fault
The same outage floods the logs:
[t]auth error,[contract]oob err,[ts]->errorand[r]dropare emitted per sequence per retry, so they become the dominant volume and can pushout the lines needed to diagnose the problem. Each is now limited to one line per minute with
an
(N suppressed)tail, via a small sharedlogThrottle, falling back toV(1)so nothingis lost at higher verbosity.
The throttles are package level rather than per instance. A per-instance limiter would still
emit once per client per interval, which on a provider with thousands of clients is not a
limit at all.
logThrottle.Allowis lock free: when goroutines race the same intervalboundary exactly one emits and the rest are counted, which is covered by a test.
Validation
The detection and gating have been running in production on a provider fleet
(https://github.com/full-bars/urnetwork-3.23-fix) for several months, across real
control-API outages. The thresholds, window and gates here are the values that fleet runs,
not new tuning proposed in this PR.
The effect is large: with contract creation gated and retries paced back, bandwidth consumption drops
to a small fraction of baseline for the duration of the outage. Two honest caveats. It
reduces the leak rather than eliminating it, since data already in flight still drains. And
traffic briefly rises after recovery as queued work clears, which is queues draining rather
than useful transfer resuming.
Deliberately not included
SendSequencealready backs off multiplicatively per resendand caps at
MaxResendInterval. Nothing to add.a single OOB failure. Dropped: it never ran in production, and triggering on one failure is
exactly the false positive the 3-failure threshold exists to prevent.
is emitted under sustained fault, never whether it is emitted.
Supersedes
Replaces urnetwork#180 and urnetwork#182, which overlapped badly (each rewrote the same log lines, and both
declared the same helper functions, so they could not merge in either order) and both went
stale. This is the two of them refreshed against current main as one change, minus the parts
upstream has since solved on its own.