Repository navigation
Conversation
…atch Split out of volcengine#5967 so that PR can stay scoped to the dispatch behaviour change. Reviewed against the feedback that suggested a breaker and a provider abstraction as prerequisites: those are worth having, but they are not what the dispatch change needs, and folding them in made that PR hard to review. Remote resilience: - A provider that fails 3 times in a row is circuit-broken; the rest of the batch skips it instead of paying for a doomed call per document. - After a 20s cooldown exactly one document is let through as a probe, so a recovered service is picked up again and a service that is still down costs a single call rather than the batch. - A failing probe re-arms the cooldown. It does not reset into a per-document retry, which is what the previous attempt did: with a threshold of 1 and a 3-document cooldown, sustained failure produced [1,0,0,1,1,1]. - A success clears all of that provider's breaker state. Observability: - pdf_remote_degraded counts documents that fell back, whether the remote failed outright or the breaker kept them off it. Provider dispatch is expressed as an ordered list of (name, converter) pairs, so another remote parser is a provider entry rather than a new strategy value. Tests: 6 breaker tests including the probe-failure regression, and an explicit assertion on the call sequence. 77 pass across the dispatch and bookmark suites.
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.
Summary
Remote resilience and provider generalization for the PDF parser, split out of volcengine#5967
so that PR can stay scoped to the dispatch behaviour change.
Stacked on volcengine#5967 (upstream:
volcengine/OpenViking#5967). The base isfeat/mineru-first-strategy-standalonerather thanmain: the breaker only earns itskeep under remote-first dispatch, which is what volcengine#5967 introduces. On
mainthe remoteparser is a local-first fallback, so a remote outage never produces the per-document
cost this is meant to remove. Review and merge after volcengine#5967.
This PR lives in the fork because its base branch lives here; the diff is exactly one
commit.
Why this is separate
Review feedback on volcengine#5967 suggested a circuit breaker and a provider abstraction. Both
are worth having, but neither is a prerequisite for the dispatch change, and folding
them in made that PR hard to review -- the reviewer judged the result to be "changed
too much". This PR carries them so volcengine#5967 can be judged on its own.
Remote circuit breaker
skips it instead of paying for a doomed call per document.
recovered service is picked up again and a service that is still down costs a single
call rather than the whole batch.
The probe-failure regression
The first attempt at this reset into a per-document retry. With a threshold of 1 and a
cooldown of 3 documents, sustained failure produced the remote-call sequence
[1, 0, 0, 1, 1, 1]-- a call per document again, which is exactly what the breakerexists to prevent.
_remote_record_failurenow separates the two cases. A failure while the circuit isclosed counts toward the threshold. A failure from a half-open probe re-arms the
cooldown from that moment, so the following documents are skipped again.
test_a_failing_probe_rearms_the_cooldown_instead_of_retrying_every_documentassertsthis directly, including that no remote call happens while the circuit is armed.
Observability
pdf_remote_degradedcounts the documents that fell back, whether the remote failedoutright or the breaker kept them off it.
Provider dispatch
Providers are an ordered list of
(name, converter)pairs from_remote_converters(),so registering another remote parser is a provider entry rather than a new
strategyvalue.
Verification
on the remote call sequence
test_pdf_strategy_dispatch.pyandtest_pdf_bookmark_extraction.pyruff checkcleanRelated