Skip to content

Commit

Permalink
Add availability_zone to the HELLO response (#1487)
Browse files Browse the repository at this point in the history
It's inconvenient for client implementations to extract the
`availability_zone` information from the `INFO` response. The `INFO`
response contains a lot of information that a client implementation
typically doesn't need.

This PR adds the availability zone to the `HELLO` response. Clients
usually already use the `HELLO` command for protocol negotiation and
also get the server `version` and `role` from its response. To keep the
`HELLO` response small, the field is only added if availability zone is
configured.

---------

Signed-off-by: Rueian <rueiancsie@gmail.com>
  • Loading branch information
rueian authored Jan 7, 2025
1 parent e1db553 commit 3b52186
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 18 deletions.
7 changes: 6 additions & 1 deletion src/networking.c
Original file line number Diff line number Diff line change
Expand Up @@ -4206,7 +4206,7 @@ void helloCommand(client *c) {

/* Let's switch to the specified RESP mode. */
if (ver) c->resp = ver;
addReplyMapLen(c, 6 + !server.sentinel_mode);
addReplyMapLen(c, 6 + !server.sentinel_mode + (sdslen(server.availability_zone) != 0));

addReplyBulkCString(c, "server");
addReplyBulkCString(c, server.extended_redis_compat ? "redis" : SERVER_NAME);
Expand Down Expand Up @@ -4235,6 +4235,11 @@ void helloCommand(client *c) {

addReplyBulkCString(c, "modules");
addReplyLoadedModules(c);

if (sdslen(server.availability_zone) != 0) {
addReplyBulkCString(c, "availability_zone");
addReplyBulkCBuffer(c, server.availability_zone, sdslen(server.availability_zone));
}
}

/* This callback is bound to POST and "Host:" command names. Those are not
Expand Down
34 changes: 34 additions & 0 deletions tests/unit/protocol.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,40 @@ start_server {tags {"protocol network"}} {

}

start_server {tags {"protocol hello"}} {
test {HELLO without protover} {
set reply [r HELLO 3]
assert_equal [dict get $reply proto] 3

set reply [r HELLO]
assert_equal [dict get $reply proto] 3

set reply [r HELLO 2]
assert_equal [dict get $reply proto] 2

set reply [r HELLO]
assert_equal [dict get $reply proto] 2
}

test {HELLO and availability-zone} {
r CONFIG SET availability-zone myzone

set reply [r HELLO 3]
assert_equal [dict get $reply availability_zone] myzone

set reply [r HELLO 2]
assert_equal [dict get $reply availability_zone] myzone

r CONFIG SET availability-zone ""

set reply [r HELLO 3]
assert_equal [dict exists $reply availability_zone] 0

set reply [r HELLO 2]
assert_equal [dict exists $reply availability_zone] 0
}
}

start_server {tags {"regression"}} {
test "Regression for a crash with blocking ops and pipelining" {
set rd [valkey_deferring_client]
Expand Down
17 changes: 0 additions & 17 deletions tests/unit/tracking.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -154,23 +154,6 @@ start_server {tags {"tracking network logreqres:skip"}} {
assert_equal [dict get $reply proto] 3
}

test {HELLO without protover} {
set reply [r HELLO 3]
assert_equal [dict get $reply proto] 3

set reply [r HELLO]
assert_equal [dict get $reply proto] 3

set reply [r HELLO 2]
assert_equal [dict get $reply proto] 2

set reply [r HELLO]
assert_equal [dict get $reply proto] 2

# restore RESP3 for next test
r HELLO 3
}

test {RESP3 based basic invalidation} {
r CLIENT TRACKING off
r CLIENT TRACKING on
Expand Down

0 comments on commit 3b52186

Please sign in to comment.