Hold 0047's two moments per entry, and answer the age they anchor (#47) - #297
Merged
Merged
Conversation
0047 says an entry stores two moments when it is enqueued - the server's own last stated time and the device's wall reading at that instant - and that its age is computed the way 0043 computes a cache entry's, on 0102's anchor, with the same correction and the same two guards. The queue that landed carried neither, and its own module documentation said so. What that costs is the failure 0047 names for a restored queue: one that treats every entry as freshly enqueued keeps its order and loses every age, so a client can say only that something is pending rather than that an action has been waiting three weeks. An entry now carries `WrittenAt` and answers `Entry::age_at`. Both are `crate::cache::freshness`'s own types rather than a second arithmetic here, which is what naming 0043 asks for and what stops the two drifting. Every moment is handed in, so nothing in the module reads a clock. A replacement keeps the earlier entry's two moments, as it keeps its position in the order, which 0047 does not say. Taking the later action's moments reports every actively touched entry as freshly enqueued, so a queue undelivered for a month says seconds for the items somebody kept scrubbing - the restored-queue failure arriving through the coalescing door, landing on the person coalescing at enqueue exists to protect. The age acts on nothing, which is 0047's rule. That absence used to be held by there being no age at all; it is held by a case now, because a guarantee that rests on a thing not existing ends the moment it does. `Reporting::report`, `Reporting::observe`, `Reporting::report_after_a_renewal` and `mid_playback::the_renewal_ended` take the pair and pass it through, for the reason they already take an `ElapsedInstant`: nothing under `src/` reads a platform clock. Neither of #47's two conditions is met by this. Both restart the core and restore a server, and nothing in this tree does either. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com>
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.
The issue this belongs to
Belongs to #47. Neither of that issue's two conditions is met here, and the
reason is unchanged: both restart the core and restore a server, and nothing in
this tree does either.
It touches two files a second issue owns.
src/playback/report.rsis #57's andsrc/session/mid_playback.rsis #35's, and neither could be separated from this:0047 stores the two moments AT ENQUEUE, so the value has to arrive from whoever
calls the enqueue, and those are the only callers in the tree.
What changed
A queue entry carries the two moments
docs/decisions/0047-the-write-queue.mdsays it stores when it is enqueued - the server's own last stated time and the
device's wall reading at that instant - and answers how long it has been waiting
from them.
The pair, the correction and the two guards are
crate::cache::freshness's owntypes rather than a second arithmetic in this module. 0047 says the age is
computed the way 0043 computes a cache entry's, on 0102's anchor, with the same
correction and the same two guards, and one type is how that stays true rather
than being kept true by whoever edits next.
Every moment is handed in. Nothing in the module reads a clock, which is what the
module already said of itself and what the
no-platform-clockrule in.github/invariants/rulesrefuses anywhere undersrc/.Two things 0047 leaves open are decided here rather than at a call site later.
A replacement keeps the earlier entry's two moments, as it keeps its position
in the order. 0047 says only the second. Taking the later action's moments is
the shape that reads as obvious, and it reports every actively touched entry as
freshly enqueued, so a queue undelivered for a month says seconds for the items
somebody kept scrubbing.
The age acts on nothing, and that is now held by a case. It was held by there
being no age at all, which is a guarantee that ends the moment one arrives.
Reporting::report,Reporting::observe,Reporting::report_after_a_renewalandmid_playback::the_renewal_endedtake the pair and pass it through, for thereason they already take an
ElapsedInstant.What failure it prevents
The one 0047 names for a restored queue, arriving from two directions.
A queue restored with no anchor keeps its order and loses every age, so a client
can say only that something is pending. What the record wants it to be able to
say is that an action has been waiting three weeks, which is the difference
between an operator seeing a device that is behind and an operator seeing
nothing.
The second direction is the one this change had to decide rather than inherit. A
replacement that took the later action's moments loses the age of everything
somebody is actively touching, which is the same loss reaching the person who
used the application most - the person 0047 gives as the reason for coalescing at
enqueue rather than at drain.
Neither has happened, because nothing in this tree restores a queue. Both are
stated as what the record argues against rather than as an incident.
Evidence
The commit being pushed:
The two commands
CONTRIBUTING.mdnames, run at it:Seven cases are new in the module:
Four gate legs run by hand on this machine, each with its own verdict:
What a guard here refuses, and the proof it bites
Three, each watched failing on its own violation and each failing alone. Every
run below is
cargo test --locked --lib server::write_queue, and the green runat the head is:
a_replacement_keeps_the_earlier_entrys_momentsrefuses a coalescing thattakes the later action's moments. The violation is the one line somebody adds
while thinking the entry should carry what it now says:
a_device_clock_that_jumped_forward_is_corrected_out_of_the_agerefuses anage computed without the current skew, which is 0043's correction being dropped
on the way through this module:
an_entry_is_never_expired_by_agerefuses an expiry, which is the thing theage's arrival makes writable for the first time. The violation is housekeeping at
enqueue, dropping what has waited past the bound beyond which an age is not
believed:
What this does not cover
Nothing is durable, which is what an anchor is for. 0047 puts the bytes in the
store 0040 defines, keyed under #41, and no restore exists to compute an age
across. What landed is the value a restore would restore.
Nothing reads an age.
Entry::age_atis answered where somebody asks, and nocaller asks: there is no diagnostics event, no capability call and no client
surface for it, because nothing in this tree holds a queue at a point where one
could be made.
Nothing checks that the two moments were read at one instant, which
WrittenAt::atalready says of itself. 0047 makes the server half the LASTstated time rather than a reading taken at the enqueue, so the gap between the
two is wider here than in the cache, and an entry queued long after the server
last spoke carries that gap in its skew and reports itself older than it is. That
is the record's own shape rather than a defect in this change, and it is written
down here because no reading of the tree says it.
Nothing measured anything. There is no number in this change that came from a run
of the core.
The coverage leg was not run on this machine.
.github/coverage/coverage.shisthe run that judges the bar,
src/server/write_queue.rsis already a module onthe pinned surface, and the verdict is the runner's on this pull request rather
than anything asserted here.
The thread detector was not run on this machine either. It names a toolchain of
its own, and the leg on this pull request is where its verdict comes from.
Who has read it
Nobody other than me. There was no second reader available, and the evidence
above stands in place of one.