Repository navigation
Spend a budget per kind of load, on a backoff nobody else is drawing - #192
Merged
Merged
Conversation
Rung 1 of ADR-0011's ladder, and the first thing that acts on a classification. The name splits the way rule 11 says it must: the public `RetryPolicy` is the *numbers* — three `RetryBudget`s, one each for a manifest, a segment and a licence, a fourth half of `PlaybackDecision` defaulting to Media3's own so every policy written before it keeps compiling — while the object that implements Media3's `LoadErrorHandlingPolicy` over them is `superplayer-resilience`'s internal `RetryingLoadErrors`, which reads the half in force at every consultation rather than once at construction. `StaticProfilePolicy` carries a row per profile with a reason for every departure, and `AdaptivePolicy` carries the profile's row through unmoved. Three budgets rather than one because one exhausted budget must not spend another's: a manifest that keeps failing is a session that is over, a segment that keeps failing is one hole in a stream that is otherwise playing. The licence budget is declared and today unreachable — Media3 routes a licence load through the policy its DRM session manager holds and not the media source factory's, and ADR-0011 rule 6 leaves DRM to Phase 6 — so it is asserted at the object rather than through a player, and says so where it is declared. Jitter is correctness and not policy (rule 12), so it is in `Backoff` and in no table: equal jitter over the upper half of a window that doubles per retry, which spreads a herd's return times across a recovering edge while keeping a floor that grows, and which is cited where it is chosen. Whether a failure is retried at all is the class's, never the error code's — the classifier decides and the budget only bounds it. `FallbackLadder` is the escalation seam #180 and #181 fill: a hook per rung, rungs 2 and 3 present and passing everything through, and the one place the rung order is imposed — Media3 asks a chunk source for a fallback selection *before* it asks for a retry delay, which is the ladder upside down, so both answers come out of one climb. Core gains `DecisionInForce`, a window onto `playbackDecision` opened before the player exists, because an extension is handed the engine configuration before the policy has answered. The header-refresh slot is filled with a pass-through for #179 to replace, which also turns on the `LoadKind` stamp the classifier needs to tell a refused segment from a refused manifest. `PlaybackHarness.buildPlayer` and `buildPool` take a resilience, the latter through the substituted player factory rather than the pool builder for the reason its cache paragraph already gives. Every acceptance box is forced through it over real HLS, counting what left the chain; the spread of the backoff is asserted across many draws, where the property actually lives. Closes #178. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeTWwmK1KcFMrfSAre23GS
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.
Closes #178.
Rung 1 of ADR-0011's ladder: retry the same URL, with exponential backoff and jitter, per error class, against separate manifest, segment and licence budgets.
The argued decisions
The name splits in two (rule 11). The public
RetryPolicyis the numbers and lives insuperplayer-coreas a fifth member ofPlaybackDecision(@JvmOverloads, defaulting toRetryPolicy.MEDIA3_DEFAULT, so every policy written before it keeps compiling). The object implementing Media3'sLoadErrorHandlingPolicyissuperplayer-resilience's internalRetryingLoadErrors. No@UnstableApitype reaches any public signature —verifyNoUnstableMedia3InPublicApipasses and the core API diff isRetryPolicy,RetryBudgetandPlaybackDecision.retryonly.RetryBudget.MEDIA3_DEFAULTis Media3's own count and ceiling exactly (3 retries, 5s cap, cited at the constant). The curve is deliberately not Media3's: Media3 grows linearly and never jitters, and rule 12 makes jitter correctness. So "Media3's default" means Media3's budget spent on a safer schedule, and the KDoc says so rather than leaving a reader to discover it.Reading the half in force at every consultation. An extension is handed the
EngineConfigurationbefore the policy has been consulted, so the object cannot be given a decision at construction. Core gains one internal seam,DecisionInForce— a window ontoSuperPlayer.playbackDecision, fed bybuild()from the reapplication's current decision or the one fixed at construction. It is not a widening: what it hands back is a public value of the player the reader is part of.RetryingLoadErrorsTest.theBudgetInForceIsReadAtEveryConsultationRatherThanAtConstructionpins it.Jitter: equal, not full.
Backoffdraws uniformly over[base/2, base],basedoubling per retry to the budget's ceiling (// ref:the AWS "Exponential Backoff And Jitter" article, which names and measures both forms). Full jitter spreads hardest and is right for a fleet of writers, but it can draw a near-zero delay — asking an edge that failed a millisecond ago, and spending one of three to five attempts on it. Equal jitter keeps a floor that grows with the attempt while still spreading arrivals across a window as wide as the base. The reason is written where the constant is chosen.The licence budget: declared, and today unreachable.
DefaultMediaSourceFactory.setLoadErrorHandlingPolicydoes not reach the DRM session manager — Media3 keeps a separateLoadErrorHandlingPolicyonDefaultDrmSessionManagerProvider(setDrmLoadErrorHandlingPolicy), and there is no setter from the media source factory to it. SuperPlayer plumbs no DRM at all (ADR-0011 rule 6 leaves it to Phase 6). So the honest answer the issue anticipated: the budget is declared publicly, because the shape of a policy should not change when the plumbing arrives;RetryingLoadErrorsalready answers for aC.DATA_TYPE_DRMload, so the number is in force the day one is routed there; and it is asserted at the object rather than through a player, withRetryPolicy.licence's KDoc andStaticProfilePolicy's saying exactly why no profile sets it.Addressing budgets by Media3's
dataType, not by core'sLoadKindstamp.getMinimumLoadableRetryCount(dataType)is asked with nothing in hand but the type, so a budget that answered one of Media3's two questions from the stamp and the other from the type could disagree with itself. OnewhenoverC.DATA_TYPE_*inRetryBudgetKind.ofDataType— not over an error code, so ADR-0011 rule 1 stands andErrorClassifierremains the only suchwhen.The ladder's trap, and the escalation seam. Media3 asks a chunk source's policy for a
FallbackSelectionfirst and a retry delay second — ADR-0011 rule 7's ladder upside down. A rung above 1 that answered the first question while rung 1 still had budget would skip rung 1 silently.FallbackLadder.climbis the single place that order is re-imposed: both ofRetryingLoadErrors' answers come out of one climb, bounded by the class's ownrungCeiling. Rungs 2 and 3 are present as escalating hooks naming #180 and #181; rungs 4–6 are deliberately not in this object, because they are player-state operations that cannot be performed from a loading thread (rule 7 assigns them to rules 5 and 10), soRungOutcome.Escalateout of the top is where they begin. A later unit adds one entry to one map and edits neitherResiliencenorRetryingLoadErrors.The header-refresh slot is filled with a pass-through. #179 replaces its body. Filled rather than left null on purpose:
TransferChainstamps every request with itsLoadKindonly when a slot is occupied, and that stamp is what letsErrorClassifiertell a refused segment from a refused manifest — which is the whole of theTransient.CdnEdge/Content.SegmentGapdistinction rung 1 acts on. It returns the upstream factory unchanged, so it allocates no data source and drops no transfer listener.Shared infrastructure.
PlaybackHarness.buildPlayer(..., resilience =)andbuildPool(..., resilience =); the pool threads it throughbuildPlayerOverat thesetPlayerFactoryclosure, not throughPlayerPool.Builder.setResilience, and the KDoc says so for the same reason itscacheparagraph does (#188). Testkit API surface regenerated.Tests
superplayer-resilience, all through./gradlew check:RetryPlaybackTest(7) — every acceptance box forced throughPlaybackHarnessover real HLS, counting requests at the transport: a fault that relents is retried and playback goes on; one that never relents spends its budget and stops (exactlymaxRetries + 1asks, then the failure); a 416 on a segment is classified unretryable and asked for exactly once despite a budget of five; a spent manifest budget leaves the segment budget whole (segment spends more asks than the manifest budget would have allowed); position never goes backwards across a retry, sampled across the span rather than at its ends; a player with no resilience keeps Media3's own retrying while one with a budget of nothing asks once; and the profile's row is what a player with no policy asks for.RetryingLoadErrorsTest(5) — the two things a player cannot show: the licence budget, and the budget being re-read at every consultation. Plus budget separation, the class's veto, and no fallback offered while rung 1 has budget.RetryBackoffTest(5) — growth, the overflow guard, and jitter as a spread over 200 draws per retry rather than as one delay, with the equal-jitter bounds asserted at both ends.AdaptiveBufferPolicynow carriesbase.retrythrough;SuperPlayerProfileTestspells out each profile's row. Golden traces are unchanged, which is ADR-0011 rule 14's other half.Review
Standards. ADR-0001 rule 2 (no
@UnstableApipublic) — mechanical, green. ADR-0005/0009 (policy behind the boundary, no Media3 type in it) —RetryPolicy/RetryBudgetname none. ADR-0011 rules 7, 11, 12, 13, 14 — each cited at the code that implements it; nothing about jitter, rung order or position appears inPlaybackDecision, a profile table or a builder, andResilience.standard()takes no arguments precisely so none can.docs/api-surface.md— all three dumps regenerated in this change.docs/testing.md—RetryPlaybackTestdrives the public API only; the two unit tests construct this module's own objects (theErrorClassifierTestprecedent) and reach core'sDecisionInForcethrough the friend path this module already uses forLoadKind, rather than past any player's facade.CONTRIBUTING.md— the one non-obvious algorithm carries its// ref:. No new dependency (media3-exoplayeris already resolved repo-wide; named in the module for the type it uses), so noTHIRD_PARTY.mdrow.CLAUDE.md's resilience paragraph extended.Smells: found and fixed one —
FailedLoad.kindwas carried but read by no rung, pure Speculative Generality, so the kind is resolved to a budget before a rung sees it.RungOutcome.FallBackToandFailedLoad.infoare unused today and kept deliberately: they are the seam #180/#181 fill, and the KDoc says which. No Repeated Switches (onewhenoverdataType, one over the budget kind, both in one file).Resilience/StandardResilienceis not a Middle Man — it holds nothing, which is howPlaybackResilience's thread-safety obligation is discharged.Spec. All eight acceptance boxes covered, each by a named test. Nothing implemented beyond the issue except the two shared-infrastructure deliverables the batch asked for. The one partial is the licence budget, and it is partial by argument rather than omission — the paragraph above, and the KDoc at the property.
Skipped
Emulator and demo runs, per the batch instruction (host memory). Nothing here changes the demo, the manifest or a published coordinate; what a device would add over the Robolectric harness is real-CDN behaviour, and the retry path is exercised against a real HLS parser and a real chain here.
CI
Dispatched (this repo's
ci.ymlisworkflow_dispatch:only, so no checks run on the PR itself):https://github.com/ramesh130/superplayer/actions/runs/35052169868 — success.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HeTWwmK1KcFMrfSAre23GS