diff --git a/packages/client/lib/tests/test-scenario/configuration.e2e.ts b/packages/client/lib/tests/test-scenario/configuration.e2e.ts index a648375f6e..a352a8f10e 100644 --- a/packages/client/lib/tests/test-scenario/configuration.e2e.ts +++ b/packages/client/lib/tests/test-scenario/configuration.e2e.ts @@ -11,7 +11,7 @@ import { } from "./test-scenario.util"; import { createClient } from "../../.."; import { FaultInjectorClient } from "./fault-injector-client"; -import { MovingEndpointType } from "../../../dist/lib/client/enterprise-maintenance-manager"; +import { MovingEndpointType } from "../../../lib/client/enterprise-maintenance-manager"; import { RedisTcpSocketOptions } from "../../client/socket"; describe("Client Configuration and Handshake", () => { @@ -59,7 +59,7 @@ describe("Client Configuration and Handshake", () => { it(`clientHandshakeWithEndpointType '${endpointType}'`, async () => { try { client = await createTestClient(clientConfig, { - maintMovingEndpointType: endpointType, + maintEndpointType: endpointType }); client.on("error", () => {}); @@ -154,7 +154,7 @@ describe("Client Configuration and Handshake", () => { describe("Feature Enablement", () => { it("connectionHandshakeIncludesEnablingNotifications", async () => { client = await createTestClient(clientConfig, { - maintPushNotifications: "enabled", + maintNotifications: "enabled" }); const { action_id } = await faultInjectorClient.migrateAndBindAction({ @@ -180,7 +180,7 @@ describe("Client Configuration and Handshake", () => { it("disabledDontReceiveNotifications", async () => { try { client = await createTestClient(clientConfig, { - maintPushNotifications: "disabled", + maintNotifications: "disabled", socket: { reconnectStrategy: false } diff --git a/packages/client/lib/tests/test-scenario/connection-handoff.e2e.ts b/packages/client/lib/tests/test-scenario/connection-handoff.e2e.ts index 3fbf5e38d4..7a9a4c24df 100644 --- a/packages/client/lib/tests/test-scenario/connection-handoff.e2e.ts +++ b/packages/client/lib/tests/test-scenario/connection-handoff.e2e.ts @@ -86,25 +86,25 @@ describe("Connection Handoff", () => { { name: "external-ip", clientOptions: { - maintMovingEndpointType: "external-ip", + maintEndpointType: "external-ip", }, }, { name: "external-fqdn", clientOptions: { - maintMovingEndpointType: "external-fqdn", + maintEndpointType: "external-fqdn", }, }, { name: "auto", clientOptions: { - maintMovingEndpointType: "auto", + maintEndpointType: "auto", }, }, { name: "none", clientOptions: { - maintMovingEndpointType: "none", + maintEndpointType: "none", }, }, ]; @@ -156,6 +156,7 @@ describe("Connection Handoff", () => { describe("Connection Cleanup", () => { it("should shut down old connection", async () => { + client = await createTestClient(clientConfig); const spyObject = spyOnTemporaryClientInstanceMethod(client, "destroy"); const { action_id: lowTimeoutBindAndMigrateActionId } = diff --git a/packages/client/lib/tests/test-scenario/fault-injector-client.ts b/packages/client/lib/tests/test-scenario/fault-injector-client.ts index 13c81412b1..c03fa1afa1 100644 --- a/packages/client/lib/tests/test-scenario/fault-injector-client.ts +++ b/packages/client/lib/tests/test-scenario/fault-injector-client.ts @@ -54,6 +54,20 @@ export class FaultInjectorClient { return this.#request("POST", "/action", action); } + // public async printStatus() { + // const action = { + // type: 'execute_rladmin_command', + // parameters: { + // rladmin_command: "status", + // bdb_id: "1" + // } + // } + // const { action_id } = await this.#request<{action_id: string}>("POST", "/action", action); + // const status = await this.waitForAction(action_id); + // //@ts-ignore + // console.log(status.output.output); + // } + /** * Gets the status of a specific action. * @param actionId The ID of the action to check @@ -87,7 +101,13 @@ export class FaultInjectorClient { while (Date.now() - startTime < maxWaitTime) { const action = await this.getActionStatus(actionId); - if (["finished", "failed", "success"].includes(action.status)) { + if (action.status === "failed") { + throw new Error( + `Action id: ${actionId} failed! Error: ${action.error}` + ); + } + + if (["finished", "success"].includes(action.status)) { return action; } @@ -118,6 +138,7 @@ export class FaultInjectorClient { type: "migrate", params: { cluster_index: clusterIndexStr, + bdb_id: bdbIdStr, }, }, { diff --git a/packages/client/lib/tests/test-scenario/negative-tests.e2e.ts b/packages/client/lib/tests/test-scenario/negative-tests.e2e.ts index 9e90b80c50..5155877701 100644 --- a/packages/client/lib/tests/test-scenario/negative-tests.e2e.ts +++ b/packages/client/lib/tests/test-scenario/negative-tests.e2e.ts @@ -7,7 +7,7 @@ describe("Negative tests", () => { () => createClient({ RESP: 2, - maintPushNotifications: "enabled", + maintNotifications: "enabled", }), "Error: Graceful Maintenance is only supported with RESP3", ); diff --git a/packages/client/lib/tests/test-scenario/pn-failover.e2e.ts b/packages/client/lib/tests/test-scenario/pn-failover.e2e.ts new file mode 100644 index 0000000000..7b977f33a2 --- /dev/null +++ b/packages/client/lib/tests/test-scenario/pn-failover.e2e.ts @@ -0,0 +1,226 @@ +import assert from "node:assert"; +import diagnostics_channel from "node:diagnostics_channel"; +import { FaultInjectorClient } from "./fault-injector-client"; +import { + createTestClient, + getDatabaseConfig, + getDatabaseConfigFromEnv, + getEnvConfig, + RedisConnectionConfig, +} from "./test-scenario.util"; +import { createClient } from "../../.."; +import { DiagnosticsEvent } from "../../client/enterprise-maintenance-manager"; +import { before } from "mocha"; + +describe("Push Notifications", () => { + const createNotificationMessageHandler = ( + result: Record, + notifications: Array + ) => { + return (message: unknown) => { + if (notifications.includes((message as DiagnosticsEvent).type)) { + const event = message as DiagnosticsEvent; + result[event.type] = (result[event.type] ?? 0) + 1; + } + }; + }; + + let onMessageHandler: ReturnType; + let clientConfig: RedisConnectionConfig; + let client: ReturnType>; + let faultInjectorClient: FaultInjectorClient; + + before(() => { + const envConfig = getEnvConfig(); + const redisConfig = getDatabaseConfigFromEnv( + envConfig.redisEndpointsConfigPath + ); + + faultInjectorClient = new FaultInjectorClient(envConfig.faultInjectorUrl); + clientConfig = getDatabaseConfig(redisConfig); + }); + + afterEach(() => { + if (onMessageHandler!) { + diagnostics_channel.unsubscribe("redis.maintenance", onMessageHandler); + } + + if (client && client.isOpen) { + client.destroy(); + } + }); + + describe("Push Notifications Enabled", () => { + beforeEach(async () => { + client = await createTestClient(clientConfig); + + await client.flushAll(); + }); + + it("should receive FAILING_OVER and FAILED_OVER push notifications", async () => { + const notifications: Array = [ + "FAILING_OVER", + "FAILED_OVER", + ]; + + const diagnosticsMap: Record = {}; + + onMessageHandler = createNotificationMessageHandler( + diagnosticsMap, + notifications + ); + + diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); + + const { action_id: failoverActionId } = + await faultInjectorClient.triggerAction({ + type: "failover", + parameters: { + bdb_id: clientConfig.bdbId.toString(), + cluster_index: 0, + }, + }); + + await faultInjectorClient.waitForAction(failoverActionId); + + assert.strictEqual( + diagnosticsMap.FAILING_OVER, + 1, + "Should have received exactly one FAILING_OVER notification" + ); + assert.strictEqual( + diagnosticsMap.FAILED_OVER, + 1, + "Should have received exactly one FAILED_OVER notification" + ); + }); + }); + + describe("Push Notifications Disabled - Client", () => { + beforeEach(async () => { + client = await createTestClient(clientConfig, { + maintNotifications: "disabled", + }); + + client.on("error", (_err) => { + // Expect the socket to be closed + // Ignore errors + }); + + await client.flushAll(); + }); + + it("should NOT receive FAILING_OVER and FAILED_OVER push notifications", async () => { + const notifications: Array = [ + "FAILING_OVER", + "FAILED_OVER", + ]; + + const diagnosticsMap: Record = {}; + + onMessageHandler = createNotificationMessageHandler( + diagnosticsMap, + notifications + ); + + diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); + + const { action_id: failoverActionId } = + await faultInjectorClient.triggerAction({ + type: "failover", + parameters: { + bdb_id: clientConfig.bdbId.toString(), + cluster_index: 0, + }, + }); + + await faultInjectorClient.waitForAction(failoverActionId); + + assert.strictEqual( + diagnosticsMap.FAILING_OVER, + undefined, + "Should have received exactly one FAILING_OVER notification" + ); + assert.strictEqual( + diagnosticsMap.FAILED_OVER, + undefined, + "Should have received exactly one FAILED_OVER notification" + ); + }); + }); + + describe("Push Notifications Disabled - Server", () => { + beforeEach(async () => { + client = await createTestClient(clientConfig); + + client.on("error", (_err) => { + // Expect the socket to be closed + // Ignore errors + }); + + await client.flushAll(); + }); + + before(async () => { + const { action_id: disablePushNotificationsActionId } = + await faultInjectorClient.triggerAction({ + type: "update_cluster_config", + parameters: { + config: { client_maint_notifications: false }, + }, + }); + + await faultInjectorClient.waitForAction(disablePushNotificationsActionId); + }); + + after(async () => { + const { action_id: enablePushNotificationsActionId } = + await faultInjectorClient.triggerAction({ + type: "update_cluster_config", + parameters: { + config: { client_maint_notifications: true }, + }, + }); + + await faultInjectorClient.waitForAction(enablePushNotificationsActionId); + }); + + it("should NOT receive FAILING_OVER and FAILED_OVER push notifications", async () => { + const notifications: Array = [ + "FAILING_OVER", + "FAILED_OVER", + ]; + + const diagnosticsMap: Record = {}; + + onMessageHandler = createNotificationMessageHandler( + diagnosticsMap, + notifications + ); + + diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); + + const { action_id: failoverActionId } = + await faultInjectorClient.triggerAction({ + type: "failover", + parameters: { + bdb_id: clientConfig.bdbId.toString(), + cluster_index: 0, + }, + }); + + await faultInjectorClient.waitForAction(failoverActionId); + + assert.strictEqual( + diagnosticsMap.FAILING_OVER, + undefined, + "Should have received exactly one FAILING_OVER notification" + ); + assert.strictEqual( + diagnosticsMap.FAILED_OVER, + undefined, + "Should have received exactly one FAILED_OVER notification" + ); + }); + }); +}); diff --git a/packages/client/lib/tests/test-scenario/push-notification.e2e.ts b/packages/client/lib/tests/test-scenario/push-notification.e2e.ts index 9962d0a02d..bfaef8351b 100644 --- a/packages/client/lib/tests/test-scenario/push-notification.e2e.ts +++ b/packages/client/lib/tests/test-scenario/push-notification.e2e.ts @@ -98,49 +98,12 @@ describe("Push Notifications", () => { ); }); - it("should receive FAILING_OVER and FAILED_OVER push notifications", async () => { - const notifications: Array = [ - "FAILING_OVER", - "FAILED_OVER", - ]; - - const diagnosticsMap: Record = {}; - - onMessageHandler = createNotificationMessageHandler( - diagnosticsMap, - notifications - ); - - diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); - - const { action_id: failoverActionId } = - await faultInjectorClient.triggerAction({ - type: "failover", - parameters: { - bdb_id: clientConfig.bdbId.toString(), - cluster_index: 0, - }, - }); - - await faultInjectorClient.waitForAction(failoverActionId); - - assert.strictEqual( - diagnosticsMap.FAILING_OVER, - 1, - "Should have received exactly one FAILING_OVER notification" - ); - assert.strictEqual( - diagnosticsMap.FAILED_OVER, - 1, - "Should have received exactly one FAILED_OVER notification" - ); - }); }); describe("Push Notifications Disabled - Client", () => { beforeEach(async () => { client = await createTestClient(clientConfig, { - maintPushNotifications: "disabled", + maintNotifications: "disabled", }); client.on("error", (_err) => { @@ -192,43 +155,6 @@ describe("Push Notifications", () => { ); }); - it("should NOT receive FAILING_OVER and FAILED_OVER push notifications", async () => { - const notifications: Array = [ - "FAILING_OVER", - "FAILED_OVER", - ]; - - const diagnosticsMap: Record = {}; - - onMessageHandler = createNotificationMessageHandler( - diagnosticsMap, - notifications - ); - - diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); - - const { action_id: failoverActionId } = - await faultInjectorClient.triggerAction({ - type: "failover", - parameters: { - bdb_id: clientConfig.bdbId.toString(), - cluster_index: 0, - }, - }); - - await faultInjectorClient.waitForAction(failoverActionId); - - assert.strictEqual( - diagnosticsMap.FAILING_OVER, - undefined, - "Should have received exactly one FAILING_OVER notification" - ); - assert.strictEqual( - diagnosticsMap.FAILED_OVER, - undefined, - "Should have received exactly one FAILED_OVER notification" - ); - }); }); describe("Push Notifications Disabled - Server", () => { @@ -308,42 +234,5 @@ describe("Push Notifications", () => { ); }); - it("should NOT receive FAILING_OVER and FAILED_OVER push notifications", async () => { - const notifications: Array = [ - "FAILING_OVER", - "FAILED_OVER", - ]; - - const diagnosticsMap: Record = {}; - - onMessageHandler = createNotificationMessageHandler( - diagnosticsMap, - notifications - ); - - diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); - - const { action_id: failoverActionId } = - await faultInjectorClient.triggerAction({ - type: "failover", - parameters: { - bdb_id: clientConfig.bdbId.toString(), - cluster_index: 0, - }, - }); - - await faultInjectorClient.waitForAction(failoverActionId); - - assert.strictEqual( - diagnosticsMap.FAILING_OVER, - undefined, - "Should have received exactly one FAILING_OVER notification" - ); - assert.strictEqual( - diagnosticsMap.FAILED_OVER, - undefined, - "Should have received exactly one FAILED_OVER notification" - ); - }); }); }); diff --git a/packages/client/lib/tests/test-scenario/test-scenario.util.ts b/packages/client/lib/tests/test-scenario/test-scenario.util.ts index c98ba90fe1..96df0acbd6 100644 --- a/packages/client/lib/tests/test-scenario/test-scenario.util.ts +++ b/packages/client/lib/tests/test-scenario/test-scenario.util.ts @@ -43,13 +43,13 @@ export function getEnvConfig(): EnvConfig { ); } - if (!process.env.FAULT_INJECTION_API_URL) { - throw new Error("FAULT_INJECTION_API_URL environment variable must be set"); + if (!process.env.RE_FAULT_INJECTOR_URL) { + throw new Error("RE_FAULT_INJECTOR_URL environment variable must be set"); } return { redisEndpointsConfigPath: process.env.REDIS_ENDPOINTS_CONFIG_PATH, - faultInjectorUrl: process.env.FAULT_INJECTION_API_URL, + faultInjectorUrl: process.env.RE_FAULT_INJECTOR_URL, }; } @@ -86,7 +86,7 @@ export interface RedisConnectionConfig { */ export function getDatabaseConfig( databasesConfig: DatabasesConfig, - databaseName?: string + databaseName = process.env.DATABASE_NAME ): RedisConnectionConfig { const dbConfig = databaseName ? databasesConfig[databaseName] @@ -163,8 +163,8 @@ export async function createTestClient( password: clientConfig.password, username: clientConfig.username, RESP: 3, - maintPushNotifications: "auto", - maintMovingEndpointType: "auto", + maintNotifications: "auto", + maintEndpointType: "auto", ...options, }); diff --git a/packages/client/lib/tests/test-scenario/timeout-during-notifications.e2e.ts b/packages/client/lib/tests/test-scenario/timeout-during-notifications.e2e.ts index a60aacb703..30cdd4669c 100644 --- a/packages/client/lib/tests/test-scenario/timeout-during-notifications.e2e.ts +++ b/packages/client/lib/tests/test-scenario/timeout-during-notifications.e2e.ts @@ -146,6 +146,7 @@ describe("Timeout Handling During Notifications", () => { type: "migrate", parameters: { cluster_index: 0, + bdb_id: clientConfig.bdbId.toString(), }, }); @@ -163,7 +164,7 @@ describe("Timeout Handling During Notifications", () => { "Command Timeout error should be instanceof Error" ); assert.ok( - durationMigrate > NORMAL_COMMAND_TIMEOUT && + durationMigrate >= NORMAL_COMMAND_TIMEOUT && durationMigrate < NORMAL_COMMAND_TIMEOUT * 1.1, `Normal command should timeout within normal timeout ms` ); @@ -198,7 +199,7 @@ describe("Timeout Handling During Notifications", () => { "Command Timeout error should be instanceof Error" ); assert.ok( - durationBind > NORMAL_COMMAND_TIMEOUT && + durationBind >= NORMAL_COMMAND_TIMEOUT && durationBind < NORMAL_COMMAND_TIMEOUT * 1.1, `Normal command should timeout within normal timeout ms` ); @@ -208,83 +209,4 @@ describe("Timeout Handling During Notifications", () => { "Command Timeout error should be TimeoutError" ); }); - - it("should relax command timeout on FAILING_OVER", async () => { - const notifications: Array = ["FAILING_OVER"]; - - const result: Record< - DiagnosticsEvent["type"], - { error: any; duration: number } - > = {}; - - const onMessageHandler = createNotificationMessageHandler( - client, - result, - notifications - ); - - diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); - - const { action_id: failoverActionId } = - await faultInjectorClient.triggerAction({ - type: "failover", - parameters: { - bdb_id: clientConfig.bdbId.toString(), - cluster_index: 0, - }, - }); - - await faultInjectorClient.waitForAction(failoverActionId); - - diagnostics_channel.unsubscribe("redis.maintenance", onMessageHandler); - - notifications.forEach((notification) => { - assert.ok( - result[notification]?.error instanceof Error, - `${notification} notification error should be instanceof Error` - ); - assert.ok( - result[notification]?.duration > RELAXED_COMMAND_TIMEOUT && - result[notification]?.duration < RELAXED_COMMAND_TIMEOUT * 1.1, - `${notification} notification should timeout within relaxed timeout` - ); - assert.strictEqual( - result[notification]?.error?.constructor?.name, - "CommandTimeoutDuringMaintenanceError", - `${notification} notification error should be CommandTimeoutDuringMaintenanceError` - ); - }); - }); - - it("should unrelax command timeout after FAILED_OVER", async () => { - const { action_id: failoverActionId } = - await faultInjectorClient.triggerAction({ - type: "failover", - parameters: { - bdb_id: clientConfig.bdbId.toString(), - cluster_index: 0, - }, - }); - - await faultInjectorClient.waitForAction(failoverActionId); - - const { error, duration } = await blockCommand(async () => { - await client.set("key", "value"); - }); - - assert.ok( - error instanceof Error, - "Command Timeout error should be instanceof Error" - ); - assert.ok( - duration > NORMAL_COMMAND_TIMEOUT && - duration < NORMAL_COMMAND_TIMEOUT * 1.1, - `Normal command should timeout within normal timeout ms` - ); - assert.strictEqual( - error?.constructor?.name, - "TimeoutError", - "Command Timeout error should be TimeoutError" - ); - }); }); diff --git a/packages/client/lib/tests/test-scenario/to-failover.e2e.ts b/packages/client/lib/tests/test-scenario/to-failover.e2e.ts new file mode 100644 index 0000000000..506aa6f74b --- /dev/null +++ b/packages/client/lib/tests/test-scenario/to-failover.e2e.ts @@ -0,0 +1,151 @@ +import assert from "node:assert"; + +import { FaultInjectorClient } from "./fault-injector-client"; +import { + getDatabaseConfig, + getDatabaseConfigFromEnv, + getEnvConfig, + RedisConnectionConfig, + blockCommand, + createTestClient, +} from "./test-scenario.util"; +import { createClient } from "../../.."; +import { before } from "mocha"; +import diagnostics_channel from "node:diagnostics_channel"; +import { DiagnosticsEvent } from "../../client/enterprise-maintenance-manager"; + +describe("Timeout Handling During Notifications", () => { + let clientConfig: RedisConnectionConfig; + let faultInjectorClient: FaultInjectorClient; + let client: ReturnType>; + + const NORMAL_COMMAND_TIMEOUT = 50; + const RELAXED_COMMAND_TIMEOUT = 2000; + + /** + * Creates a handler for the `redis.maintenance` channel that will execute and block a command on the client + * when a notification is received and save the result in the `result` object. + * This is used to test that the command timeout is relaxed during notifications. + */ + const createNotificationMessageHandler = ( + client: ReturnType>, + result: Record, + notifications: Array + ) => { + return (message: unknown) => { + if (notifications.includes((message as DiagnosticsEvent).type)) { + setImmediate(async () => { + result[(message as DiagnosticsEvent).type] = await blockCommand( + async () => { + await client.set("key", "value"); + } + ); + }); + } + }; + }; + + before(() => { + const envConfig = getEnvConfig(); + const redisConfig = getDatabaseConfigFromEnv( + envConfig.redisEndpointsConfigPath + ); + + clientConfig = getDatabaseConfig(redisConfig); + faultInjectorClient = new FaultInjectorClient(envConfig.faultInjectorUrl); + }); + + beforeEach(async () => { + client = await createTestClient(clientConfig, { + commandOptions: { timeout: NORMAL_COMMAND_TIMEOUT }, + maintRelaxedCommandTimeout: RELAXED_COMMAND_TIMEOUT, + }); + + await client.flushAll(); + }); + + afterEach(() => { + if (client && client.isOpen) { + client.destroy(); + } + }); + + it("should relax command timeout on FAILING_OVER", async () => { + const notifications: Array = ["FAILING_OVER"]; + + const result: Record< + DiagnosticsEvent["type"], + { error: any; duration: number } + > = {}; + + const onMessageHandler = createNotificationMessageHandler( + client, + result, + notifications + ); + + diagnostics_channel.subscribe("redis.maintenance", onMessageHandler); + + const { action_id: failoverActionId } = + await faultInjectorClient.triggerAction({ + type: "failover", + parameters: { + bdb_id: clientConfig.bdbId.toString(), + cluster_index: 0, + }, + }); + + await faultInjectorClient.waitForAction(failoverActionId); + + diagnostics_channel.unsubscribe("redis.maintenance", onMessageHandler); + + notifications.forEach((notification) => { + assert.ok( + result[notification]?.error instanceof Error, + `${notification} notification error should be instanceof Error` + ); + assert.ok( + result[notification]?.duration > RELAXED_COMMAND_TIMEOUT && + result[notification]?.duration < RELAXED_COMMAND_TIMEOUT * 1.1, + `${notification} notification should timeout within relaxed timeout` + ); + assert.strictEqual( + result[notification]?.error?.constructor?.name, + "CommandTimeoutDuringMaintenanceError", + `${notification} notification error should be CommandTimeoutDuringMaintenanceError` + ); + }); + }); + + it("should unrelax command timeout after FAILED_OVER", async () => { + const { action_id: failoverActionId } = + await faultInjectorClient.triggerAction({ + type: "failover", + parameters: { + bdb_id: clientConfig.bdbId.toString(), + cluster_index: 0, + }, + }); + + await faultInjectorClient.waitForAction(failoverActionId); + + const { error, duration } = await blockCommand(async () => { + await client.set("key", "value"); + }); + + assert.ok( + error instanceof Error, + "Command Timeout error should be instanceof Error" + ); + assert.ok( + duration > NORMAL_COMMAND_TIMEOUT && + duration < NORMAL_COMMAND_TIMEOUT * 1.1, + `Normal command should timeout within normal timeout ms` + ); + assert.strictEqual( + error?.constructor?.name, + "TimeoutError", + "Command Timeout error should be TimeoutError" + ); + }); +});