Add master/slave option in --cluster call command #6491

thandayuthapani · 2019-10-23T11:07:57Z

Related to https://github.com/antirez/redis/issues/6474

Output Log:
Default Option:

src/redis-cli --cluster call 127.0.0.1:6001 set key1 value1
>>> Calling set key1 value1
127.0.0.1:6001: MOVED 9189 127.0.0.1:6002

127.0.0.1:7001: MOVED 9189 127.0.0.1:6002

127.0.0.1:6003: MOVED 9189 127.0.0.1:6002

127.0.0.1:6002: OK
127.0.0.1:7002: MOVED 9189 127.0.0.1:6002

127.0.0.1:7003: MOVED 9189 127.0.0.1:6002

Master Option:

src/redis-cli --cluster call 127.0.0.1:6001 master set key2 value1
>>> Calling set key2 value1
127.0.0.1:6001: OK
127.0.0.1:6003: MOVED 4998 127.0.0.1:6001

127.0.0.1:6002: MOVED 4998 127.0.0.1:6001

Slave Option:

src/redis-cli --cluster call 127.0.0.1:6001 slave set key2 value1
>>> Calling set key2 value1
127.0.0.1:7001: MOVED 4998 127.0.0.1:6001

127.0.0.1:7002: MOVED 4998 127.0.0.1:6001

127.0.0.1:7003: MOVED 4998 127.0.0.1:6001

thandayuthapani · 2019-10-23T11:15:54Z

Did not use --master or --slave, since there is already a --slave sub-command, for slave mode. So it might create conflict while parsing the command.

artix75 · 2019-10-23T16:51:28Z

Why do you want to use the call subcommand with redis set command?

thandayuthapani · 2019-10-23T17:12:09Z

@artix75 This PR is related to this issue #6474. Have included output log just to show expected behaviour as shown in that issue. So have used call and set in the above log.

thandayuthapani · 2019-10-24T14:14:17Z

I have currently implemented the command like this,
src/redis-cli --cluster call 127.0.0.1:6001 **master/slave** set key2 value1
Or I can implement like this also
src/redis-cli --cluster call **master/slave** 127.0.0.1:6001 set key2 value1

Or if you want me to implement in some other way, can you let me know. So I can make those changes?

This change might not be necessary for set command, but it might be useful for calling all slaves for setting masterauth instead of setting it in each slave separately. This is one case where call subcommand can be used to set some configs like masterauth in all slaves, when in master side Auth is enabled using requirepass config.

@artix75 Can you suggest any modification if necessary so that I can work on that.

artix75 · 2019-10-24T14:30:06Z

@thandayuthapani I just asked since the use-case of your example is quite weird/rare (it makes no sense IMHO to use the call subcommand with Redis commands involving keys, such as the set command).

Anyway, I'd suggest using the conventional --cluster-* pattern for the command-line options. So, instead of master or slave, you could name them --cluster-only-masters or --cluster-only-slaves.

hengku · 2019-10-24T18:28:17Z

Hi @thandayuthapani , I checked current command-line options and agreed with @artix75 . What I am understanding is: '--cluster-only-masters' or '--cluster-only-slaves' specifies on which Redis servers we want to operate; the sub-commands specify which operations we want to execute.

thandayuthapani · 2019-10-25T10:03:50Z

@artix75 Have made those changes. Please have a look.

artix75

Please, remind to document new command options (--cluster-only-*) in the --cluster help too (see the clusterManagerCommandHelp function). Thank you.

src/redis-cli.c

thandayuthapani · 2019-10-30T09:24:29Z

@artix75 Have made those changes.. Please have a look.

src/redis-cli.c

thandayuthapani · 2019-10-31T08:30:46Z

@artix75 Have made those changes. Please have a look.

thandayuthapani · 2019-11-06T13:20:12Z

@artix75 Is there any other changes to be done for this PR?

artix75 · 2019-11-07T15:21:49Z

@artix75 Is there any other changes to be done for this PR?

Now it's ok for me. Could you compact this PR in just one commit, in order to avoid all those "Addressed review comments" commits?

thandayuthapani · 2019-11-08T06:07:29Z

@artix75 Have squashed all those commits into single commit.

artix75 · 2019-11-08T18:21:58Z

@artix75 Have squashed all those commits into single commit.

@thandayuthapani It's ok for me, thank you.
@antirez This can be merged.

thandayuthapani · 2019-11-19T16:55:21Z

@antirez I hope this PR can be merged. @artix75 is okay with this enhancement.

thandayuthapani · 2019-12-02T10:58:01Z

Hi @antirez, can you please take a look this PR to see if it can be merged?

itamarhaber

Hi @thandayuthapani - sorry for the long delay, I think this can be finally merged, thanks.

src/redis-cli.c

itamarhaber · 2020-09-02T13:21:45Z

Fixes #6474

* Add master/slave option in --cluster call command * Update src/redis-cli.c * Update src/redis-cli.c Co-authored-by: Itamar Haber <itamar@redislabs.com> (cherry picked from commit f22f64f)

…is#6491) * Add master/slave option in --cluster call command * Update src/redis-cli.c * Update src/redis-cli.c Co-authored-by: Itamar Haber <itamar@redislabs.com>

…is#6491) * Add master/slave option in --cluster call command * Update src/redis-cli.c * Update src/redis-cli.c Co-authored-by: Itamar Haber <itamar@redislabs.com> (cherry picked from commit f22f64f)

amberpharswan · 2024-06-25T12:30:20Z

Please update PR description for sample commands.
Example, I am connecting to Redis7.1 elasticache with auth.
This works: redis6-cli -a "password" --tls --cluster call clustercfg.asdfghjkl.use1.cache.amazonaws.com:6379 KEYS "pattern*"

but this doesn't: redis6-cli -a "password" --tls --cluster-only-masters call clustercfg.asdfghjkl.use1.cache.amazonaws.com:6379 KEYS "pattern*"
cluster-only-masters is picking localhost as host somehow
Error: Could not connect to Redis at 127.0.0.1:6379: Connection refused

thandayuthapani mentioned this pull request Oct 23, 2019

redis-cli --cluster tries to broadcast a command to all the shards including slaves #6474

Closed

3 tasks

artix75 reviewed Oct 29, 2019

View reviewed changes

src/redis-cli.c Outdated Show resolved Hide resolved

src/redis-cli.c Outdated Show resolved Hide resolved

src/redis-cli.c Outdated Show resolved Hide resolved

artix75 reviewed Oct 30, 2019

View reviewed changes

src/redis-cli.c Outdated Show resolved Hide resolved

Add master/slave option in --cluster call command

92b4e02

thandayuthapani force-pushed the cli branch from 7067355 to 92b4e02 Compare November 8, 2019 06:05

Merge branch 'unstable' into cli

59dae45

itamarhaber previously approved these changes Sep 2, 2020

View reviewed changes

src/redis-cli.c Outdated Show resolved Hide resolved

src/redis-cli.c Outdated Show resolved Hide resolved

Update src/redis-cli.c

4bfc35f

itamarhaber dismissed their stale review via 4bfc35f September 2, 2020 13:14

Update src/redis-cli.c

3456fb4

itamarhaber approved these changes Sep 2, 2020

View reviewed changes

itamarhaber merged commit f22f64f into redis:unstable Sep 2, 2020

Add master/slave option in --cluster call command #6491

Add master/slave option in --cluster call command #6491

Uh oh!

Conversation

thandayuthapani commented Oct 23, 2019

Uh oh!

thandayuthapani commented Oct 23, 2019

Uh oh!

artix75 commented Oct 23, 2019

Uh oh!

thandayuthapani commented Oct 23, 2019

Uh oh!

thandayuthapani commented Oct 24, 2019

Uh oh!

artix75 commented Oct 24, 2019

Uh oh!

hengku commented Oct 24, 2019

Uh oh!

thandayuthapani commented Oct 25, 2019

Uh oh!

artix75 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thandayuthapani commented Oct 30, 2019

Uh oh!

Uh oh!

thandayuthapani commented Oct 31, 2019

Uh oh!

thandayuthapani commented Nov 6, 2019

Uh oh!

artix75 commented Nov 7, 2019

Uh oh!

thandayuthapani commented Nov 8, 2019

Uh oh!

artix75 commented Nov 8, 2019

Uh oh!

thandayuthapani commented Nov 19, 2019

Uh oh!

thandayuthapani commented Dec 2, 2019

Uh oh!

itamarhaber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

itamarhaber commented Sep 2, 2020

Uh oh!

amberpharswan commented Jun 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants