From 56fc99754f18bb296feba7824ad07073cdfc639a Mon Sep 17 00:00:00 2001 From: Nikolay Karadzhov Date: Wed, 22 Jul 2026 18:12:39 +0300 Subject: [PATCH 1/2] test(client): raise timeout for cluster PubSub listener-move test The 'should move listeners when PubSub node disconnects from the cluster' test reassigns all 8192 slots one-by-one and then polls both nodes for cluster_state:ok. On slow CI runners that work can exceed mocha's default 2000ms timeout, causing intermittent "Timeout of 2000ms exceeded" failures across many PRs. testWithCluster ran the it() at the default timeout with no way to override it (only the before hook set 30000ms). Add an optional testTimeout to the shared CommonTestOptions, apply it in testWithCluster's it() when set, and raise this test's timeout to 30000ms. The guard means all other cluster tests are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/client/lib/cluster/index.spec.ts | 5 ++++- packages/test-utils/lib/index.ts | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/client/lib/cluster/index.spec.ts b/packages/client/lib/cluster/index.spec.ts index 6de2a8a623e..d3c6376205d 100644 --- a/packages/client/lib/cluster/index.spec.ts +++ b/packages/client/lib/cluster/index.spec.ts @@ -431,7 +431,10 @@ describe('Cluster', () => { }, { serverArguments: [], numberOfMasters: 2, - minimumDockerVersion: [7] + minimumDockerVersion: [7], + // reassigning all 8192 slots one-by-one plus polling both nodes for + // `cluster_state:ok` can exceed mocha's default 2000ms on slow CI runners + testTimeout: 30000 }); testUtils.testWithCluster('ssubscribe & sunsubscribe', async cluster => { diff --git a/packages/test-utils/lib/index.ts b/packages/test-utils/lib/index.ts index d63a9fb6e1a..ac0fb3b144c 100644 --- a/packages/test-utils/lib/index.ts +++ b/packages/test-utils/lib/index.ts @@ -109,6 +109,7 @@ interface CommonTestOptions { serverArguments: Array; minimumDockerVersion?: Array; skipTest?: boolean; + testTimeout?: number; } interface ClientTestOptions< @@ -886,6 +887,7 @@ export default class TestUtils { } it(title, async function () { + if (options.testTimeout) this.timeout(options.testTimeout); if (options.skipTest) return this.skip(); if (!dockersPromise) return this.skip(); const RESP = (options.clusterConfiguration?.RESP ?? DEFAULT_RESP) as RESP; From a8c075f004c7966134463f5bd6db173fb61bbc0d Mon Sep 17 00:00:00 2001 From: Nikolay Karadzhov Date: Wed, 22 Jul 2026 21:45:35 +0300 Subject: [PATCH 2/2] test(client): fix cluster PubSub listener-move test timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'should move listeners when PubSub node disconnects from the cluster' test kept exceeding its timeout on slow CI runners. Raising the timeout only masked the problem; the root cause was two bugs. 1. Slow slot reassignment. The test reassigned an entire 8192-slot half one slot at a time with `CLUSTER SETSLOT .. NODE` against both nodes — 16384 round-trips — which overflowed even the raised 30s timeout. Replace the per-slot loop with a single `CLUSTER DELSLOTSRANGE`/`ADDSLOTSRANGE` pair. 2. Wrong ownership check. The range to migrate was selected with `cluster.slots[0].master === migrating`, an object-identity comparison that did not hold, so the range was chosen incorrectly. The old per-slot `SETSLOT NODE` masked this because it reassigns a slot regardless of current owner; `ADDSLOTS` does not. Compare by `.address` instead. `CLUSTER ADDSLOTS` rejects a slot still assigned in the node's own cluster view, and gossip has not yet propagated the migrating node's release to the importing node, so release the range on both nodes before claiming it on the importing node (mirrors the existing sharded-channel-moved test). The existing `cluster_state:ok` polling continues to cover reconvergence. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/client/lib/cluster/index.spec.ts | 33 ++++++++++++----------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/client/lib/cluster/index.spec.ts b/packages/client/lib/cluster/index.spec.ts index d3c6376205d..f4b94fc81b8 100644 --- a/packages/client/lib/cluster/index.spec.ts +++ b/packages/client/lib/cluster/index.spec.ts @@ -390,7 +390,7 @@ describe('Cluster', () => { cluster.nodeClient(importing) ]); - const range = cluster.slots[0].master === migrating ? { + const range = cluster.slots[0].master.address === migrating.address ? { key: 'bar', // 5061 start: 0, end: 8191 @@ -400,18 +400,21 @@ describe('Cluster', () => { end: 16383 }; - // reassigning slots with `CLUSTER SETSLOT .. NODE` can leave the nodes in a - // transient `cluster_state:fail` state, so claim the slots on the importing - // node first, then release them on the migrating node, and wait for both - // nodes to report a healthy cluster before going on - const importingPromises: Array> = [], - migratingPromises: Array> = []; - for (let i = range.start; i <= range.end; i++) { - importingPromises.push(importingClient.clusterSetSlot(i, 'NODE', importing.id)); - migratingPromises.push(migratingClient.clusterSetSlot(i, 'NODE', importing.id)); - } - await Promise.all(importingPromises); - await Promise.all(migratingPromises); + // Reassign the whole slot range in one shot with DEL/ADDSLOTSRANGE + // instead of thousands of per-slot `CLUSTER SETSLOT .. NODE` calls, + // which overflowed the test timeout on slow CI runners. Release the + // range on both nodes first (`CLUSTER ADDSLOTS` rejects a slot that is + // still assigned in the node's own cluster view, and gossip has not yet + // propagated the migrating node's release to the importing node), then + // claim it on the importing node, then wait for both nodes to reconverge + // to a healthy cluster before going on (the reassignment leaves a + // transient `cluster_state:fail` window). + const slotRange = { start: range.start, end: range.end }; + await Promise.all([ + migratingClient.clusterDelSlotsRange(slotRange), + importingClient.clusterDelSlotsRange(slotRange) + ]); + await importingClient.clusterAddSlotsRange(slotRange); for (const client of [migratingClient, importingClient]) { while (!(await client.clusterInfo()).startsWith('cluster_state:ok')) { @@ -432,8 +435,8 @@ describe('Cluster', () => { serverArguments: [], numberOfMasters: 2, minimumDockerVersion: [7], - // reassigning all 8192 slots one-by-one plus polling both nodes for - // `cluster_state:ok` can exceed mocha's default 2000ms on slow CI runners + // range reassignment plus polling both nodes for `cluster_state:ok` can + // exceed mocha's default 2000ms on slow CI runners testTimeout: 30000 });