Catch up a boundary change a server was not listening for - #95
Merged
Conversation
Two improvements from Nathan Tarbert's #94, which arrived five minutes after #90 merged and was better than it in these two places. A NOTIFY reaches whoever is listening at the time. A replica that was restarting, or whose connection had dropped, is not, so it missed the announcement and went on enforcing the rules it read at boot until something restarted it. That is the original bug wearing a smaller hat and worse for being intermittent: the fleet disagrees with itself and nothing says so. The handler is now passed as `onlisten` as well, so the row is re-read whenever the driver establishes or re-establishes the subscription, which is exactly when a notification could have been missed. The announcement now goes inside the write transaction, so it is delivered on commit. A write that rolls back announces nothing, and there is no window where the row has changed and the other servers have not been told. Same shape as channel activity, which is what should have been copied. Two tests. The reconnect one fails without `onlisten`. The changelog also picks up the last day of merges, which had run ahead of it.
davidmckayv
requested review from
MikeRyanDev,
guidovizoso and
tylerslaton
as code owners
August 21, 2026 18:21
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.
Two improvements from @NathanTarbert's #94, plus the changelog catching up with the last day.
#94 arrived five minutes after #90 merged, so we were working the same item in parallel without
knowing. His was better than mine in two places and both are worth having.
The reconnect re-read
A
NOTIFYreaches whoever is listening at the time. A replica that was restarting, or whoseconnection had dropped, is not, so it missed the announcement and went on enforcing the rules it read
at boot until something restarted it. That is the bug #88 item 1 describes wearing a smaller hat, and
worse for being intermittent: the fleet disagrees with itself and nothing says so.
The handler is now passed as
onlistenas well asonnotify, so the row is re-read whenever thedriver establishes or re-establishes the subscription — exactly the moments a notification could have
been missed. I did not think of this.
The announcement inside the write transaction
Mine announced after the write returned. A write that rolled back could still have announced, and
there was a window where the row had committed and the other servers had not been told. It is in the
transaction now, so delivery happens on commit.
channels/routes.tsalready did it this way and Ishould have copied it more closely.
Proof
Two tests. The reconnect one changes the rule while the server has no subscription, then establishes
one, and fails without
onlisten. The rollback one asserts a listening server does not move when awrite fails.
bun run test:ci: 1008 tests, 0 fail. Typecheck and format clean.Changelog
The last day of merges had run ahead of it: #28, #37, #45, #46, #90, #30 and #92 were all in
mainand none were written down. Added, along with the two upgrade notes that came with them — the Bot id
rules, and
AUDIT_RETENTION_DAYS.The README is untouched. It is a build doc.
Where it runs
catches up when its subscription returns, rather than at its next restart.