Repository navigation
bus: replace a redis subscription that stops answering pings - #155
Open
cosmin-staicu wants to merge 1 commit into
Open
cosmin-staicu wants to merge 1 commit into
cosmin-staicu wants to merge 1 commit into
Conversation
The redis bus read its subscription with ReceiveMessage, which blocks with no deadline and never pings. go-redis health-checks a PubSub only through its Channel() API, so a subscription socket that silently stops delivering is never noticed: a connection dropped without a FIN, or one Redis stops serving once the credentials it authenticated with expire (go-redis re-authenticates pooled connections on streaming credential updates, but not PubSub ones). The process keeps publishing while receiving nothing, and RPCs fail with "no response from servers" until some later SUBSCRIBE or UNSUBSCRIBE happens to hit a write error. Read with ReceiveTimeout instead. A read that sees nothing for the health check interval (30s) sends a PING; if the next interval passes with no reply of any kind, or the subscription returns a Redis error reply such as NOAUTH, the bus opens a new PubSub subscribed to every current channel and closes the old one. The replacement and the subscription reconciler are serialized on a new mutex, and the reconciler now holds it until currentChannels reflects its change, so a replacement always subscribes exactly the current set. A quiet connection that answers its PING is kept. The tests put a TCP proxy between the subscriber and Redis that can silence the connections it holds without closing them, or inject an error reply, and check that delivery resumes on a new connection within two intervals and that a quiet healthy subscription is not replaced. Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>
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.
The Redis bus reads its subscription with
ReceiveMessage, which blocks with no deadline and never pings. go-redis only health-checks a PubSub through itsChannel()API, so a subscription socket that silently stops delivering is never noticed. Two ways this happens: the connection drops without a FIN, or Redis stops serving it once the credentials it authenticated with expire (go-redis re-authenticates pooled connections on streaming credential updates, but not PubSub ones). The process keeps publishing while it receives nothing, and RPCs fail with "no response from servers" until a later SUBSCRIBE or UNSUBSCRIBE happens to hit a write error.The read now uses
ReceiveTimeout. A read that sees nothing for the health check interval (30s) sends a PING. If the next interval passes with no reply of any kind, or the subscription returns a Redis error reply such as NOAUTH, the bus opens a new PubSub subscribed to every current channel and closes the old one. A quiet connection that answers its PING is kept.The replacement and the subscription reconciler are serialized on a new mutex, and the reconciler holds it until
currentChannelsreflects its change, so a replacement always subscribes exactly the current set.The tests put a TCP proxy between the subscriber and Redis that can silence the connections it holds without closing them, or inject an error reply, and check that delivery resumes on a new connection within two intervals and that a quiet healthy subscription is kept. A constructor in
export_test.goshortens the interval for them, so the public API is unchanged.