sync: full_scans is not two per pass, and the docs said it was - #71
Merged
Conversation
`sync_passes` shipped three hours ago with a doc comment that names a
correction factor. It says `full_scans` counts TWO per `sync_once`, and
that reading it as a pass rate "overstates the rate by 2x". A reader who
applies that 2x to the live fleet gets a wrong number.
MEASURED ON SILBER, ONE 300 s WINDOW, TWO ENTRIES IN THE SAME WINDOW:
3.71 scans per pass on `st2-bus-default` and 2.33 on
`st2-declarations-default`. Not 2.00, and not the same on both.
WHY. `scan_entry` has three callers, not one. `sync_once` calls it twice,
once each side of the peer step. `complete_inbound` calls it exactly once
per guarded inbound transaction. `prepare_inbound_entry` calls it at most
once more, and skips it when the durable scan is still good. So the
relation is a bound, not a constant:
2*passes + guarded <= full_scans <= 2*passes + 2*guarded
Both live samples sit inside it. Inbound traffic is not a fixed multiple
of local passes, so no constant can convert one counter into the other,
and the error is invisible from this side.
WHY IT MATTERS. Dividing a live `full_scans` by 2 overstates the rate on
the busy entry by 85%. That is the same class of miscorrection that
produced the 58% guard figure, which was reported against the fleet and
then withdrawn. This counter was added to stop that error and it
documented it instead.
THE TEST, AND I WATCHED IT FAIL. `full_scans_counts_inbound_transactions_
as_well_as_passes` drives three passes and two guarded inbound
transactions, then pins the bound. I mutated the code to the false belief
(no increment in `scan_entry`, `fetch_add(2)` in `sync_once`) and it
failed with "scans=6, passes=3".
THE EXISTING TEST STILL PASSED UNDER THAT MUTATION. It runs on
`LoopbackTransport` with no inbound traffic, so it pins the isolated path
and cannot see this. That test is not wrong and it is not changed. Its
doc now says which condition it holds under.
NO BEHAVIOUR CHANGES. This is a comment, a test, and a bound.
Agent: Silber.fabric
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.
sync_passesshipped three hours ago (#68) with a doc comment naming a correction factor:full_scanscounts "TWO per call", and reading it as a pass rate "overstates the rate by 2x". Applying that 2x to the live fleet gives a wrong number.Measured
Silber, one 300 s window, both entries sampled in the same window:
st2-bus-defaultst2-declarations-defaultNot 2.00, and not the same on both.
Why
scan_entryhas three callers, not one:sync_oncecalls it twice, once each side of the peer step.complete_inboundcalls it exactly once per guarded inbound transaction.prepare_inbound_entrycalls it at most once more, and skips it when the durable scan is still good.So the relation is a bound, not a constant:
Both live samples sit inside it. Inbound traffic is not a fixed multiple of local passes, so no constant converts one counter into the other, and the error is invisible from this side.
Why it matters
Dividing a live
full_scansby 2 overstates the rate on the busy entry by 85%. That is the same class of miscorrection that produced the 58% guard figure, which was reported against the fleet and then withdrawn. This counter was added to stop that error; it documented it instead.The test, and I watched it fail
full_scans_counts_inbound_transactions_as_well_as_passesdrives three passes and two guarded inbound transactions, then pins the bound. I mutated the code to the false belief — no increment inscan_entry,fetch_add(2)insync_once— and it failed:The existing test still passed under that mutation. It runs on
LoopbackTransportwith no inbound traffic, so it pins the isolated path and cannot see this. That test is not wrong and is not changed; its doc now states the condition it holds under.Scope
No behaviour changes. A comment, a test, and a bound.
Agent: Silber.fabric