Repository navigation
Fix socket drainer reporting an extra round and waiting after the last batch - #6897
Open
ramiscaleai wants to merge 1 commit into
Open
ramiscaleai wants to merge 1 commit into
ramiscaleai wants to merge 1 commit into
Conversation
PoolDrainer computed the number of rounds as div(total, size) + 1, which is one too many when the number of connections is a multiple of the batch size. The :socket_drain telemetry/log then reported e.g. "round 2 of 3" for a drain with two batches, and the drainer slept for :batch_interval after the last batch. Use a ceiling division and report the real size of each batch. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Phoenix.Socket.PoolDrainercomputes the number of rounds asdiv(total, size) + 1.Stream.chunk_every/2only producesceil(total / size)batches, so when the number of connections is an exact multiple of:batch_sizethe drainer believes there is one more round than there really is. Two effects:[:phoenix, :socket_drain]telemetry event, and the log line built from it, report a wrong number of rounds, e.g. "round 2 of 3" for a drain made of two batches.index < rounds, so the drainer also sleeps:batch_intervalafter the last batch, when there is nothing left to do. The example in thePhoenix.Endpoint.socket/3docs (150k connections with the default:batch_sizeof 10_000 and:batch_intervalof 2_000) is such a case: the drainer needs 15 x 2s = 30s, the whole default:shutdown, instead of 14 x 2s = 28s.The fix computes
roundswith a ceiling division and reportslength(pids)as thecountof each batch (the special case for the last batch is no longer needed).Tests
Nothing exercised
PoolDrainer.terminate/2, so this addstest/phoenix/socket/pool_drainer_test.exs. It drains an isolatedPoolSupervisorpool and checks:phx_drain;Before the fix, the exact multiple test fails (
rounds: 3, expected2) and the wait test fails (5001ms elapsed for a 5000ms interval): 5 tests, 2 failures. After the fix: 5 tests, 0 failures. The rest oftest/phoenixgives the same results with and without this change.I ran this locally on Elixir 1.14 / OTP 25, in a scratch copy of the repository with a small compatibility shim (
mix.exsrequires 1.15).lib/and the test file are identical to this PR; CI covers the supported versions.AI disclosure: Claude Code (an AI coding agent) found this bug, wrote the fix and the tests, and drafted this description. The tests above were run locally.
🤖 Generated with Claude Code