Skip to content

Commit 79749f2

Browse files
authored
fix(sentinel): propagate RESP option to clients (#3011)
`createSentinel` takes RESP as an option, but does not propagate down to the actual clients. This creates confusion for the users as they expect the option to be set to all clients, which is reasonable. In case of clientSideCaching, this problem manifests as validation failure because clientSideCaching requires RESP3, but if we dont propagate, clients start with the default RESP2 fixes #3010
1 parent 6f3380b commit 79749f2

File tree

3 files changed

+22
-8
lines changed

3 files changed

+22
-8
lines changed

packages/client/lib/sentinel/index.spec.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('RedisSentinel', () => {
2323
{ host: 'localhost', port: 26379 }
2424
]
2525
};
26-
26+
2727
it('should throw error when clientSideCache is enabled with RESP 2', () => {
2828
assert.throws(
2929
() => RedisSentinel.create({
@@ -46,14 +46,24 @@ describe('RedisSentinel', () => {
4646
});
4747

4848
it('should not throw when clientSideCache is enabled with RESP 3', () => {
49-
assert.doesNotThrow(() =>
49+
assert.doesNotThrow(() =>
5050
RedisSentinel.create({
5151
...options,
5252
clientSideCache: clientSideCacheConfig,
5353
RESP: 3 as const,
5454
})
5555
);
5656
});
57+
58+
testUtils.testWithClientSentinel('should successfully connect to sentinel', async () => {
59+
}, {
60+
...GLOBAL.SENTINEL.OPEN,
61+
sentinelOptions: {
62+
RESP: 3,
63+
clientSideCache: { ttl: 0, maxEntries: 0, evictPolicy: 'LRU'},
64+
},
65+
})
66+
5767
});
5868
});
5969
});
@@ -417,7 +427,7 @@ async function steadyState(frame: SentinelFramework) {
417427
sentinel.setTracer(tracer);
418428
await sentinel.connect();
419429
await nodePromise;
420-
430+
421431
await sentinel.flushAll();
422432
} finally {
423433
if (sentinel !== undefined) {
@@ -443,7 +453,7 @@ describe('legacy tests', () => {
443453
this.timeout(15000);
444454

445455
last = Date.now();
446-
456+
447457
function deltaMeasurer() {
448458
const delta = Date.now() - last;
449459
if (delta > longestDelta) {
@@ -508,7 +518,7 @@ describe('legacy tests', () => {
508518
}
509519

510520
stopMeasuringBlocking = true;
511-
521+
512522
await frame.cleanup();
513523
})
514524

@@ -1032,6 +1042,3 @@ describe('legacy tests', () => {
10321042
})
10331043
});
10341044
});
1035-
1036-
1037-

packages/client/lib/sentinel/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,7 @@ class RedisSentinelInternal<
625625
readonly #sentinelClientOptions: RedisClientOptions<typeof RedisSentinelModule, RedisFunctions, RedisScripts, RespVersions, TypeMapping, RedisTcpSocketOptions>;
626626
readonly #scanInterval: number;
627627
readonly #passthroughClientErrorEvents: boolean;
628+
readonly #RESP?: RespVersions;
628629

629630
#anotherReset = false;
630631

@@ -673,6 +674,7 @@ class RedisSentinelInternal<
673674

674675
this.#name = options.name;
675676

677+
this.#RESP = options.RESP;
676678
this.#sentinelRootNodes = Array.from(options.sentinelRootNodes);
677679
this.#maxCommandRediscovers = options.maxCommandRediscovers ?? 16;
678680
this.#masterPoolSize = options.masterPoolSize ?? 1;
@@ -716,6 +718,9 @@ class RedisSentinelInternal<
716718

717719
#createClient(node: RedisNode, clientOptions: RedisClientOptions, reconnectStrategy?: undefined | false) {
718720
return RedisClient.create({
721+
//first take the globally set RESP
722+
RESP: this.#RESP,
723+
//then take the client options, which can in theory overwrite it
719724
...clientOptions,
720725
socket: {
721726
...clientOptions.socket,

packages/test-utils/lib/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ export default class TestUtils {
337337
port: promise.port
338338
}));
339339

340+
340341
const sentinel = createSentinel({
341342
name: 'mymaster',
342343
sentinelRootNodes: rootNodes,
@@ -352,6 +353,7 @@ export default class TestUtils {
352353
functions: options?.functions || {},
353354
masterPoolSize: options?.masterPoolSize || undefined,
354355
reserveClient: options?.reserveClient || false,
356+
...options?.sentinelOptions
355357
}) as RedisSentinelType<M, F, S, RESP, TYPE_MAPPING>;
356358

357359
if (options.disableClientSetup) {

0 commit comments

Comments
 (0)