Repository navigation
Conversation
…ith a cap when retrying Redis connections, otherwise we don't recover from outages automatically.
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.
If Redis is unavailable for more than about 12 seconds, the egress gateway never recovers from this.
The ledger client's
retryStrategyreturnsnullafter 5 attempts 2 seconds apart, after whichioredisends the connection for good. The/readyprobe then fails on every request becausepingEgressLedgergetsConnection is closed, but the/liveprobe keeps returning 200. As a result, Kubernetes leaves the pod running inNotReadystate indefinitely, and the system can never recover without manual intervention.We hit this when the node running the bundled Redis was drained. Redis came back on another node within a minute or so, but the gateway stayed
NotReadyuntil we deleted the pod manually.This PR changes the ledger client to use the existing shared helper used by the queue connections (
redisReconnectDelay).The options are moved into an exported
egressLedgerRedisOptions()so the retry behaviour can be tested without a slow outage test.I added a test in
egress-ledger-reconnect.test.tsto verify we get suitable integers up to the cap of 2000ms, rather than returningnullafter the 6th retry.I ran a local test which isn't included here as it takes ~25 seconds to run, but it tested the behaviour against a readl
redis-server, killed the server, saw the pings fail, then restarted it and see pings return. It fails onmainwithConnection is closed, but works on this branch.