Repository navigation
Keep a long-poll publish when its listen has already expired - #6899
Closed
oliviasculley wants to merge 1 commit into
Closed
oliviasculley wants to merge 1 commit into
oliviasculley wants to merge 1 commit into
Conversation
A flush that runs after the listen queued its expired notice used to take the buffer and send it to a listen that had already returned. The next listen then had nothing. The expired listen also takes a publish that is already in its mailbox.
oliviasculley
marked this pull request as ready for review
November 6, 2026 06:06
SteffenDE
added a commit
that referenced
this pull request
Nov 6, 2026
A listen that waited for `window_ms` sent `:expired` to the session and
returned 204 right away. The session could still handle a flush that the
listen had sent before. It then sent its buffered messages to a process
that no longer read them and cleared the buffer, so the next listen
received nothing. This happens when the session is busy for longer than
`window_ms` while a flush of the listen is queued.
The listen now waits for the session before it returns. After the
window, it sends `:expired` followed by `:subscribe`, which the session
already echoes. The session receives the messages of a listen in the
order they were sent, so the echo tells the listen that no flush is
left:
* If the session answered a flush, the messages arrive before the
echo and the listen returns them.
* If the echo arrives first, the listen returns 204 as before. The
session keeps what it buffers afterwards for the next listen.
* If neither arrives within `pubsub_timeout_ms`, the listen responds
with status 410. It cannot tell whether the session still answers
the flush. phoenix.js then starts a new session and rejoins its
channels, as it does when a poll cannot resume its session.
When the messages arrive first, the echo stays unread. With Cowboy, the
request process stops after the response. Bandit ignores unhandled
messages.
Only the listen changes. The session code is the same, so nodes with and
without this change can serve the same session during a rolling deploy.
A listen served by a node without the change keeps the old behavior.
A poll against a session that stopped or is blocked now returns 410
after `window_ms` plus `pubsub_timeout_ms`. Before, it returned 204
after `window_ms`, and the next poll returned 410.
Messages can still be lost when the response does not reach the client.
That needs acknowledgements from the client, see #6885.
The tests hold the session before it handles a flush. They cover a
session that continues after the listen window and one that does not
answer in time, for Bandit and Cowboy, with direct and PubSub delivery.
The problem was reported in #6899.
SteffenDE
added a commit
that referenced
this pull request
Nov 6, 2026
A listen that waited for `window_ms` sent `:expired` to the session and
returned 204 right away. The session could still handle a flush that the
listen had sent before. It then sent its buffered messages to a process
that no longer read them and cleared the buffer, so the next listen
received nothing. This happens when the session is busy for longer than
`window_ms` while a flush of the listen is queued.
The listen now waits for the session before it returns. After the
window, it sends `:expired` followed by `:subscribe`, which the session
already echoes. The session receives the messages of a listen in the
order they were sent, so the echo tells the listen that no flush is
left:
* If the session answered a flush, the messages arrive before the
echo and the listen returns them.
* If the echo arrives first, the listen returns 204 as before. The
session keeps what it buffers afterwards for the next listen.
* If neither arrives within `pubsub_timeout_ms`, the listen responds
with status 410. It cannot tell whether the session still answers
the flush. phoenix.js then starts a new session and rejoins its
channels, as it does when a poll cannot resume its session.
When the messages arrive first, the echo stays unread. With Cowboy, the
request process stops after the response. Bandit ignores unhandled
messages.
Only the listen changes. The session code is the same, so nodes with and
without this change can serve the same session during a rolling deploy.
A listen served by a node without the change keeps the old behavior.
A poll against a session that stopped or is blocked now returns 410
after `window_ms` plus `pubsub_timeout_ms`. Before, it returned 204
after `window_ms`, and the next poll returned 410.
Messages can still be lost when the response does not reach the client.
That needs acknowledgements from the client, see #6885.
The tests hold the session before it handles a flush. They cover a
session that continues after the listen window and one that does not
answer in time, for Bandit and Cowboy, with direct and PubSub delivery.
Closes #6899.
Member
|
Thank you, handled in #6901 (merged into 1.8). We're planning some changes to long polling in 1.9, which will also handle this. |
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 listen that hits
window_msbroadcasts{:expired, client_ref, ref}and returns no content. Two orderings drop a publish that was in flight for that listen.If the publish is already in the listen process mailbox when the timer fires, the old code returned before reading it. The message is then gone.
If the session flushes after that expired notice is already in its mailbox, the old code took the buffer and sent it to a listen that had already returned. The next listen then had nothing.
expire_listen/4still broadcasts the expired notice, then takes{:messages, _, ref}withreceiveafter 0 when one is already waiting. The session flush leaves the buffer alone when that same expired notice is already queued, and clears the client ref for that listen. The next listen returns the publish.The test holds the session inside a join, lets the listen expire, and only then releases the flush. The expired listen is status 204. The next listen contains the join reply. That passes on Bandit and on Cowboy.
I do not have an issue for this. I can open one if that is easier to track. Is this the right place for the keep, or would you rather the listen always drain and leave the session as it is?