Skip to content

Commit 4de9ae4

Browse files
mo-amznmo-amzn
authored
Fix Node AzAffinity Flaky Test (valkey-io#3107)
-------------------------------------------------------------- Signed-off-by: Muhammad Awawdi <mawawdi@amazon.com>
1 parent 9e58738 commit 4de9ae4

File tree

1 file changed

+17
-69
lines changed

1 file changed

+17
-69
lines changed

‎node/tests/GlideClusterClient.test.ts‎

Lines changed: 17 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,50 +2170,13 @@ describe("GlideClusterClient", () => {
21702170
);
21712171

21722172
describe("AZAffinity Read Strategy Tests", () => {
2173-
async function getNumberOfReplicas(
2174-
azClient: GlideClusterClient,
2175-
): Promise<number> {
2176-
const replicationInfo = (await azClient.info({
2177-
sections: [InfoOptions.Replication],
2178-
})) as Record<string, string>;
2179-
let totalReplicas = 0;
2180-
Object.values(replicationInfo).forEach((nodeInfo) => {
2181-
const lines = nodeInfo.split(/\r?\n/);
2182-
const connectedReplicasLine = lines.find(
2183-
(line) =>
2184-
line.startsWith("connected_slaves:") ||
2185-
line.startsWith("connected_replicas:"),
2186-
);
2187-
2188-
if (connectedReplicasLine) {
2189-
const parts = connectedReplicasLine.split(":");
2190-
const numReplicas = parseInt(parts[1], 10);
2191-
2192-
if (!isNaN(numReplicas)) {
2193-
// Sum up replicas from each primary node
2194-
totalReplicas += numReplicas;
2195-
}
2196-
}
2197-
});
2198-
2199-
if (totalReplicas > 0) {
2200-
return totalReplicas;
2201-
}
2202-
2203-
throw new Error(
2204-
"Could not find replica information in any node's response",
2205-
);
2206-
}
2207-
22082173
it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
22092174
"should route GET commands to all replicas with the same AZ using protocol %p",
22102175
async (protocol) => {
22112176
// Skip test if version is below 8.0.0
22122177
if (cluster.checkIfServerVersionLessThan("8.0.0")) return;
22132178

22142179
const az = "us-east-1a";
2215-
const GET_CALLS_PER_REPLICA = 3;
2216-
22172180
let client_for_config_set;
22182181
let client_for_testing_az;
22192182

@@ -2224,7 +2187,6 @@ describe("GlideClusterClient", () => {
22242187
getClientConfigurationOption(
22252188
azCluster.getAddresses(),
22262189
protocol,
2227-
{ requestTimeout: 3000 },
22282190
),
22292191
);
22302192

@@ -2234,28 +2196,13 @@ describe("GlideClusterClient", () => {
22342196
{ route: "allNodes" },
22352197
);
22362198

2237-
// Retrieve the number of replicas dynamically
2238-
const n_replicas = await getNumberOfReplicas(
2239-
client_for_config_set,
2240-
);
2241-
2242-
if (n_replicas === 0) {
2243-
throw new Error(
2244-
"No replicas found in the cluster. Test requires at least one replica.",
2245-
);
2246-
}
2247-
2248-
const GET_CALLS = GET_CALLS_PER_REPLICA * n_replicas;
2249-
const get_cmdstat = `calls=${GET_CALLS_PER_REPLICA}`;
2250-
22512199
// Stage 2: Create AZ affinity client and verify configuration
22522200
client_for_testing_az =
22532201
await GlideClusterClient.createClient(
22542202
getClientConfigurationOption(
22552203
azCluster.getAddresses(),
22562204
protocol,
22572205
{
2258-
requestTimeout: 3000,
22592206
readFrom: "AZAffinity",
22602207
clientAz: az,
22612208
},
@@ -2273,17 +2220,22 @@ describe("GlideClusterClient", () => {
22732220
),
22742221
);
22752222

2223+
const get_calls_per_replica = 25;
2224+
const get_calls = 100;
2225+
const get_cmdstat = `cmdstat_get:calls=${get_calls_per_replica}`;
2226+
22762227
// Stage 3: Set test data and perform GET operations
22772228
await client_for_testing_az.set("foo", "testvalue");
22782229

2279-
for (let i = 0; i < GET_CALLS; i++) {
2230+
for (let i = 0; i < get_calls; i++) {
22802231
await client_for_testing_az.get("foo");
22812232
}
22822233

22832234
// Stage 4: Verify GET commands were routed correctly
2284-
const info_result = (await client_for_testing_az.info(
2285-
{ sections: [InfoOptions.All], route: "allNodes" }, // Get both replication and commandstats info
2286-
)) as Record<string, string>;
2235+
const info_result = (await client_for_testing_az.info({
2236+
sections: [InfoOptions.All],
2237+
route: "allNodes",
2238+
})) as Record<string, string>;
22872239

22882240
const matching_entries_count = Object.values(
22892241
info_result,
@@ -2296,7 +2248,7 @@ describe("GlideClusterClient", () => {
22962248
return isReplicaNode && infoStr.includes(get_cmdstat);
22972249
}).length;
22982250

2299-
expect(matching_entries_count).toBe(n_replicas); // Should expect 12 as the cluster was created with 3 primary and 4 replicas, totalling 12 replica nodes
2251+
expect(matching_entries_count).toBe(4);
23002252
} finally {
23012253
// Cleanup
23022254
await client_for_config_set?.configSet(
@@ -2316,8 +2268,8 @@ describe("GlideClusterClient", () => {
23162268
if (cluster.checkIfServerVersionLessThan("8.0.0")) return;
23172269

23182270
const az = "us-east-1a";
2319-
const GET_CALLS = 3;
2320-
const get_cmdstat = `calls=${GET_CALLS}`;
2271+
const get_calls = 3;
2272+
const get_cmdstat = `cmdstat_get:calls=${get_calls}`;
23212273
let client_for_config_set;
23222274
let client_for_testing_az;
23232275

@@ -2328,7 +2280,6 @@ describe("GlideClusterClient", () => {
23282280
getClientConfigurationOption(
23292281
azCluster.getAddresses(),
23302282
protocol,
2331-
{ requestTimeout: 3000 },
23322283
),
23332284
);
23342285

@@ -2341,7 +2292,7 @@ describe("GlideClusterClient", () => {
23412292

23422293
await client_for_config_set.configSet(
23432294
{ "availability-zone": az },
2344-
{ route: { type: "replicaSlotId", id: 12182 } },
2295+
{ route: { type: "replicaSlotKey", key: "foo" } },
23452296
);
23462297

23472298
// Stage 2: Create AZ affinity client and verify configuration
@@ -2351,15 +2302,13 @@ describe("GlideClusterClient", () => {
23512302
azCluster.getAddresses(),
23522303
protocol,
23532304
{
2354-
requestTimeout: 3000,
23552305
readFrom: "AZAffinity",
23562306
clientAz: az,
23572307
},
23582308
),
23592309
);
2360-
await client_for_testing_az.set("foo", "testvalue");
23612310

2362-
for (let i = 0; i < GET_CALLS; i++) {
2311+
for (let i = 0; i < get_calls; i++) {
23632312
await client_for_testing_az.get("foo");
23642313
}
23652314

@@ -2406,7 +2355,7 @@ describe("GlideClusterClient", () => {
24062355
// Skip test if version is below 8.0.0
24072356
if (cluster.checkIfServerVersionLessThan("8.0.0")) return;
24082357

2409-
const GET_CALLS = 4;
2358+
const get_calls = 4;
24102359
const replica_calls = 1;
24112360
const get_cmdstat = `cmdstat_get:calls=${replica_calls}`;
24122361
let client_for_testing_az;
@@ -2421,7 +2370,6 @@ describe("GlideClusterClient", () => {
24212370
{
24222371
readFrom: "AZAffinity",
24232372
clientAz: "non-existing-az",
2424-
requestTimeout: 3000,
24252373
},
24262374
),
24272375
);
@@ -2432,7 +2380,7 @@ describe("GlideClusterClient", () => {
24322380
});
24332381

24342382
// Issue GET commands
2435-
for (let i = 0; i < GET_CALLS; i++) {
2383+
for (let i = 0; i < get_calls; i++) {
24362384
await client_for_testing_az.get("foo");
24372385
}
24382386

@@ -2449,7 +2397,7 @@ describe("GlideClusterClient", () => {
24492397
return nodeResponses.includes(get_cmdstat);
24502398
}).length;
24512399

2452-
// Validate that only one replica handled the GET calls
2400+
// Validate that the get calls were distributed across replicas, each replica recieved 1 get call
24532401
expect(matchingEntriesCount).toBe(4);
24542402
} finally {
24552403
// Cleanup: Close the client after test execution

0 commit comments

Comments
 (0)