Skip to content
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

feat: ability force custom scripts to be readOnly and execute on slaves #1057

Merged
merged 1 commit into from Feb 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/cluster/index.ts
Expand Up @@ -554,8 +554,9 @@ class Cluster extends EventEmitter {
let to = this.options.scaleReads;
if (to !== "master") {
const isCommandReadOnly =
commands.exists(command.name) &&
commands.hasFlag(command.name, "readonly");
command.isReadOnly ||
(commands.exists(command.name) &&
commands.hasFlag(command.name, "readonly"));
if (!isCommandReadOnly) {
to = "master";
}
Expand Down
9 changes: 9 additions & 0 deletions lib/command.ts
Expand Up @@ -22,6 +22,10 @@ interface ICommandOptions {
replyEncoding?: string | null;
errorStack?: string;
keyPrefix?: string;
/**
* Force the command to be readOnly so it will also execute on slaves
*/
readOnly?: boolean;
}

type ArgumentTransformer = (args: any[]) => any[];
Expand Down Expand Up @@ -142,6 +146,7 @@ export default class Command implements ICommand {
}

public ignore?: boolean;
public isReadOnly?: boolean;

private replyEncoding: string | null;
private errorStack: string;
Expand Down Expand Up @@ -185,6 +190,10 @@ export default class Command implements ICommand {
if (options.keyPrefix) {
this._iterateKeys(key => options.keyPrefix + key);
}

if (options.readOnly) {
this.isReadOnly = true;
}
}

private initPromise() {
Expand Down
4 changes: 3 additions & 1 deletion lib/commander.ts
Expand Up @@ -78,13 +78,15 @@ Commander.prototype.send_command = Commander.prototype.call;
* @param {object} definition
* @param {string} definition.lua - the lua code
* @param {number} [definition.numberOfKeys=null] - the number of keys.
* @param {boolean} [definition.readOnly=false] - force this script to be readonly so it executes on slaves as well.
* If omit, you have to pass the number of keys as the first argument every time you invoke the command
*/
Commander.prototype.defineCommand = function(name, definition) {
var script = new Script(
definition.lua,
definition.numberOfKeys,
this.options.keyPrefix
this.options.keyPrefix,
definition.readOnly
);
this.scriptsSet[name] = script;
this[name] = generateScriptingFunction(script, "utf8");
Expand Down
6 changes: 5 additions & 1 deletion lib/script.ts
Expand Up @@ -10,7 +10,8 @@ export default class Script {
constructor(
private lua: string,
private numberOfKeys: number = null,
private keyPrefix: string = ""
private keyPrefix: string = "",
private readOnly: boolean = false
) {
this.sha = createHash("sha1")
.update(lua)
Expand All @@ -29,6 +30,9 @@ export default class Script {
if (this.keyPrefix) {
options.keyPrefix = this.keyPrefix;
}
if (this.readOnly) {
options.readOnly = true;
}

const evalsha = new Command("evalsha", [this.sha].concat(args), options);
evalsha.isCustomCommand = true;
Expand Down
26 changes: 25 additions & 1 deletion test/functional/cluster/index.ts
@@ -1,6 +1,6 @@
import MockServer from "../../helpers/mock_server";
import { expect } from "chai";
import { Cluster } from "../../../lib";
import { Cluster, default as Redis } from "../../../lib";
import * as utils from "../../../lib/utils";
import * as sinon from "sinon";

Expand Down Expand Up @@ -193,6 +193,30 @@ describe("cluster", function() {
});
});
});

it("should send custom readOnly scripts to a slave", function(done) {
var cluster = new Cluster([{ host: "127.0.0.1", port: "30001" }], {
scaleReads: "slave"
});
cluster.on("ready", function() {
const redis: Redis = cluster;
redis.defineCommand("test", {
numberOfKeys: 1,
lua: "return {KEYS[1],ARGV[1],ARGV[2]}",
readOnly: true
});

const stub = sinon.stub(utils, "sample").returns("127.0.0.1:30003");
redis.test("k1", "a1", "a2", function(err, result) {
stub.restore();
expect(stub.callCount).to.eql(1);
// because of the beforeEach handler this will be the port of the slave called
expect(result).to.eql(30003);
cluster.disconnect();
done();
});
});
});
});

context("custom", function() {
Expand Down
1 change: 1 addition & 0 deletions test/functional/maxRetriesPerRequest.ts
Expand Up @@ -5,6 +5,7 @@ import { MaxRetriesPerRequestError } from "../../lib/errors";
describe("maxRetriesPerRequest", function() {
it("throw the correct error when reached the limit", function(done) {
var redis = new Redis(9999, {
connectTimeout: 1,
retryStrategy() {
return 1;
}
Expand Down