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

replica redirect read&write to master in standalone mode #325

Open
wants to merge 4 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
8 changes: 0 additions & 8 deletions src/cluster.c
Expand Up @@ -1461,20 +1461,12 @@ void askingCommand(client *c) {
* In this mode slaves will not redirect clients as long as clients access
* with read-only commands to keys that are served by the slave's master. */
void readonlyCommand(client *c) {
if (server.cluster_enabled == 0) {
addReplyError(c,"This instance has cluster support disabled");
return;
}
c->flags |= CLIENT_READONLY;
addReply(c,shared.ok);
}

/* The READWRITE command just clears the READONLY command state. */
void readwriteCommand(client *c) {
if (server.cluster_enabled == 0) {
addReplyError(c,"This instance has cluster support disabled");
return;
}
c->flags &= ~CLIENT_READONLY;
addReply(c,shared.ok);
}
1 change: 1 addition & 0 deletions src/config.c
Expand Up @@ -3094,6 +3094,7 @@ standardConfig static_configs[] = {
createBoolConfig("replica-serve-stale-data", "slave-serve-stale-data", MODIFIABLE_CONFIG, server.repl_serve_stale_data, 1, NULL, NULL),
createBoolConfig("replica-read-only", "slave-read-only", DEBUG_CONFIG | MODIFIABLE_CONFIG, server.repl_slave_ro, 1, NULL, NULL),
createBoolConfig("replica-ignore-maxmemory", "slave-ignore-maxmemory", MODIFIABLE_CONFIG, server.repl_slave_ignore_maxmemory, 1, NULL, NULL),
createBoolConfig("replica-enable-redirect", NULL, MODIFIABLE_CONFIG, server.repl_replica_enable_redirect, 0, NULL, NULL),
createBoolConfig("jemalloc-bg-thread", NULL, MODIFIABLE_CONFIG, server.jemalloc_bg_thread, 1, NULL, updateJemallocBgThread),
createBoolConfig("activedefrag", NULL, DEBUG_CONFIG | MODIFIABLE_CONFIG, server.active_defrag_enabled, 0, isValidActiveDefrag, NULL),
createBoolConfig("syslog-enabled", NULL, IMMUTABLE_CONFIG, server.syslog_enabled, 0, NULL, NULL),
Expand Down
11 changes: 11 additions & 0 deletions src/server.c
Expand Up @@ -4009,6 +4009,17 @@ int processCommand(client *c) {
}
}

if (!server.cluster_enabled &&
server.repl_replica_enable_redirect &&
server.masterhost &&
!mustObeyClient(c) &&
(is_write_command ||
(is_read_command && !(c->flags & CLIENT_READONLY)))) {
addReplyErrorSds(c,sdscatprintf(sdsempty(), "-REDIRECT %s:%d",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is -1 missing here? Keep the same format as MOVED and the client can reuse the parsing code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 is not part of MOVED.

MOVED is for a slot. REDIRECT is for all writes to the node, without slot. (Only cluster has slots.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MOVED is for a slot. REDIRECT is for all writes to the node, without slot. (Only cluster has slots.)

ok, that's reasonable from valkey developer.

server.masterhost, server.masterport));
return C_OK;
}

/* Disconnect some clients if total clients memory is too high. We do this
* before key eviction, after the last command was executed and consumed
* some client output buffer memory. */
Expand Down
1 change: 1 addition & 0 deletions src/server.h
Expand Up @@ -1911,6 +1911,7 @@ struct valkeyServer {
int repl_serve_stale_data; /* Serve stale data when link is down? */
int repl_slave_ro; /* Slave is read only? */
int repl_slave_ignore_maxmemory; /* If true slaves do not evict. */
int repl_replica_enable_redirect; /* If true replica would redirect read&write commands to master. */
time_t repl_down_since; /* Unix time at which link with master went down */
int repl_disable_tcp_nodelay; /* Disable TCP_NODELAY after SYNC? */
int slave_priority; /* Reported in INFO and used by Sentinel. */
Expand Down
36 changes: 36 additions & 0 deletions tests/integration/replica-redirect.tcl
@@ -0,0 +1,36 @@
start_server {tags {needs:repl external:skip}} {
start_server {} {
set master_host [srv -1 host]
set master_port [srv -1 port]

r replicaof $master_host $master_port
wait_for_condition 50 100 {
[s 0 master_link_status] eq {up}
} else {
fail "Replicas not replicating from master"
}

test {replica allow read command by default} {
r get foo
} {}

test {replica reply READONLY error for write command by default} {
assert_error {READONLY*} {r set foo bar}
}

test {replica redirect read and write command when enable replica-enable-redirect} {
r config set replica-enable-redirect yes
assert_error "REDIRECT $master_host:$master_port" {r set foo bar}
assert_error "REDIRECT $master_host:$master_port" {r get foo}
}

test {non-data access commands are not redirected} {
r ping
} {PONG}

test {replica allow read command in READONLY mode} {
r readonly
r get foo
} {}
}
}
12 changes: 12 additions & 0 deletions valkey.conf
Expand Up @@ -817,6 +817,18 @@ replica-priority 100
# replica-announce-ip 5.5.5.5
# replica-announce-port 1234

# You can configure a replica instance to redirect data access commands
# to its master or not, excluding commands such as:
# INFO, AUTH, ROLE, SUBSCRIBE, UNSUBSCRIBE, PUBLISH.
#
# When enabled, a replica instance will reply "-MOVED -1 master-ip:port"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# When enabled, a replica instance will reply "-MOVED -1 master-ip:port"
# When enabled, a replica instance will reply "-REDIRECT -1 master-ip:port"

# for data access commands. Normally these are write or read commands, if
# you want to run read commands while replica-enable-redirect is enabled,
# use the READONLY command first.
#
# This config only takes effect in standalone mode.
replica-enable-redirect no

############################### KEYS TRACKING #################################

# The client side caching of values is assisted via server-side support.
Expand Down