Repository navigation
Release the writer lease when Chopin is stopped while it starts - #733
Merged
Merged
Conversation
A Chopin stopped soon after it started could exit without releasing the `chopin:writer` lease. The next process on the same database then refused to start with "another Chopin instance owns the database" until the lease expired, up to 30 seconds later. Two gaps caused this: - The SIGINT and SIGTERM handlers were registered only at the very end of startup: after the server was already listening, and after research recovery and the job runner started. A stop in that window took Node's default action, so the process exited without running the drain that releases the lease. Supervisors that start Chopin, wait for it to answer and then stop it hit this often, especially on slower machines. - Startup's session reset replaces the held lease with a re-fenced one. A drain that ran while the reset was in flight released the old fencing value, which no longer matched the row. The handlers are now registered as soon as the lease is held. The drain waits for the session reset before it releases, and skips stopping the HTTP server when it isn't listening yet. Reproduced by starting Chopin, sending SIGTERM as soon as it answered HTTP, and starting it again at once: the restart was refused 3 times in 5 before this change and 0 in 10 after it. Assistant-model: Claude Opus 5.5 (fast) Assistant-workflow: inline Assistant-verification: red-before-fix passed: the new startup.test.ts case, which stops the server as soon as it holds the lease, fails without the change (lease still held after exit) and passes 8 runs in a row with it Assistant-verification: manual scenario passed: SIGTERM as soon as Chopin answered, then an immediate restart; refused 3/5 before the fix, 0/10 after Assistant-verification: bun test passed: GIT_CONFIG_GLOBAL=/dev/null bun test (0 fail), and the PostgreSQL lifecycle suite (122 pass) Assistant-verification: typecheck and lint passed: bun run types and bun run ci Co-authored-by: Alex Lavaee <lavaman131@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.
A Chopin stopped soon after it started could exit without releasing the
chopin:writerlease. The next process on the same database then refusedto start with "another Chopin instance owns the database" until the lease
expired, up to 30 seconds later.
Two gaps caused this:
of startup: after the server was already listening, and after research
recovery and the job runner started. A stop in that window took Node's
default action, so the process exited without running the drain that
releases the lease. Supervisors that start Chopin, wait for it to
answer and then stop it hit this often, especially on slower machines.
A drain that ran while the reset was in flight released the old
fencing value, which no longer matched the row.
The handlers are now registered as soon as the lease is held. The drain
waits for the session reset before it releases, and skips stopping the
HTTP server when it isn't listening yet.
Reproduced by starting Chopin, sending SIGTERM as soon as it answered
HTTP, and starting it again at once: the restart was refused 3 times in
5 before this change and 0 in 10 after it.
Assistant-model: Claude Opus 5.5 (fast)
Assistant-workflow: inline
Assistant-verification: red-before-fix passed: the new startup.test.ts case, which stops the server as soon as it holds the lease, fails without the change (lease still held after exit) and passes 8 runs in a row with it
Assistant-verification: manual scenario passed: SIGTERM as soon as Chopin answered, then an immediate restart; refused 3/5 before the fix, 0/10 after
Assistant-verification: bun test passed: GIT_CONFIG_GLOBAL=/dev/null bun test (0 fail), and the PostgreSQL lifecycle suite (122 pass)
Assistant-verification: typecheck and lint passed: bun run types and bun run ci
Co-authored-by: Alex Lavaee lavaman131@github.com