T-8292 [Fable] Atomic unix socket takeover + inode-guarded cleanup - #8
Merged
Merged
Conversation
After a full docker restart, a replacement ingester can bind its unix socket while the old container is still draining; the old server's stop() then unlinked the path by name, deleting the new server's socket. The new server keeps listening on an unlinked inode and never recovers (BetterStackHQ Linear T-8292). - start() now binds to a unique temp path and atomically renames it over the target, so the path always points at a live socket and takeover of a still-bound path is a single step. - stop() records the bound socket file's (dev, ino) and only unlinks the path if it still matches, so an older generation never removes a newer generation's socket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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.
Problem
After a complete docker shutdown/restart, unix sockets sometimes never recover.
Root cause is a cross-generation race in this gem's socket lifecycle:
start()did exists →remove_file→bind, silently stealing the path from a still-running older server, with a window where the path doesn't exist at all.stop()unconditionally unlinked the bind path by name, with no check that the file was still the socket this server bound.On a normal rolling deploy (swarm stop-first) generations never overlap, so this never bit. After a full docker restart, swarm reconciliation can start the new task while the old container is still draining (5s
DRAIN_TIME): the new server binds the socket, then the old server'sstop()deletes the new server's socket file. The new server keeps listening on an unlinked inode forever — haproxy gets ENOENT on every connect and nothing ever recreates the file, until the replica is manually restarted.The ticket's
WARN Failed to remove socket file: NotFoundlog line is that manual restart: the victim discovering at shutdown that its socket file had been missing for its whole run. The harmful deletion itself was silent.Fix
start(): bind to a unique temp path ({path}.{pid}.{seq}.tmp) and atomicallyrename(2)it over the target. The path always points at a live socket, and takeover of a still-bound path is a single step.stop(): record the bound socket file's(dev, ino)at bind time (lstat'd on the temp path, so it can't race) and only unlink the path if it still matches. An older generation can no longer remove a newer generation's socket; if the path was taken over it's left in place with an info log.Error-message change: startup failures at the target path now raise
Failed to install Unix socket file …(rename step) instead ofFailed to remove existing Unix socket file ….Tests
test_unix_socket_takeover_preserves_new_generation_socket— old server stops while a new server owns the path: socket file survives, is connectable, and the new server's own stop still cleans it up. Reproduces the production race (and the ticket's NotFound warning) on unfixed code.test_unix_socket_stop_leaves_foreign_file— stop leaves a file it didn't bind untouched.Full suite: 39 runs, 0 failures (1 skip = sudo-gated test).