Skip to content

Commit 1bc8ca0

Browse files
ShogunPandaAVVS
authored andcommitted
fix: Fix autopipeline and downgrade p-map to support Node 6. [#1216]
1 parent 6feee28 commit 1bc8ca0

File tree

5 files changed

+81
-20
lines changed

5 files changed

+81
-20
lines changed

Diff for: .travis.yml

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
language: node_js
22

33
node_js:
4+
- "6"
45
- "8"
5-
- "9"
66
- "10"
7-
- "11"
87
- "12"
8+
- "14"
9+
- "15"
910

1011
services:
1112
- redis-server

Diff for: lib/autoPipelining.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export const notAllowedAutoPipelineCommands = [
1717
"unpsubscribe",
1818
];
1919

20-
function findAutoPipeline(client, ...args: Array<string>): string {
20+
function findAutoPipeline(client, _commandName, ...args: Array<string>): string {
2121
if (!client.isCluster) {
2222
return "main";
2323
}

Diff for: package-lock.json

+8-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
"denque": "^1.1.0",
3939
"lodash.defaults": "^4.2.0",
4040
"lodash.flatten": "^4.4.0",
41-
"p-map": "^4.0.0",
41+
"p-map": "^2.1.0",
4242
"redis-commands": "1.6.0",
4343
"redis-errors": "^1.2.0",
4444
"redis-parser": "^3.0.0",

Diff for: test/functional/cluster/autopipelining.ts

+68-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { expect, use } from "chai";
2+
import * as calculateKeySlot from 'cluster-key-slot';
3+
24
import { default as Cluster } from "../../../lib/cluster";
35
import MockServer from "../../helpers/mock_server";
46

@@ -395,14 +397,29 @@ describe("autoPipelining for cluster", function () {
395397
await new Promise((resolve) => cluster.once("connect", resolve));
396398

397399
const promise1 = cluster.set("foo1", "bar");
398-
const promise2 = cluster.set("foo2", "bar");
400+
const promise2 = cluster.set("foo5", "bar");
401+
const promise3 = cluster.set("foo2", "bar");
402+
const promise4 = cluster.set("foo6", "bar");
403+
404+
// Override slots to induce a failure
405+
const key1Slot = calculateKeySlot('foo1');
406+
const key2Slot = calculateKeySlot('foo2');
407+
const key5Slot = calculateKeySlot('foo5');
408+
cluster.slots[key1Slot] = cluster.slots[key2Slot];
409+
cluster.slots[key2Slot] = cluster.slots[key5Slot];
399410

400411
await expect(promise1).to.eventually.be.rejectedWith(
401412
"All keys in the pipeline should belong to the same slots allocation group"
402413
);
403414
await expect(promise2).to.eventually.be.rejectedWith(
404415
"All keys in the pipeline should belong to the same slots allocation group"
405416
);
417+
await expect(promise3).to.eventually.be.rejectedWith(
418+
"All keys in the pipeline should belong to the same slots allocation group"
419+
);
420+
await expect(promise4).to.eventually.be.rejectedWith(
421+
"All keys in the pipeline should belong to the same slots allocation group"
422+
);
406423

407424
cluster.disconnect();
408425
});
@@ -411,7 +428,7 @@ describe("autoPipelining for cluster", function () {
411428
const cluster = new Cluster(hosts, { enableAutoPipelining: true });
412429

413430
cluster.once("connect", () => {
414-
let err1, err2;
431+
let err1, err2, err3, err4;
415432

416433
function cb() {
417434
expect(err1.message).to.eql(
@@ -420,6 +437,12 @@ describe("autoPipelining for cluster", function () {
420437
expect(err2.message).to.eql(
421438
"All keys in the pipeline should belong to the same slots allocation group"
422439
);
440+
expect(err3.message).to.eql(
441+
"All keys in the pipeline should belong to the same slots allocation group"
442+
);
443+
expect(err4.message).to.eql(
444+
"All keys in the pipeline should belong to the same slots allocation group"
445+
);
423446
expect(cluster.autoPipelineQueueSize).to.eql(0);
424447

425448
cluster.disconnect();
@@ -431,22 +454,49 @@ describe("autoPipelining for cluster", function () {
431454
cluster.set("foo1", "bar1", (err) => {
432455
err1 = err;
433456

434-
if (err1 && err2) {
457+
if (err1 && err2 && err3 && err4) {
435458
cb();
436459
}
437460
});
438461

439462
expect(cluster.autoPipelineQueueSize).to.eql(1);
440463

441-
cluster.set("foo2", (err) => {
464+
cluster.set("foo2", "bar2", (err) => {
442465
err2 = err;
443466

444-
if (err1 && err2) {
467+
if (err1 && err2 && err3 && err4) {
445468
cb();
446469
}
447470
});
448471

449472
expect(cluster.autoPipelineQueueSize).to.eql(2);
473+
474+
cluster.set("foo5", "bar5", (err) => {
475+
err3 = err;
476+
477+
if (err1 && err2 && err3 && err4) {
478+
cb();
479+
}
480+
});
481+
482+
expect(cluster.autoPipelineQueueSize).to.eql(3);
483+
484+
cluster.set("foo6", "bar6", (err) => {
485+
err4 = err;
486+
487+
if (err1 && err2 && err3 && err4) {
488+
cb();
489+
}
490+
});
491+
492+
expect(cluster.autoPipelineQueueSize).to.eql(4);
493+
494+
// Override slots to induce a failure
495+
const key1Slot = calculateKeySlot('foo1');
496+
const key2Slot = calculateKeySlot('foo2');
497+
const key5Slot = calculateKeySlot('foo5');
498+
cluster.slots[key1Slot] = cluster.slots[key2Slot];
499+
cluster.slots[key2Slot] = cluster.slots[key5Slot];
450500
});
451501
});
452502

@@ -457,13 +507,16 @@ describe("autoPipelining for cluster", function () {
457507
process.removeAllListeners("uncaughtException");
458508

459509
cluster.once("connect", () => {
460-
let err1;
510+
let err1, err5;
461511

462512
process.once("uncaughtException", (err) => {
463513
expect(err.message).to.eql("ERROR");
464514
expect(err1.message).to.eql(
465515
"All keys in the pipeline should belong to the same slots allocation group"
466516
);
517+
expect(err5.message).to.eql(
518+
"All keys in the pipeline should belong to the same slots allocation group"
519+
);
467520

468521
for (const listener of listeners) {
469522
process.on("uncaughtException", listener);
@@ -476,14 +529,21 @@ describe("autoPipelining for cluster", function () {
476529
cluster.set("foo1", "bar1", (err) => {
477530
err1 = err;
478531
});
532+
cluster.set("foo5", "bar5", (err) => {
533+
err5 = err;
534+
});
479535

480-
expect(cluster.autoPipelineQueueSize).to.eql(1);
536+
expect(cluster.autoPipelineQueueSize).to.eql(2);
481537

482538
cluster.set("foo2", (err) => {
483539
throw new Error("ERROR");
484540
});
485541

486-
expect(cluster.autoPipelineQueueSize).to.eql(2);
542+
expect(cluster.autoPipelineQueueSize).to.eql(3);
543+
544+
const key1Slot = calculateKeySlot('foo1');
545+
const key2Slot = calculateKeySlot('foo2');
546+
cluster.slots[key1Slot] = cluster.slots[key2Slot];
487547
});
488548
});
489549
});

0 commit comments

Comments
 (0)