fix(remote): report pull bootstrap progress on stderr (#882) - #883
Closed
fujibee wants to merge 2 commits into
Closed
fix(remote): report pull bootstrap progress on stderr (#882)#883fujibee wants to merge 2 commits into
fujibee wants to merge 2 commits into
Conversation
A pull of 2079 messages runs for 312 seconds and prints nothing. A verifier running it against a real server on Windows saw the silence, concluded the command had hung, and killed it -- a working command was indistinguishable from a stalled one, and the diagnosis went thirty minutes in the wrong direction (#882). Progress goes to stderr only. cmd_pull captures this process's stdout as the result channel and greps it for pull_bootstrap_result, so a line written there would ride in the stream the caller parses. The lines separate fetching from applying because applying spawns a child process per batch and fetching does not: when the output stops moving, which line it stopped on says which half to look at.
A terminal does not timestamp its own scrollback, so "it printed this and then stopped" cannot say whether the stop was ten seconds or ten minutes. The Linux run that finished took 312 seconds; without elapsed, a slow phase and a stuck one look identical to whoever is watching.
Owner
Author
|
Folded into #895, which carries these two commits unchanged along with the fix for the failure they were written to make visible. Keeping them apart would have landed the size fix first and the progress output later — which is the order the reporter's own experience argues against. What cost them 79 minutes was not being able to see anything, and a fix that lands without that leaves the next failure just as silent. |
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.
Declared reviewers: 1
Closes part of #882.
Change class: user-visible output.
agmsg remote pullgains lines onstderr. It adds no stdout, so nothing that parses the result changes.
What was wrong
A pull of 2079 messages runs for 312 seconds and prints nothing at all. An
external verifier ran it against a real server on Windows, saw the silence,
concluded the command had hung, and killed it. A working command and a stalled
one were indistinguishable, and roughly half an hour of diagnosis went into a
command that may well have been fine.
What it prints now
stderr only.
cmd_pullcaptures this process's stdout as the resultchannel —
result="$(... pull-bootstrap ...)", thengrep '"pull_bootstrap_result"'— so a line written there would ride in the stream the caller parses.
scripts/remote.sh:936already records this convention for the neighbouringcall;
scripts/remote-sync.sh:47execs node with no redirection, so stderrreaches the operator's terminal unchanged.
Fetching and applying are named separately on purpose: applying spawns a child
process per batch and fetching does not, so when the output stops moving, the
line it stopped on says which half to look at. On Windows — where the report
came from — that is the whole open question.
Elapsed seconds are stamped because a terminal does not timestamp its own
scrollback: without them, "it printed this and then stopped" cannot say whether
the stop was ten seconds or ten minutes.
Measured
End-to-end on the shipped path (not just in-process), streams captured to
separate files:
tests/remote_sync_engine.test.mjs— 95 pass, 0 fail. The new case capturesboth streams, because the regression worth catching is a progress line
landing in stdout, and that is invisible if only stderr is measured. It asserts
stdout is exactly one line and parses as
pull_bootstrap_result.Mutation table, re-run after the assertions were rewritten for the elapsed
prefix:
pullProgresswrites to stdout instead of stderrapplying N messagesline deletedWhat this does not do
It does not fix the Windows stall. It makes the stall reportable: the next run
says which phase it stopped in and after how long. Granularity is one page
(
limit=1000), so a stall inside a single apply is still a silent interval —narrowing further would mean changing the driver's protocol, which this does
not touch.