Skip to content

[CAE-342] Initial sentinel issues fix #2912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/client/lib/client/index.ts
Original file line number Diff line number Diff line change
@@ -299,6 +299,9 @@ export default class RedisClient<
#monitorCallback?: MonitorCallback<TYPE_MAPPING>;
private _self = this;
private _commandOptions?: CommandOptions<TYPE_MAPPING>;
// flag used to annotate that the client
// was in a watch transaction when
// a topology change occured
#dirtyWatch?: string;
#epoch: number;
#watchEpoch?: number;
@@ -325,6 +328,10 @@ export default class RedisClient<
return this._self.#watchEpoch !== undefined;
}

get isDirtyWatch() {
return this._self.#dirtyWatch !== undefined
}

setDirtyWatch(msg: string) {
this._self.#dirtyWatch = msg;
}
15 changes: 10 additions & 5 deletions packages/client/lib/sentinel/index.spec.ts
Original file line number Diff line number Diff line change
@@ -78,7 +78,7 @@ async function steadyState(frame: SentinelFramework) {
}

["redis-sentinel-test-password", undefined].forEach(function (password) {
describe.skip(`Sentinel - password = ${password}`, () => {
describe(`Sentinel - password = ${password}`, () => {
const config: RedisSentinelConfig = { sentinelName: "test", numberOfNodes: 3, password: password };
const frame = new SentinelFramework(config);
let tracer = new Array<string>();
@@ -197,6 +197,11 @@ async function steadyState(frame: SentinelFramework) {

await assert.doesNotReject(sentinel.get('x'));
});

it('failed to connect', async function() {
sentinel = frame.getSentinelClient({sentinelRootNodes: [{host: "127.0.0.1", port: 1010}], maxCommandRediscovers: 0})
await assert.rejects(sentinel.connect());
});

it('try to connect multiple times', async function () {
sentinel = frame.getSentinelClient();
@@ -436,7 +441,7 @@ async function steadyState(frame: SentinelFramework) {
assert.equal(await promise, null);
});

it('reserve client, takes a client out of pool', async function () {
it.skip('reserve client, takes a client out of pool', async function () {
this.timeout(30000);

sentinel = frame.getSentinelClient({ masterPoolSize: 2, reserveClient: true });
@@ -481,7 +486,7 @@ async function steadyState(frame: SentinelFramework) {
});

// by taking a lease, we know we will block on master as no clients are available, but as read occuring, means replica read occurs
it('replica reads', async function () {
it.skip('replica reads', async function () {
this.timeout(30000);

sentinel = frame.getSentinelClient({ replicaPoolSize: 1 });
@@ -718,7 +723,7 @@ async function steadyState(frame: SentinelFramework) {
tracer.push("multi was rejected");
});

it('plain pubsub - channel', async function () {
it.skip('plain pubsub - channel', async function () {
this.timeout(30000);

sentinel = frame.getSentinelClient();
@@ -757,7 +762,7 @@ async function steadyState(frame: SentinelFramework) {
assert.equal(tester, false);
});

it('plain pubsub - pattern', async function () {
it.skip('plain pubsub - pattern', async function () {
this.timeout(30000);

sentinel = frame.getSentinelClient();
5 changes: 3 additions & 2 deletions packages/client/lib/sentinel/index.ts
Original file line number Diff line number Diff line change
@@ -682,9 +682,10 @@ class RedisSentinelInternal<

async #connect() {
let count = 0;
while (true) {
while (true) {
this.#trace("starting connect loop");

count+=1;
if (this.#destroy) {
this.#trace("in #connect and want to destroy")
return;
@@ -1109,7 +1110,7 @@ class RedisSentinelInternal<

this.#trace(`transform: destroying old masters if open`);
for (const client of this.#masterClients) {
masterWatches.push(client.isWatching);
masterWatches.push(client.isWatching || client.isDirtyWatch);

if (client.isOpen) {
client.destroy()
25 changes: 12 additions & 13 deletions packages/client/lib/sentinel/test-util.ts
Original file line number Diff line number Diff line change
@@ -55,7 +55,7 @@ export interface RedisServerDocker {
abstract class DockerBase {
async spawnRedisServerDocker({ image, version }: RedisServerDockerConfig, serverArguments: Array<string>, environment?: string): Promise<RedisServerDocker> {
const port = (await portIterator.next()).value;
let cmdLine = `docker run --init -d --network host `;
let cmdLine = `docker run --init -d --network host -e PORT=${port.toString()} `;
if (environment !== undefined) {
cmdLine += `-e ${environment} `;
}
@@ -180,9 +180,14 @@ export class SentinelFramework extends DockerBase {
RedisScripts,
RespVersions,
TypeMapping>>, errors = true) {
if (opts?.sentinelRootNodes !== undefined) {
throw new Error("cannot specify sentinelRootNodes here");
}
// remove this safeguard
// in order to test the case when
// connecting to sentinel fails

// if (opts?.sentinelRootNodes !== undefined) {
// throw new Error("cannot specify sentinelRootNodes here");
// }

if (opts?.name !== undefined) {
throw new Error("cannot specify sentinel db name here");
}
@@ -245,13 +250,13 @@ export class SentinelFramework extends DockerBase {
}

protected async spawnRedisSentinelNodeDocker() {
const imageInfo: RedisServerDockerConfig = this.config.nodeDockerConfig ?? { image: "redis/redis-stack-server", version: "latest" };
const imageInfo: RedisServerDockerConfig = this.config.nodeDockerConfig ?? { image: "redislabs/client-libs-test", version: "8.0-M05-pre" };
const serverArguments: Array<string> = this.config.nodeServerArguments ?? [];
let environment;
if (this.config.password !== undefined) {
environment = `REDIS_ARGS="{port} --requirepass ${this.config.password}"`;
environment = `REDIS_PASSWORD=${this.config.password}`;
} else {
environment = 'REDIS_ARGS="{port}"';
environment = undefined;
}

const docker = await this.spawnRedisServerDocker(imageInfo, serverArguments, environment);
@@ -276,9 +281,6 @@ export class SentinelFramework extends DockerBase {
for (let i = 0; i < (this.config.numberOfNodes ?? 0) - 1; i++) {
promises.push(
this.spawnRedisSentinelNodeDocker().then(async node => {
if (this.config.password !== undefined) {
await node.client.configSet({'masterauth': this.config.password})
}
await node.client.replicaOf('127.0.0.1', master.docker.port);
return node;
})
@@ -401,9 +403,6 @@ export class SentinelFramework extends DockerBase {
const masterPort = await this.getMasterPort();
const newNode = await this.spawnRedisSentinelNodeDocker();

if (this.config.password !== undefined) {
await newNode.client.configSet({'masterauth': this.config.password})
}
await newNode.client.replicaOf('127.0.0.1', masterPort);

this.#nodeList.push(newNode);
Loading
Oops, something went wrong.