valkey icon indicating copy to clipboard operation
valkey copied to clipboard

[BUG] "CLIENT CAPA redirect" has no effect in scripts

Open gmbnomis opened this issue 1 year ago • 1 comments

Describe the bug

A read or write command should be redirected when issued by a client that understands "REDIRECT". This does not work when the command is part of a script.

To reproduce

Connect the CLI client to a replica and do:

localhost:6380> client capa redirect
OK
localhost:6380> set foo bar
(error) REDIRECT redis:6379
localhost:6380> EVAL "return redis.call('SET', KEYS[1], ARGV[1])" 1 foo bar
(error) READONLY You can't write against a read only replica. script: d8f2fad9f8e86a53d2a6ebd960b33c4972cacc37, on @user_script:1.
localhost:6380> get foo
(error) REDIRECT redis:6379
localhost:6380> EVAL "return redis.call('GET', KEYS[1])" 1 foo
"bar"

(Note: I did a set foo bar on the primary to test replication, that's why the last command returns "bar".)

Expected behavior

REDIRECT should be issued regardless of the context if the client understands redirect. (for reference, Valkey cluster issues the same "MOVED" in both cases:

127.0.0.1:30004> readwrite
OK
127.0.0.1:30004> set b b
(error) MOVED 3300 127.0.0.1:30001
127.0.0.1:30004> EVAL "return redis.call('SET', KEYS[1], ARGV[1])" 1 b b
(error) MOVED 3300 127.0.0.1:30001

)

gmbnomis avatar Aug 04 '24 14:08 gmbnomis

It's a little odd EVAL_RO get's redirected to the primary, but not EVAL. We should evaluate if we want the REDIRECT flag to act more like the cluster mode version.

madolson avatar Aug 05 '24 14:08 madolson

... We should evaluate if we want the REDIRECT flag to act more like the cluster mode version.

@madolson Yes, it turned out that there are indeed other cases we need to take care of. I addressed the ones I am aware of in PR #1781, but there is a fundamental difference between cluster and standalone modes with respect to publish/subscribe that I don't know how to solve best. I created issue #1780 for it.

gmbnomis avatar Feb 25 '25 21:02 gmbnomis