Repository navigation
Phase 6a: retry — the policy core and both stacks - #72
Merged
Merged
Conversation
Add Dexpace::Resilience -- Policy (the two-axis classifier consult, the
backoff calculator, the total pacing-header parser over the private
PacingParsers, the retry-count resolver, the recovery-only budget and
RETRY-12's constants), Resend.eligible? (the re-sendability gate over
phase 1's idempotent set and phase 3b's replayability), and the frozen
RetrySettings both retry stacks build from, the first reader of 5a's
Keys::MAX_RETRY_ATTEMPTS -- plus the flat Dexpace::RetryPredicateError.
Widen two earlier-phase files in place: ProtocolError bakes XCUT-5's
retryable_by_status? from 5a's Retryability at construction (phase 4b's
postponement; deliberately not #retryable?, P6-10), and HTTPDate's day
group becomes (\d{1,2}) so RETRY-15's single-digit day parses through the
one RFC 1123 parser (R1); 5a's rejection loop swaps that element for the
bare-date row CFG-31 still refuses.
One calculator, two budget policies: backoff_delay takes no total-timeout
and budget_remaining is a separate function only the recovery engine
names (R6, P6-5). A zero initial delay answers 0.0 before the power is
taken, because 0.0 * Infinity is NaN and [NaN, 8.0].min raises (P6-53).
Widen Pipeline::Cursor with one read-only member, #bundle -- CTX-14's per-call correlation bundle -- seeded by a new optional `bundle:` keyword on Pipeline#call and AsyncPipeline#call (Bundle::NONE when omitted), validated in Cursor.build, threaded through both private drivers' #advance and carried across #fork exactly as #options is. No existing signature moves, and Transport.conforms? still accepts both runtimes because the keyword is optional beside the SPI's three positionals (P6-51). The one consumer is 5b's instrumentation step, on both runtimes: #open_span now takes the cursor's bundle and prefers its tracer factory when the bundle is not NONE, else the step's own keyword, and Tracing.correlate is handed the same bundle, so a populated one pushes its trace and span ids onto the diagnostic context. That is the reconciled precedence's first clause, which phase 5 recorded as having no implementation path; a call that seeds nothing resolves exactly as phase 5 did. The bundle carries no operation name, so the span is still named by the method token (P6-52), and it is not the retry step's HTTP-tracer source (P6-7). Existing tests only widened where the code required it: the cursor's public-method pin gains #bundle, and the two step suites' hand-rolled cursor stand-ins answer #bundle with NONE.
Add Resilience::RetryStep and Resilience::AsyncRetryStep at Stages::RETRY, over the private RetryStepHelpers mixin that holds the decision, the delay resolution, the two override hooks and the trail handling both share. Each drives the downstream chain once per attempt through a fresh Cursor#fork -- the first drive included -- so hop 1 and hop n are one object and each attempt re-executes the chain with fresh per-attempt state (RETRY-44); neither writes cursor state of its own. The synchronous step waits through 5a's cancellable Clock#sleep and checks the token at the top of every attempt. The async step is a trampoline, not a recursion: one Pump per call resumes under a re-arm flag held by a Thread::Mutex across the flip only, so an inline settlement re-enters #resume without growing the stack (the design's recursive sketch overflows at about 1,500 attempts on every supported Ruby); a positive delay goes through Dexpace::Async.delay, which without a Fiber.scheduler raises SeamError synchronously and fails the future with the trail attached (R2's third route), and a zero-length delay continues inline. A settled fatal answering #retryable? is delivered unclassified rather than retried (P6-55). OBS-29's per-attempt group is emitted through the tracer the `http_tracer_factory:` keyword produces once per operation, called with the cursor (R3, P6-7); retries_exhausted fires only when a retryable failure met a spent budget. The interface _HTTPTracer phase 5c declined to declare arrives in the http_tracer sig with the wiring, `context` untyped because the recovery stack hands a Request where the stage stack hands a Cursor. The error-status response is closed before the wait and before any throwable propagates (RETRY-35); no total-timeout is named on this stack (RETRY-28, P6-5).
Add Resilience::RecoveryRetry, chapter 9's first stack (RECOV-17 to RECOV-30 and RECOV-34, which phase 4 postponed), installed as Recovery::Orchestrator's `transport:` decorator rather than as a ResponseChain step (P6-3): Transform#apply carries no request to re-send, and only a transport-decorator position can dispatch its own re-sends while the request chain's stamping stays once per exchange. Sitting below the orchestrator's rescue, a raising transport is retried here as an exception (P6-8), each attempt's response is classified through phase 4b's own Recovery.buffer_error_body then ProtocolError.for_or_nil -- the buffering is what frees the connection before the wait -- and the two terminal shapes are a returned response when the failure was never retryable and a raised throwable otherwise, with every prior failure attached as suppressed (P6-9). This is the only stack with a total-timeout: the budget is a maximum-attempts cap counting the first send as attempt 1 (max_retries plus one, an identity, P6-6) and the time remaining through Policy.budget_remaining, so a delay that would overshoot is suppressed and the last failure surfaced. The HTTP tracer is produced once per operation with the Request, there being no cursor on this stack. The entry file now requires the whole retry layer, the entry test pins the six Resilience constants and the flat error with the private helpers unreachable, and the runtime surface manifest gains the forty-eight rows the phase's public surface adds.
Review round 0 of the phase-6a stack found four behaviours the design states and the code did not fully honour, and one comment nit. The pacing parsers converted an unbounded digit run: a 10 MB header value stalled the retry decision for seconds in String#to_f / #to_i and, past ~309 digits, emitted Ruby's out-of-range warning that the suite's NFR-6 raiser turned into an error Policy#parse_form's rescue swallowed. Both grammars now bound their runs at fifteen digits -- the largest run a Float carries exactly, and 10**15 seconds is thirty million years, so a longer run is RETRY-16's out-of-range value and answers nil without a conversion -- and a 64-byte ceiling in front of every parser keeps the HTTP-date attempt from reading a hostile value either (the longest well-formed form is the 29-byte RFC 1123 date). RetryStep#wait emitted the tracer's attempt_failed outside the RETRY-35 fence, so a tracer that raised left the superseded response open on the sync driver while the async driver closed it. The emission now runs inside the same fence as the delay resolution, so the response is closed before the raise propagates on both drivers. A caller's should_retry answering true retried a downstream CancelledError. Policy.cancellation? walks the cause chain for a CancelledError; Policy.retryable? answers false for one before either branch, and the stage drivers' decision answers :stop for one before the re-sendability gate and before the predicate, so RETRY-23's "never" holds on all three drivers whatever a capability or a predicate says. A negative configured MAX_RETRY_ATTEMPTS raised at RetrySettings.build, so RETRY-41's clamp-and-log was unreachable from any driver. RetrySettings.build takes a logger: keyword, read once at build, and resolves the configured value through Policy.effective_max_retries, so a negative configured value is clamped to the default and the clamp logged as one contained config diagnostic; an explicit negative max_retries: is still refused by RECOV-34. Also: step.rb's class comment cited P6-52 for the no-operation-name statement, which is P6-51's; the RBS mirrors follow each change and the core manifest gains Policy#cancellation? (1004 -> 1005 rows).
Review round 1 of the phase-6a stack found the sync/async drift round 0 closed for attempt_failed still open on the terminal path: RetryStep's settle emitted the tracer's retries_exhausted outside any fence, so a tracer that raised there propagated while the terminal error-status response it had just been handed stayed open, whereas the async pump's finish already ran inside its guarded block and closed it. The trail attachment and the retries_exhausted emission now run inside the same fence as the decision and the delay resolution, so a throwing tracer propagates (OBS-30) with the terminal response closed first on both drivers; a tracer that does not raise leaves the returned response open exactly as before (RETRY-34), and the exception path is untouched. The class comment and the method's comment state the terminal path too.
This was referenced Sep 18, 2026
This was referenced Sep 19, 2026
Closed
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.
Part of #22. First PR of phase 6a's three-PR stack — the code. Its tests are the next PR up and the phase record the one after. Cut from
mainatf1fe848(phase 5 complete); phase 6c (#24) was built in parallel off the same base and lands as its own stack — whichever merges second gets the rebase-and-reprove pass.What lands
The retry subsystem in
dexpace-core— 45 files, +2,165 / −72 — the largest sub-phase in the roadmap:RETRY-1–RETRY-45and the fifteenRECOVIDs phase 4 handed here (RECOV-17–RECOV-30,RECOV-34), plus four items earlier phases postponed. One policy core, two stacks, no second calculator.Dexpace::Resilience::Policy(extend self) — the tuning constants (200 ms / ×2 / 8 s / 0.2 jitter / 3 sends,RETRY-12),retry_eligible?(status, set:)withRETRY-37's authoritative-contains semantics (a configured set that narrows, narrows),throwable_retryable?asXCUT-6's capability query overDexpace.each_cause(neveris_a?(::IOError)— P6-4's blind spot is stated),backoff_delay(saturating at the cap; a zero initial delay guarded, P6-53),effective_max_retrieswithRETRY-41's clamp logged,budget_remainingreferenced by the recovery engine only (P6-5's text-scannable split),pacing_delayover the privatePacingParsers(Retry-Afterseconds or HTTP-date,retry-after-ms/x-ms-retry-after-ms,X-RateLimit-Resetepoch; every grammar bounded to fifteen digits behind a 64-byte screen, P6-61, so a hostile header answersnilin microseconds rather than stalling), andcancellation?(RETRY-23: aCancelledErroranywhere in the chain is never retryable, P6-60).Resend.eligible?—RETRY-5–RETRY-8/RECOV-18overMethod#idempotent?andBody#replayable?. Only.eligible?: 6b's.replayable_body?andNotReplayableErrorextend this module when 6b lands.RetrySettings— one frozenData(includesModel), two consumers;max_retriesfrom 5a'sKeys::MAX_RETRY_ATTEMPTSwith the stage vocabulary fixed (max_attempts = max_retries + 1, P6-6), a caller's status set owned (RECOV-34),randomdefaulting to the::Randomclass (P6-52),logger:for the clamp diagnostic (P6-59).RetryStep/AsyncRetryStepatStages::RETRY— fork for every drive, the first included, neverCursor#call; close-before-wait with the tracer emission inside the same fence (RETRY-35, R0-4/R1-1);cancellation.check!at the top of every attempt (Clock#sleep(0.0)returns before its token check); the async driver a real trampoline — a private per-callPumpwith arunning/rearmflag pair (P6-54), measured flat across 2,001 attempts, because the design's recursive sketch overflows at ~1,500 on every supported Ruby; a positiveAsync.delaywith noFiber.schedulerfails the returned future withSeamError(R2 route 3), a zero-length delay re-arms inline (RETRY-30/31); no scheduler keyword.RecoveryRetry— the recovery-stack engine installed asRecovery::Orchestrator'stransport:(P6-3), rescuing::StandardErrorand classifying (P6-8; the fatal family passes untouched,RETRY-25),RECOV-20's total-timeout with zero = unbounded and the per-attempt deadline clamped,RECOV-19's re-classification of every re-sent response (503, 503, 200 terminates on the 200), the terminal throwable raised with the trail andcause: nil(P6-9,RETRY-34).OBS-29's per-attempt group emitted by all three drivers throughhttp_tracer_factory:(a callable called once per operation with the cursor, or the request on the recovery stack — P6-7), defaulting to the NULL tracer;retries_exhaustedfires only on a retryable failure at a spent budget (P6-58);interface _HTTPTracerdeclared inside 5c'shttp_tracer.rbs.RECOVfifteen (phase 4's),ProtocolError#retryable_by_status?baked at construction from 5a'sRetryabilityand deliberately not#retryable?(4b's; P6-10),CFG-35's throwable half (5a's), the per-attempt half ofOBS-29's wiring (5c's).Dexpace::RetryPredicateErrorflat inerror/(RETRY-40, P6-56).Cursorcontext-bundle widening (Task 8, P6-51):Cursor#bundle,bundle: Bundle::NONEonCursor.build,Pipeline#call,AsyncPipeline#calland both drivers'#advance, carried across#fork; 5b'sStep#open_span(request, bundle)and the span correlation now read the cursor's bundle — the first clause of the instrumentation step's precedence rule is live.Transport.conforms?still holds; the departure from 4c's P4-38 reasoning is recorded.sig/mirror:http_date.rb(the day group(\d{1,2}),RETRY-15— an absent weekday is still refused),error/protocol_error.rb,pipeline/cursor.rb,pipeline.rb,async_pipeline.rb,pipeline/sync_driver.rb,pipeline/async_driver.rb,instrumentation/step.rb,instrumentation/async_step.rb; comments inbundle.rb,tracing.rb,http_tracer.rb. Five earlier-phase test pins the code invalidated ride this branch:dexpace_test.rb's layer table, 4c'scursor_test.rbmethod-set pin, 5a'shttp_date_test.rbrejection element, 5b's cursor stand-ins instep_test.rb/async_step_test.rb.sig/mirrorslib/one file per file (the twoprivate_constants withhooks.rbs's comment); the manifest regenerated once from 956 to 1005 rows (+49), every row read against the object model and P6-1/P6-2.Decisions taken in the open, against the plan's text
Ledger rows P6-51–P6-61 and the checklist's "Deviations from the plan" (33 items) itemise them. The plan was written against phase 4/5 designs and one interpreter; the load-bearing corrections: no
Step#bundle_forexists (the seam is#open_span), phase 2'sFakeTransportwas not overwritten, 5c'sRecordingHTTPTracerwas reused, the async pump is a trampoline not a recursion,module_functionisextend self,Randomisuntypedin RBS (NFR-11).Layering
Each tip of the stack is green under every gate on its own tree. This branch is green on the SimpleCov floor too — 94.94% on 4.0.6 (94.87% on 3.2.11; 2,051 runs, 0 failures). The tests PR takes the same tree to 99.98%.
Verification
--ignore-parent-exclusion) 410 files clean.RETRY-14convergence test (both stacks exhaust after the same wire sends, and move together when the setting changes); the engine as the orchestrator's transport with the idempotency key stamped once and the same request object re-sent (assert_same); the fork/close/sleep ordering against the real driver; the no-scheduler async wait failing within wall-clock bounds; the seven 6a suites identical underMAX_RETRY_ATTEMPTSunset / 0 / −1 / abc (hermeticity, R0-1).Known follow-ups from the final review (nits; not blocking a gate)
gems/dexpace-core/test/dexpace_test.rb:176: the smoke-suite comment says "five public Resilience constants" beside an assertion listing six.Pump#retry_afteris pinned by no test (the reviewer's mutation survives equivalently; the sync side is pinned) — on the tests PR.RETRY-30paragraph are false as written → phase 10's inbound list (P6-54);Pipeline.standardmust threadsettings:andhttp_tracer_factory:and the async preset's YARD must state the no-scheduler behaviour → 6b's Task 13a; a raising#retryable?propagates unfenced fromPolicy.throwable_retryable?→ phase 9'sXCUT-6disposition.