fix(storage): probe before the pipe, not inside it (#462) - #904
Open
fujibee wants to merge 1 commit into
Open
Conversation
agmsg_sqlite memoises the escape probe so it costs one sqlite3 process per shell rather than one per call (#462). The right-hand side of a pipeline is a subshell: it inherits the memo, but a memo it sets there dies with it. A process whose first database access is piped therefore records nothing and every piped call after it probes again -- measured at two sqlite3 processes per call, and it does not converge. Ten piped sites, derived rather than listed. Three arrived with #895's move onto stdin, which was reviewed and cleared without the shell rule underneath it being noticed; the rest predate it. A redirection is not a pipe and needs nothing. The scan that found them is now a test, so the eleventh is caught rather than counted on being noticed.
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
Change class: performance, no behaviour change. Every statement runs as
before; some of them stop starting a second sqlite3 process to ask a question
that was already answered.
What is wrong
agmsg_sqlitememoises the escape-flag probe so it costs one sqlite3 processper shell rather than one per call — that is #462, and the reason is written
above the function.
The right-hand side of a pipeline is a subshell. It inherits a memo, but a
memo it sets there dies with it. So a process whose first database access is
piped records nothing, and every piped call after it probes again.
Measured, in a shell whose first access is piped:
Against the same call after an unpiped one has warmed it:
It does not converge: the piped form pays the probe forever.
Where
Ten sites, derived with
grep -rn '| agmsg_sqlite' scripts/rather thanenumerated. Three of them arrived with #895 — my change, which moved
data-sized SQL onto stdin and was reviewed and cleared without the shell rule
underneath it being noticed. The rest predate it, including the migration
helper and the legacy import path.
A redirection is not a pipe.
agmsg_sqlite db < fileruns in the currentshell and memoises normally, so the four
< "$sql_file"sites need nothing —including the one #899 just added.
The fix
agmsg_sqlite_warmbesideagmsg_sqlite, called on the line before apipeline. Two of the driver's statements go through a helper that warms, so the
rule has one place there rather than one per statement.
Measured
The new case is a scan, not a list: it derives the piped sites and fails on
any without a warm above it. Written that way because the sites move — three
appeared in a single day, in a change that two people read.
It also asserts the scan can see at least eight sites, so an instrument that
matched nothing could not pass as "everything is warm".