Skip to content

Commit

Permalink
fix: disable retries on cluster subscriber connections. See discussio…
Browse files Browse the repository at this point in the history
…n in #1589
  • Loading branch information
headlessme committed Jun 13, 2022
1 parent 200e0f4 commit 68457a6
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions lib/cluster/ClusterSubscriber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ export default class ClusterSubscriber {
private connectionPool: ConnectionPool,
private emitter: EventEmitter
) {
// If the current node we're using as the subscriber disappears
// from the node pool for some reason, we will select a new one
// to connect to.
// Note that this event is only triggered if the connection to
// the node has been used; cluster subscriptions are setup with
// lazyConnect = true. It's possible for the subscriber node to
// disappear without this method being called!
// See https://github.com/luin/ioredis/pull/1589
this.connectionPool.on("-node", (_, key: string) => {
if (!this.started || !this.subscriber) {
return;
Expand Down Expand Up @@ -97,6 +105,10 @@ export default class ClusterSubscriber {
connectionName: getConnectionName("subscriber", options.connectionName),
lazyConnect: true,
tls: options.tls,
// Don't try to reconnect the subscriber connection. If the connection fails
// we will get an end event (handled below), at which point we'll pick a new
// node from the pool and try to connect to that as the subscriber connection.
retryStrategy: null
});

// Ignore the errors since they're handled in the connection pool.
Expand All @@ -105,14 +117,19 @@ export default class ClusterSubscriber {
// The node we lost connection to may not come back up in a
// reasonable amount of time (e.g. a slave that's taken down
// for maintainence), we could potentially miss many published
// messages so we should reconnect as quickly as possible.
// messages so we should reconnect as quickly as possible, to
// a different node if needed.
const currentSub = this.subscriber;
this.subscriber.once("close", () => {
this.subscriber.once("end", () => {
if (!this.started) {
debug("subscriber has disconnected, but ClusterSubscriber is not started, so not reconnecting.");
return;
}
// If the subscriber closes whilst it's still the active connection,
// we might as well try to connecting to a new node if possible to
// minimise the number of missed publishes.
if (this.started && currentSub === this.subscriber) {
debug("current subscriber closed, selecting a new node.");
if (currentSub === this.subscriber) {
debug("subscriber has disconnected, selecting a new one...");
this.selectSubscriber();
}
});
Expand Down

0 comments on commit 68457a6

Please sign in to comment.