Repository navigation
Don't drop LongPoll messages flushed after the listen window - #6901
Merged
Merged
Conversation
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.
Contributor
|
Thank you! That was fast! |
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 waited for
window_mssent:expiredto 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 thanwindow_mswhile a flush of the listen is queued.The listen now waits for the session before it returns. After the window, it sends
:expiredfollowed 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: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_mspluspubsub_timeout_ms. Before, it returned 204 afterwindow_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.