Eagerly reconnect in PgListener::try_recv
#3585
Merged
Conversation
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
abonander
approved these changes
Nov 27, 2024
When PgListener's underlying connection is closed, try_recv() will return Ok(None) and reconnect on the next call. In this case, user code is supposed to reload its state from the database (or otherwise handle potentially missing messages). However, if the user code uses another database connection to do so then there is a period between when the state is reloaded and PgListener's connection is re-established where notifications are lost without any indication that this has happened. This commit changes PgListener to eagerly reconnect by default. At the suggestion of @abonander on discord, I have also included an option to switch back to the old behaviour in the case where someone was depending on it. Now, if the connection is closed then, by default, user code can do whatever it needs to do in order to recover and any notifications emitted in the meantime will be waiting for it when it is done.
swlynch99
force-pushed
the
pglistener-eager-reconnect
branch
from
ed49eef to
d782e42
Compare
Contributor
Author
|
Formatting should be fixed now. That was the only CI job that failed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When
PgListener's underlying connection is closed,try_recv()will returnOk(None)and reconnect on the next call. In this case, user code is supposed to reload its state from the database (or otherwise handle potentially missing messages). However, if the user code uses another database connection to do so then there is a period between when the state is reloaded andPgListener's connection is re-established where notifications are lost without any indication that this has happened.This commit changes
PgListenerto eagerly reconnect by default. At the suggestion of @abonander on discord, I have also included an option to switch back to the old behaviour in the case where someone was depending on it.Now, if the connection is closed then, by default, user code can do whatever it needs to do in order to recover and any notifications emitted in the meantime will be waiting for it when it is done.
I have defaulted
eager_connecttotruesince I think the current behaviour is a bit of a footgun but if you think that is too big of a change then I can change it to false.