1
0
mirror of https://github.com/redis/node-redis.git synced 2025-08-06 02:15:48 +03:00

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
This commit is contained in:
Nikolay Karadzhov
2025-07-07 11:06:34 +03:00
committed by GitHub
parent 6f3380ba68
commit 79749f2461
3 changed files with 22 additions and 8 deletions

View File

@@ -23,7 +23,7 @@ describe('RedisSentinel', () => {
{ host: 'localhost', port: 26379 } { host: 'localhost', port: 26379 }
] ]
}; };
it('should throw error when clientSideCache is enabled with RESP 2', () => { it('should throw error when clientSideCache is enabled with RESP 2', () => {
assert.throws( assert.throws(
() => RedisSentinel.create({ () => RedisSentinel.create({
@@ -46,7 +46,7 @@ describe('RedisSentinel', () => {
}); });
it('should not throw when clientSideCache is enabled with RESP 3', () => { it('should not throw when clientSideCache is enabled with RESP 3', () => {
assert.doesNotThrow(() => assert.doesNotThrow(() =>
RedisSentinel.create({ RedisSentinel.create({
...options, ...options,
clientSideCache: clientSideCacheConfig, clientSideCache: clientSideCacheConfig,
@@ -54,6 +54,16 @@ describe('RedisSentinel', () => {
}) })
); );
}); });
testUtils.testWithClientSentinel('should successfully connect to sentinel', async () => {
}, {
...GLOBAL.SENTINEL.OPEN,
sentinelOptions: {
RESP: 3,
clientSideCache: { ttl: 0, maxEntries: 0, evictPolicy: 'LRU'},
},
})
}); });
}); });
}); });
@@ -417,7 +427,7 @@ async function steadyState(frame: SentinelFramework) {
sentinel.setTracer(tracer); sentinel.setTracer(tracer);
await sentinel.connect(); await sentinel.connect();
await nodePromise; await nodePromise;
await sentinel.flushAll(); await sentinel.flushAll();
} finally { } finally {
if (sentinel !== undefined) { if (sentinel !== undefined) {
@@ -443,7 +453,7 @@ describe('legacy tests', () => {
this.timeout(15000); this.timeout(15000);
last = Date.now(); last = Date.now();
function deltaMeasurer() { function deltaMeasurer() {
const delta = Date.now() - last; const delta = Date.now() - last;
if (delta > longestDelta) { if (delta > longestDelta) {
@@ -508,7 +518,7 @@ describe('legacy tests', () => {
} }
stopMeasuringBlocking = true; stopMeasuringBlocking = true;
await frame.cleanup(); await frame.cleanup();
}) })
@@ -1032,6 +1042,3 @@ describe('legacy tests', () => {
}) })
}); });
}); });

View File

@@ -625,6 +625,7 @@ class RedisSentinelInternal<
readonly #sentinelClientOptions: RedisClientOptions<typeof RedisSentinelModule, RedisFunctions, RedisScripts, RespVersions, TypeMapping, RedisTcpSocketOptions>; readonly #sentinelClientOptions: RedisClientOptions<typeof RedisSentinelModule, RedisFunctions, RedisScripts, RespVersions, TypeMapping, RedisTcpSocketOptions>;
readonly #scanInterval: number; readonly #scanInterval: number;
readonly #passthroughClientErrorEvents: boolean; readonly #passthroughClientErrorEvents: boolean;
readonly #RESP?: RespVersions;
#anotherReset = false; #anotherReset = false;
@@ -673,6 +674,7 @@ class RedisSentinelInternal<
this.#name = options.name; this.#name = options.name;
this.#RESP = options.RESP;
this.#sentinelRootNodes = Array.from(options.sentinelRootNodes); this.#sentinelRootNodes = Array.from(options.sentinelRootNodes);
this.#maxCommandRediscovers = options.maxCommandRediscovers ?? 16; this.#maxCommandRediscovers = options.maxCommandRediscovers ?? 16;
this.#masterPoolSize = options.masterPoolSize ?? 1; this.#masterPoolSize = options.masterPoolSize ?? 1;
@@ -716,6 +718,9 @@ class RedisSentinelInternal<
#createClient(node: RedisNode, clientOptions: RedisClientOptions, reconnectStrategy?: undefined | false) { #createClient(node: RedisNode, clientOptions: RedisClientOptions, reconnectStrategy?: undefined | false) {
return RedisClient.create({ return RedisClient.create({
//first take the globally set RESP
RESP: this.#RESP,
//then take the client options, which can in theory overwrite it
...clientOptions, ...clientOptions,
socket: { socket: {
...clientOptions.socket, ...clientOptions.socket,

View File

@@ -337,6 +337,7 @@ export default class TestUtils {
port: promise.port port: promise.port
})); }));
const sentinel = createSentinel({ const sentinel = createSentinel({
name: 'mymaster', name: 'mymaster',
sentinelRootNodes: rootNodes, sentinelRootNodes: rootNodes,
@@ -352,6 +353,7 @@ export default class TestUtils {
functions: options?.functions || {}, functions: options?.functions || {},
masterPoolSize: options?.masterPoolSize || undefined, masterPoolSize: options?.masterPoolSize || undefined,
reserveClient: options?.reserveClient || false, reserveClient: options?.reserveClient || false,
...options?.sentinelOptions
}) as RedisSentinelType<M, F, S, RESP, TYPE_MAPPING>; }) as RedisSentinelType<M, F, S, RESP, TYPE_MAPPING>;
if (options.disableClientSetup) { if (options.disableClientSetup) {