-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[sentinel] update master address if it changes. #847
Conversation
Otherwise we'll end up spamming the connection pool because each time the master is discovered, it'll trigger the disconnect of the pool - as `self.master_address` never gets updated.
Can we get this merged to master? The connections in the pool all become one-shot connections if the master moves. When those connections are flushed quickly like that, you end up with tons of connections in TIME_WAIT state. Eventually this leads to having no source ports available for your connection, which results in this error:
|
LGTM! |
+1 here. Really need this merged. Having to monkey-patch. |
@andymccurdy Are there any plans on merging this? |
@andymccurdy bump on this one! |
Would be great if it could be merged. We're running into issues when running a lot of tasks in Celery with Gevent and Redis as it's result backend. Whenever the master moves, we get errors like these (only a restart of Celery fixes it):
We waited until 3.2.0 to see if it fixed things but it didn't. By applying this patch, all tasks run fine again after a master move. (Except the first task after the move, complaining about "connection reset by peer") |
@andymccurdy Could we get this merged? |
Thanks! |
Otherwise we'll end up spamming the connection pool because each time the master is discovered, it'll trigger the disconnect of the pool - as
self.master_address
never gets updated.