From aff3c9b127b4c8025c9ef611d9dfc322325c930c Mon Sep 17 00:00:00 2001 From: Mik13 Date: Thu, 23 Feb 2023 12:35:14 +0100 Subject: [PATCH 01/10] Fix multi.exec with empty queue and previous watch When calling exec on a multi instance which you did not use, no command is sent currently. This is a problem for watched keys, because no EXEC means no unwatch, which might cause hard-to-debug problems. Proposed Fix: Sending UNWATCH --- packages/client/lib/client/multi-command.ts | 1 - packages/client/lib/cluster/multi-command.ts | 1 - packages/client/lib/multi-command.spec.ts | 8 +++++--- packages/client/lib/multi-command.ts | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/client/lib/client/multi-command.ts b/packages/client/lib/client/multi-command.ts index 749ab4c4e0f..60fdbdac2a9 100644 --- a/packages/client/lib/client/multi-command.ts +++ b/packages/client/lib/client/multi-command.ts @@ -171,7 +171,6 @@ export default class RedisClientMultiCommand { } const commands = this.#multi.exec(); - if (!commands) return []; return this.#multi.handleExecReplies( await this.#executor( diff --git a/packages/client/lib/cluster/multi-command.ts b/packages/client/lib/cluster/multi-command.ts index 52ab6eb0e23..5e15fec16fc 100644 --- a/packages/client/lib/cluster/multi-command.ts +++ b/packages/client/lib/cluster/multi-command.ts @@ -121,7 +121,6 @@ export default class RedisClusterMultiCommand { } const commands = this.#multi.exec(); - if (!commands) return []; return this.#multi.handleExecReplies( await this.#executor(commands, this.#firstKey, RedisMultiCommand.generateChainId()) diff --git a/packages/client/lib/multi-command.spec.ts b/packages/client/lib/multi-command.spec.ts index 7e9667cd518..24e691e90c0 100644 --- a/packages/client/lib/multi-command.spec.ts +++ b/packages/client/lib/multi-command.spec.ts @@ -46,14 +46,16 @@ describe('Multi Command', () => { }); describe('exec', () => { - it('undefined', () => { + it('without commands', () => { assert.equal( new RedisMultiCommand().exec(), - undefined + [ + { args: ['UNWATCH'] } + ] ); }); - it('Array', () => { + it('with commands', () => { const multi = new RedisMultiCommand(); multi.addCommand(['PING']); diff --git a/packages/client/lib/multi-command.ts b/packages/client/lib/multi-command.ts index 8865cc8e002..dba14631f0b 100644 --- a/packages/client/lib/multi-command.ts +++ b/packages/client/lib/multi-command.ts @@ -69,9 +69,9 @@ export default class RedisMultiCommand { return transformedArguments; } - exec(): undefined | Array { + exec(): Array { if (!this.queue.length) { - return; + return [{ args: ['UNWATCH'] }]; } return [ From 48259a741b551e956b6fb5857d441928e5507794 Mon Sep 17 00:00:00 2001 From: Leibale Date: Thu, 23 Feb 2023 14:30:42 -0500 Subject: [PATCH 02/10] execute empty multi command (instead of skipping) --- packages/client/lib/client/index.ts | 27 +++++++++++++------- packages/client/lib/client/multi-command.ts | 26 +++++++++---------- packages/client/lib/cluster/multi-command.ts | 24 ++++++++--------- packages/client/lib/multi-command.spec.ts | 19 ++++++-------- packages/client/lib/multi-command.ts | 12 --------- 5 files changed, 49 insertions(+), 59 deletions(-) diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index b4bf49fc7bc..00c2cff73dc 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -75,13 +75,13 @@ type WithCommands = { export type WithModules = { [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P] as ExcludeMappedString]: RedisCommandSignature; + [C in keyof M[P]as ExcludeMappedString]: RedisCommandSignature; }; }; export type WithFunctions = { [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P] as ExcludeMappedString]: RedisCommandSignature; + [FF in keyof F[P]as ExcludeMappedString]: RedisCommandSignature; }; }; @@ -350,7 +350,7 @@ export default class RedisClient< } }; - for (const [ name, command ] of Object.entries(COMMANDS as RedisCommands)) { + for (const [name, command] of Object.entries(COMMANDS as RedisCommands)) { this.#defineLegacyCommand(name, command); (this as any)[name.toLowerCase()] ??= (this as any)[name]; } @@ -460,7 +460,7 @@ export default class RedisClient< ); } else if (!this.#socket.isReady && this.#options?.disableOfflineQueue) { return Promise.reject(new ClientOfflineError()); - } + } const promise = this.#queue.addCommand(args, options); this.#tick(); @@ -725,11 +725,14 @@ export default class RedisClient< return Promise.reject(new ClientClosedError()); } - const promise = Promise.all( - commands.map(({ args }) => { - return this.#queue.addCommand(args, { chainId }); - }) - ); + const promise = chainId ? + // if `chainId` has a value, it's a `MULTI` (and not "pipeline") - need to add the `MULTI` and `EXEC` commands + Promise.all([ + this.#queue.addCommand(['MULTI'], { chainId }), + this.#addMultiCommands(commands, chainId), + this.#queue.addCommand(['EXEC'], { chainId }) + ]) : + this.#addMultiCommands(commands, chainId); this.#tick(); @@ -742,6 +745,12 @@ export default class RedisClient< return results; } + #addMultiCommands(commands: Array, chainId?: symbol) { + return Promise.all( + commands.map(({ args }) => this.#queue.addCommand(args, { chainId })) + ); + } + async* scanIterator(options?: ScanCommandOptions): AsyncIterable { let cursor = 0; do { diff --git a/packages/client/lib/client/multi-command.ts b/packages/client/lib/client/multi-command.ts index 60fdbdac2a9..b28d77fca6a 100644 --- a/packages/client/lib/client/multi-command.ts +++ b/packages/client/lib/client/multi-command.ts @@ -15,36 +15,36 @@ type WithCommands< F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof typeof COMMANDS]: CommandSignature<(typeof COMMANDS)[P], M, F, S>; -}; + [P in keyof typeof COMMANDS]: CommandSignature<(typeof COMMANDS)[P], M, F, S>; + }; type WithModules< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P] as ExcludeMappedString]: CommandSignature; + [P in keyof M as ExcludeMappedString

]: { + [C in keyof M[P]as ExcludeMappedString]: CommandSignature; + }; }; -}; type WithFunctions< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P] as ExcludeMappedString]: CommandSignature; + [P in keyof F as ExcludeMappedString

]: { + [FF in keyof F[P]as ExcludeMappedString]: CommandSignature; + }; }; -}; type WithScripts< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof S as ExcludeMappedString

]: CommandSignature; -}; + [P in keyof S as ExcludeMappedString

]: CommandSignature; + }; export type RedisClientMultiCommandType< M extends RedisModules, @@ -117,7 +117,7 @@ export default class RedisClientMultiCommand { }); }; - for (const [ name, command ] of Object.entries(COMMANDS as RedisCommands)) { + for (const [name, command] of Object.entries(COMMANDS as RedisCommands)) { this.#defineLegacyCommand(name, command); (this as any)[name.toLowerCase()] ??= (this as any)[name]; } @@ -170,11 +170,9 @@ export default class RedisClientMultiCommand { return this.execAsPipeline(); } - const commands = this.#multi.exec(); - return this.#multi.handleExecReplies( await this.#executor( - commands, + this.#multi.queue, this.#selectedDB, RedisMultiCommand.generateChainId() ) diff --git a/packages/client/lib/cluster/multi-command.ts b/packages/client/lib/cluster/multi-command.ts index 5e15fec16fc..0e870ff56de 100644 --- a/packages/client/lib/cluster/multi-command.ts +++ b/packages/client/lib/cluster/multi-command.ts @@ -16,36 +16,36 @@ type WithCommands< F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof typeof COMMANDS]: RedisClusterMultiCommandSignature<(typeof COMMANDS)[P], M, F, S>; -}; + [P in keyof typeof COMMANDS]: RedisClusterMultiCommandSignature<(typeof COMMANDS)[P], M, F, S>; + }; type WithModules< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P] as ExcludeMappedString]: RedisClusterMultiCommandSignature; + [P in keyof M as ExcludeMappedString

]: { + [C in keyof M[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; + }; }; -}; type WithFunctions< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P] as ExcludeMappedString]: RedisClusterMultiCommandSignature; + [P in keyof F as ExcludeMappedString

]: { + [FF in keyof F[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; + }; }; -}; type WithScripts< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof S as ExcludeMappedString

]: RedisClusterMultiCommandSignature; -}; + [P in keyof S as ExcludeMappedString

]: RedisClusterMultiCommandSignature; + }; export type RedisClusterMultiCommandType< M extends RedisModules, @@ -120,10 +120,8 @@ export default class RedisClusterMultiCommand { return this.execAsPipeline(); } - const commands = this.#multi.exec(); - return this.#multi.handleExecReplies( - await this.#executor(commands, this.#firstKey, RedisMultiCommand.generateChainId()) + await this.#executor(this.#multi.queue, this.#firstKey, RedisMultiCommand.generateChainId()) ); } diff --git a/packages/client/lib/multi-command.spec.ts b/packages/client/lib/multi-command.spec.ts index 24e691e90c0..b0f79c6e157 100644 --- a/packages/client/lib/multi-command.spec.ts +++ b/packages/client/lib/multi-command.spec.ts @@ -47,11 +47,9 @@ describe('Multi Command', () => { describe('exec', () => { it('without commands', () => { - assert.equal( - new RedisMultiCommand().exec(), - [ - { args: ['UNWATCH'] } - ] + assert.deepEqual( + new RedisMultiCommand().queue, + [] ); }); @@ -60,12 +58,11 @@ describe('Multi Command', () => { multi.addCommand(['PING']); assert.deepEqual( - multi.exec(), - [ - { args: ['MULTI'] }, - { args: ['PING'], transformReply: undefined }, - { args: ['EXEC'] } - ] + multi.queue, + [{ + args: ['PING'], + transformReply: undefined + }] ); }); }); diff --git a/packages/client/lib/multi-command.ts b/packages/client/lib/multi-command.ts index dba14631f0b..08f23ffa454 100644 --- a/packages/client/lib/multi-command.ts +++ b/packages/client/lib/multi-command.ts @@ -69,18 +69,6 @@ export default class RedisMultiCommand { return transformedArguments; } - exec(): Array { - if (!this.queue.length) { - return [{ args: ['UNWATCH'] }]; - } - - return [ - { args: ['MULTI'] }, - ...this.queue, - { args: ['EXEC'] } - ]; - } - handleExecReplies(rawReplies: Array): Array { const execReply = rawReplies[rawReplies.length - 1] as (null | Array); if (execReply === null) { From 7654f5f167d01e8ddd4d1e8a21e057d300f75b7c Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 23 Feb 2023 15:38:10 -0500 Subject: [PATCH 03/10] Update index.ts --- packages/client/lib/client/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 00c2cff73dc..0edf84a3883 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -75,13 +75,13 @@ type WithCommands = { export type WithModules = { [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P]as ExcludeMappedString]: RedisCommandSignature; + [C in keyof M[P] as ExcludeMappedString]: RedisCommandSignature; }; }; export type WithFunctions = { [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P]as ExcludeMappedString]: RedisCommandSignature; + [FF in keyof F[P] as ExcludeMappedString]: RedisCommandSignature; }; }; From c488138451b3b37f857b32034c883ef796354373 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 23 Feb 2023 15:39:15 -0500 Subject: [PATCH 04/10] Update index.ts --- packages/client/lib/client/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 0edf84a3883..c8161a6368d 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -350,7 +350,7 @@ export default class RedisClient< } }; - for (const [name, command] of Object.entries(COMMANDS as RedisCommands)) { + for (const [ name, command ] of Object.entries(COMMANDS as RedisCommands)) { this.#defineLegacyCommand(name, command); (this as any)[name.toLowerCase()] ??= (this as any)[name]; } From bf6470ce085777b561e3567b59204d2815c770d7 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 23 Feb 2023 15:39:50 -0500 Subject: [PATCH 05/10] Update multi-command.ts --- packages/client/lib/client/multi-command.ts | 22 ++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/client/lib/client/multi-command.ts b/packages/client/lib/client/multi-command.ts index b28d77fca6a..d786715da27 100644 --- a/packages/client/lib/client/multi-command.ts +++ b/packages/client/lib/client/multi-command.ts @@ -15,36 +15,36 @@ type WithCommands< F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof typeof COMMANDS]: CommandSignature<(typeof COMMANDS)[P], M, F, S>; - }; + [P in keyof typeof COMMANDS]: CommandSignature<(typeof COMMANDS)[P], M, F, S>; +}; type WithModules< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P]as ExcludeMappedString]: CommandSignature; - }; + [P in keyof M as ExcludeMappedString

]: { + [C in keyof M[P]as ExcludeMappedString]: CommandSignature; }; +}; type WithFunctions< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P]as ExcludeMappedString]: CommandSignature; - }; + [P in keyof F as ExcludeMappedString

]: { + [FF in keyof F[P]as ExcludeMappedString]: CommandSignature; }; +}; type WithScripts< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof S as ExcludeMappedString

]: CommandSignature; - }; + [P in keyof S as ExcludeMappedString

]: CommandSignature; +}; export type RedisClientMultiCommandType< M extends RedisModules, @@ -117,7 +117,7 @@ export default class RedisClientMultiCommand { }); }; - for (const [name, command] of Object.entries(COMMANDS as RedisCommands)) { + for (const [ name, command ] of Object.entries(COMMANDS as RedisCommands)) { this.#defineLegacyCommand(name, command); (this as any)[name.toLowerCase()] ??= (this as any)[name]; } From 71c1b431a9e22354f94015d878beb49e232836ab Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 23 Feb 2023 15:40:17 -0500 Subject: [PATCH 06/10] Update multi-command.ts --- packages/client/lib/cluster/multi-command.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/client/lib/cluster/multi-command.ts b/packages/client/lib/cluster/multi-command.ts index 0e870ff56de..bd8cbc6aeca 100644 --- a/packages/client/lib/cluster/multi-command.ts +++ b/packages/client/lib/cluster/multi-command.ts @@ -16,36 +16,36 @@ type WithCommands< F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof typeof COMMANDS]: RedisClusterMultiCommandSignature<(typeof COMMANDS)[P], M, F, S>; - }; + [P in keyof typeof COMMANDS]: RedisClusterMultiCommandSignature<(typeof COMMANDS)[P], M, F, S>; +}; type WithModules< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; - }; + [P in keyof M as ExcludeMappedString

]: { + [C in keyof M[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; }; +}; type WithFunctions< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; - }; + [P in keyof F as ExcludeMappedString

]: { + [FF in keyof F[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; }; +}; type WithScripts< M extends RedisModules, F extends RedisFunctions, S extends RedisScripts > = { - [P in keyof S as ExcludeMappedString

]: RedisClusterMultiCommandSignature; - }; + [P in keyof S as ExcludeMappedString

]: RedisClusterMultiCommandSignature; +}; export type RedisClusterMultiCommandType< M extends RedisModules, From 8658a87787ab1a6704e4fb007f5cc2a1840c4a40 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 23 Feb 2023 15:40:50 -0500 Subject: [PATCH 07/10] Update multi-command.ts --- packages/client/lib/client/multi-command.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/client/lib/client/multi-command.ts b/packages/client/lib/client/multi-command.ts index d786715da27..d33346b5212 100644 --- a/packages/client/lib/client/multi-command.ts +++ b/packages/client/lib/client/multi-command.ts @@ -24,7 +24,7 @@ type WithModules< S extends RedisScripts > = { [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P]as ExcludeMappedString]: CommandSignature; + [C in keyof M[P] as ExcludeMappedString]: CommandSignature; }; }; @@ -34,7 +34,7 @@ type WithFunctions< S extends RedisScripts > = { [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P]as ExcludeMappedString]: CommandSignature; + [FF in keyof F[P] as ExcludeMappedString]: CommandSignature; }; }; From 086e260653da6935e479369725846e85de2b740d Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 23 Feb 2023 15:41:23 -0500 Subject: [PATCH 08/10] Update multi-command.ts --- packages/client/lib/cluster/multi-command.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/client/lib/cluster/multi-command.ts b/packages/client/lib/cluster/multi-command.ts index bd8cbc6aeca..ef3c7590ec7 100644 --- a/packages/client/lib/cluster/multi-command.ts +++ b/packages/client/lib/cluster/multi-command.ts @@ -25,7 +25,7 @@ type WithModules< S extends RedisScripts > = { [P in keyof M as ExcludeMappedString

]: { - [C in keyof M[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; + [C in keyof M[P] as ExcludeMappedString]: RedisClusterMultiCommandSignature; }; }; @@ -35,7 +35,7 @@ type WithFunctions< S extends RedisScripts > = { [P in keyof F as ExcludeMappedString

]: { - [FF in keyof F[P]as ExcludeMappedString]: RedisClusterMultiCommandSignature; + [FF in keyof F[P] as ExcludeMappedString]: RedisClusterMultiCommandSignature; }; }; From ab5e09590fbd34b299b4150f753385e604441239 Mon Sep 17 00:00:00 2001 From: Leibale Date: Thu, 23 Feb 2023 16:16:30 -0500 Subject: [PATCH 09/10] short circuit empty pipelines --- packages/client/lib/client/index.spec.ts | 25 ++++++++++++++------- packages/client/lib/client/multi-command.ts | 2 ++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 25c966c2719..7396a2e9c37 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -518,14 +518,23 @@ describe('Client', () => { ); }, GLOBAL.SERVERS.OPEN); - testUtils.testWithClient('execAsPipeline', async client => { - assert.deepEqual( - await client.multi() - .ping() - .exec(true), - ['PONG'] - ); - }, GLOBAL.SERVERS.OPEN); + describe('execAsPipeline', () => { + testUtils.testWithClient('exec(true)', async client => { + assert.deepEqual( + await client.multi() + .ping() + .exec(true), + ['PONG'] + ); + }, GLOBAL.SERVERS.OPEN); + + testUtils.testWithClient('empty execAsPipeline', async client => { + assert.deepEqual( + await client.multi().execAsPipeline(), + [] + ); + }, GLOBAL.SERVERS.OPEN); + }); testUtils.testWithClient('should remember selected db', async client => { await client.multi() diff --git a/packages/client/lib/client/multi-command.ts b/packages/client/lib/client/multi-command.ts index d33346b5212..e347667bf2c 100644 --- a/packages/client/lib/client/multi-command.ts +++ b/packages/client/lib/client/multi-command.ts @@ -182,6 +182,8 @@ export default class RedisClientMultiCommand { EXEC = this.exec; async execAsPipeline(): Promise> { + if (this.#multi.queue.length === 0) return []; + return this.#multi.transformReplies( await this.#executor( this.#multi.queue, From 8fc6bf337a2100e12712a6d4ead4a71652b4807c Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Fri, 24 Feb 2023 14:27:43 -0500 Subject: [PATCH 10/10] Update index.ts --- packages/client/lib/client/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index c8161a6368d..5dd386647ef 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -732,7 +732,7 @@ export default class RedisClient< this.#addMultiCommands(commands, chainId), this.#queue.addCommand(['EXEC'], { chainId }) ]) : - this.#addMultiCommands(commands, chainId); + this.#addMultiCommands(commands); this.#tick();