Make the report 0057 decides, and put every one of them through 0047's queue (#57) - #290
Merged
Merged
Conversation
…s queue 0057 fixes that every position report goes onto the queue in 0047 rather than to a server, that five events report the moment they happen, and that the interval produces one while something is playing. The interval and the events were values in src/playback/cadence.rs and nothing made a report: no position was ever enqueued, and #57's three open conditions had no subject. src/playback/report.rs is the report. It takes the queue on every call that reports and hands back what the queue did, so a report that was not asked of the queue is not a value it can produce; it carries the position always, which is 0056's rule about a report with no position reading as the item finished; and it makes no report between interval ticks, which is the alternative 0057 refuses first. Four guards were watched failing. Deleting the question of whether a report is due reddens four cases, which is reporting on every position change. Leaving the interval where it was after an interval report reddens one. Leaving it where it was after an event reddens five, the paused case among them. Making a seek wait for the interval reddens three, and the scrub case is one of them. No coalescing rule is written here: forty seeks leave one entry because 0047 coalesces at enqueue, and src/server/write_queue.rs is untouched. Which request carries a report is not decided here. 0010 classifies the paths for playback started and ended as accumulations, and 0047 does not queue an accumulation, so how a report occasioned by started or stopped is delivered is a question between those two records and 0057 that #27's transport meets. Closes #57. 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
Closes #57
The last section says which of its four conditions this reaches, which is all of them, and what it leaves outside.
What changed
src/playback/report.rsis the reportdocs/decisions/0057-the-progress-reporting-cadence.mddecides: the one act that puts a playback position on the queue in 0047, on each of the five events the moment it happens and on the interval while something is playing.src/playback/cadence.rsalready held the interval and the events as values; nothing made a report out of them, so no position was ever enqueued and #57's three open conditions had no subject in the tree.Three things the module holds, each because the convenient shape is wrong in a way nothing would report.
Every call that reports takes the queue and hands back what the queue did. 0057 says every report is put on the queue rather than sent, and 0047 says why: a path that sends directly and queues only when the server is away is two paths that agree until a device's connectivity changes mid-playback. So the type has no second way out. A report is
WhatTheEnqueueDid, and nothing in the module can produce one without asking the queue.A report always states the position. 0056 reads the server at one commit: a report carrying no position is read as the whole duration, which is the item finished.
PositionReporthas no absent value, and a report at the beginning carries the beginning.Observing a position makes a report only when the interval says one is due. A client hands the core every position its player produces, and most of those calls enqueue nothing. That is the whole of the cadence, and it is the shape the method loses the moment one question is deleted, which is the first guard below.
The other four files register the module where every landing registers one: the crate's thread assertions in
src/lib.rs, the module list and two paragraphs insrc/playback/mod.rs, the same assertions asked from outside the crate intests/thread_statements.rs, and the paragraph insrc/playback/cadence.rsthat said no report existed and #57's conditions were met by nothing.The means is the one the tree already carries, which is the check
## Choosing the meansasks for. This is a value beside two values that already exist, the interval incadenceand the queue inwrite_queue, in the language 0011 measured and chose, and every case asserts against the queue's own answers rather than against a second apparatus.What failure it prevents
Reporting on every position change. The player hands a client a position several times a second, and a reporter written straight from that produces a request per second per stream, multiplied by every stream in a household, at the one place on this board where the machine belongs to the person paying for it. 0057 prices that as its first refused alternative, and the module's
observeis where that shape arrives.A report that skips the queue. Sending directly when the server is there and queueing when it is not reads as the smaller change, and 0047 refuses it because the two paths disagree only on a phone on a train. The module cannot express it.
A seek that waits for the interval. It reads as an optimisation, since a scrub already coalesces to one entry, and it costs the report that carries where a person actually is when the process is killed while they scrub. 0057 fixes that a seek reports the moment it happens.
Evidence
The module's own cases, at the commit above:
The whole suite, which is one of the two commands the README names:
The other one, and the three legs that run without a network:
The size of the change:
What a guard here refuses, and the proof it bites
Four guards, each watched failing on the version of the mistake it is against, each restored afterwards, and the restored file re-run green. Every run below is
cargo test --locked --lib playback::report, and the deliberate violation is the edit named above each run.Nothing is reported between interval ticks. The violation is the due question deleted from
observe, which is the reporter that reports on every position change:A report on the interval moves the interval on. The violation is the interval left where it was after an interval report, so the next one is due at once:
An event moves the interval the way 0057 says it does. The violation is the interval left where it was after an event, so a started stream never runs and a paused one is never told it stopped:
A seek does not wait for the interval. The violation is the optimisation somebody writes because a scrub coalesces anyway: a seek that reports only where the interval is already due, answering the queue's replaced-in-place without asking it:
The fourth is the one worth reading: the queue answered nothing and the scrub case still saw one entry, holding the position playback started at rather than the one the person scrubbed to. A case that counted entries alone would have passed it.
What this does not cover
Which of #57's conditions this reaches: all four, and this section is what it leaves outside them. The cadence and its reason were recorded by 0057. A scrub produces one report, each immediate event reports without waiting for the interval, and every report is observed passing through the queue, each by a case named for it above.
Nothing is delivered. No report leaves the device. Nothing in this tree opens a connection, the drain 0047 describes runs on 0045's recovery report and does not exist, and the queue every report passes through is not durable, which
src/server/write_queue.rssays of itself and #47 holds. What is proven here is the reporting and never the delivery, so 0057's statement about the report a person most expects to have landed, durable rather than delivered, is today neither.Which request carries a report is not decided here, and two landed records read differently on it. 0057 says every one of the five events' reports goes on the queue. 0010's table classifies
POST /Sessions/PlayingandPOST /Sessions/Playing/Stoppedas accumulations, and 0047 says an action that cannot be expressed as an assertion is not queued but attempted when it is asked for. What this module builds is every report as an assertion of position, whatever occasioned it, which is the shape 0047 admits and the progress path carries. Whether a report occasioned by started or stopped is also an accumulating call made outside the queue, or the progress assertion alone, is a question between those records that #27's transport meets and nothing here answers. The occasion is carried on the report so that whoever answers it has what it needs.No client-facing call exists.
Reportingis a value a caller drives with the queue in hand; the calls a client makes are #115's creation and its lanes, and no session in this tree holds a queue for one to be driven against.The position is admitted elsewhere.
Reportingtakes anAdmittedPositionrather than a number, so 0056's two bounds are applied at whichever boundary hands it one, and nothing here applies them. 0056's clamp report through 0100, once per item, is still not made, for the reasonsrc/playback/mod.rsalready gives.The interval is chosen and not measured. Ten seconds is 0057's number and that record says so of itself. #65 is the harness a measured replacement would come from.
The coverage leg has no new subject.
src/playback/is not an area on the pinned surface, which that register's own paragraph states and this change does not alter.The document-paths, thread-detector and target legs were not run here. Nothing in this change adds or edits a tracked Markdown file, which is the first leg's whole subject; the second needs a nightly compiler this machine does not have installed; the third compiles for triples this machine has no standard libraries for. That is a claim about what each leg reads rather than a run of it, and the run on the runner is the verdict.
Who has read it
Nobody other than me. There was no second reader available for it, and the evidence above carries the change in place of one.