Back off from a short first wait instead of a flat 200 ms - #99
Merged
Conversation
`wait_for_completion` slept a fixed 200 ms before asking a second time, so a job the server had already finished still cost the caller that much of this function sleeping. Measured with the real binary against a real server, a five byte file: before: 236, 233, 237 ms after: 44, 44, 42 ms The wait now starts at 10 ms and doubles to the old 200 ms ceiling, so the schedule is 10, 20, 40, 80, 160, 200, 200, ... A long job is unaffected by design: it reaches the ceiling after 310 ms and stays there, which the same measurement confirms on a 60 MB incompressible input taking around 8.3 s (8347, 8550, 8562 before against 8271, 8479, 8297 after). The whole price is about three extra requests over the life of a job of any real length. The schedule is a pure function in `protocol` rather than a counter inside the loop, matching the split the rest of that module already has, so it can be checked without a server and without waiting for it. Tests: four in tests/protocol.rs pinning the first wait, the doubling, the ceiling, monotonicity and saturation on a nonsense input; one in tests/client.rs driving a stub that reports `compressing` three times, which takes 627 ms if the fixed interval comes back and around 70 ms as it stands. Closes #48
…ested
The backoff was only reachable through a socket and a real `thread::sleep`, so
the one test covering it drove a stub server and asserted on a wall clock with
a 400 ms margin. That proves the fixed interval has not come back and very
little else: it cannot say what the schedule *is*, and it gets slower and
flakier the more of it you try to cover.
`waiting` now holds the loop behind two one-method traits, `Sleeper` and
`Poller`, with `RealSleeper` and a private `HttpPoller` as the production
pair. Between them a test can state the thing no stub server can state
deterministically: a job that takes exactly N milliseconds. The fake sleeper
accumulates virtual time instead of spending it and the fake job reads that
accumulation to decide whether it has finished.
`wait_for` returns `Waited { polls, slept }`, its own account of what it did.
That is the part that keeps the tests honest: asserting on the return value is
asserting on the subject, while reaching into a fake to count calls is
asserting on the fake, and passes when the fake is wrong.
The schedule moves out of `protocol` with it. That module answers "what does
this JSON mean"; how often to ask is a different question and now has its own
file.
Ten cases in tests/waiting.rs, running in microseconds. All five mutations I
tried are caught by several of them each: starting at the ceiling, never
growing, sleeping before asking, not counting the settling poll, and removing
the ceiling.
Two of them exist to be honest rather than flattering. The backoff is not
uniformly faster: a job finishing just after the ramp is asked again a whole
ceiling later, so at 199 ms the old schedule finished at 200 and this one
finishes at 310. The bands are narrow, the worst case measured across 0 to
2000 ms is 110 ms, and it is bounded by one ceiling. Both the bound and the
gain are pinned.
The stub-server test stays. It is the only thing proving `RealSleeper` and
`HttpPoller` are actually wired in rather than the policy being right in
isolation.
This was referenced Aug 26, 2026
Merged
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.
Fixes #48.
wait_for_completionslept a fixed 200 ms before asking a second time, so a job the server had already finished still cost the caller that much of the client sleeping. The wait now starts at 10 ms and doubles to the old 200 ms ceiling: 10, 20, 40, 80, 160, 200, 200, ...Measured
Real CLI against a real
collapse-server-backend, four runs each, first discarded as warm-up.The small file reproduces the ~235 ms the issue reported. The long job is unchanged: it reaches the ceiling after 310 ms and stays there.
It is not uniformly faster, and that is pinned
A job that finishes just after the ramp is asked again a whole ceiling later, where the flat schedule might have caught it sooner. At 199 ms the old schedule finished at 200 and this one finishes at 310.
The bands are narrow, the worst case across 0 to 2000 ms is 110 ms, and it is bounded by one
MAX_POLL_DELAY.the_backoff_is_never_worse_by_more_than_one_ceilingasserts the bound, and it also asserts the worst case is not zero, so it fails if it ever stops measuring anything.everything_that_finishes_inside_the_ramp_is_faster_than_beforeholds the other side for every job under 150 ms.Worth stating plainly since the issue only asked about the fast case.
The seam
The first version of this was only reachable through a socket and a real
thread::sleep, so the single test drove a stub server and asserted on a wall clock with a 400 ms margin. That proves the fixed interval has not come back and little else.waitingnow holds the loop behind two one-method traits:RealSleeperand a privateHttpPollerare the production pair. Between them a test states the thing no stub server can state deterministically: a job that takes exactly N milliseconds. The fake sleeper accumulates virtual time instead of spending it; the fake job reads that accumulation to decide whether it is done.wait_forreturnsWaited { polls, slept }, its own account of what it did. That is what keeps the tests honest: asserting on the return value is asserting on the subject, while reaching into a fake to count calls is asserting on the fake and passes when the fake is wrong.The schedule moved out of
protocolwith it. That module answers "what does this JSON mean"; how often to ask is a different question.Tests
Ten in
tests/waiting.rs, running in microseconds because none of them sleeps. Each of these mutations was applied and is caught by several cases:The stub-server test in
tests/client.rsstays. It is the only thing provingRealSleeperandHttpPollerare actually wired in, rather than the policy being right in isolation.Not in scope
#71 is still open and lives in the same loop. There is no deadline, no maximum number of polls and no per-request timeout, so a server that answers
compressingforever still hangs the caller forever. A backoff changes how often it asks, not whether it stops. The seam added here makes it a few lines, and aSleeperthat reports elapsed virtual time makes it testable the same way.