RecordFirstLifecycleRequest took the instance lock and read the latest
generation, then called RecordLifecycleRequest, which took the lock and
read the latest generation again. It now checks its address, locks and
reads once, refuses a later generation of an instance that already
holds a request, reads the content, and passes the latest generation to
decide, the per-instance decision RecordLifecycleRequest also calls
(recordNew above the latest, answerAddress otherwise). decide takes a
*Queries and opens nothing. The person is still parsed after the
first-request check, so a malformed person on a used instance under a
later generation still answers a conflict.
A new create, an identical repeat and a create above the ceiling now run
two statements fewer: the second LockLifecycleInstance and
LatestLifecycleGeneration. The three rows of the lifecycle answers
golden ("first: a new request", "first: an identical repeat", "first:
above the ceiling on an empty instance") lose that pair; they were
edited by hand before the run and the test passed without -update, so
every other row is byte-equal. Answers, rows, errors and writes do not
change unless an instance's latest generation moves between the
parent's two reads. Inserts happen only under the instance lock, and
status and acknowledgement updates leave the generation alone, so the
one writer that can move it is DeleteUnstartedLifecycleRequest, which
takes no instance lock. If it commits between the parent's two reads, a
double submit's repeat is recorded as a new create there and answered a
conflict here, from the one read. That ordering needs a commit between
two statements of one transaction and is not probed.
docs/database-locks.md, "Lifecycle intake", said the second lock was
harmless; it now describes the single lock and read, and says the delete
of an unstarted request runs outside the instance lock. The comment
"taking it again ... is harmless" is deleted with the second lock.
How it was shown to change no output: the packages
internal/integration and internal/integrations/fedwiki/... gave the same
302 test and subtest names with the same results at the parent and here
(301 pass, 1 skip, the replay recorder, skipped on both sides); the
Temporal extraction (activities, workflows, workflow bodies, payload
types) is byte-equal; FedWiki's recorded histories replay; the sites
walkthrough, which creates a site with a double submit, passes on both
sides; the project lint prints the same on both sides; the multiset of
lifecycle.go's string literals is equal (20 formats, no literal added or
removed); the lock-key query, the single address read in heldAt, the
unchanged sameLifecycleContent and the two spec and answer types are
unchanged. 9 break records, 8 per side (one break's code moved into
decide, so it has an old and a new record), each caught by the old test
at the parent and the new test here. Cognitive
complexity (pinned golangci-lint): RecordLifecycleRequest 8 to 6,
RecordFirstLifecycleRequest 7 to 9, decide 1.
Unprobed: the race with an unstarted request's delete, for the reason
above; every database error after the first statement, because no test
makes one fail and nothing else; the *sql.Conn path, which inOwnTx
treats as a transaction as before and no caller uses.
cgalo5758
9f103c13ef