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.

Sign up for GitHub

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

Allow Module blocking operations (not blocked on keys) to succeed when cluster is down #693

Merged
merged 8 commits into from
Jul 1, 2024
Merged

Allow Module blocking operations (not blocked on keys) to succeed when cluster is down #693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
66af907
Allow Module authentication to succeed when cluster is down
KarthikSubbarao Jun 25, 2024
911456a
Fix formatting
KarthikSubbarao Jun 25, 2024
2be807e
Update test to ensure cluster is down
KarthikSubbarao Jun 25, 2024
b9a181d
Update test
KarthikSubbarao Jul 1, 2024
e588107
Exempt Module blocking operations (not blocked on keys) from cluster …
KarthikSubbarao Jul 1, 2024
1c9c498
Merge branch 'unstable' into auth
KarthikSubbarao Jul 1, 2024
799017b
Merge from unstable
KarthikSubbarao Jul 1, 2024
fba04c5
Merge from unstable
KarthikSubbarao Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/cluster.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,10 @@ int clusterRedirectBlockedClientIfNeeded(client *c) {
dictEntry *de;
dictIterator *di;

/* If the client is blocked on module, but not on a specific key,
* don't unblock it. */
if (c->bstate.btype == BLOCKED_MODULE && !moduleClientIsBlockedOnKeys(c)) return 0;

/* If the cluster is down, unblock the client with the right error.
* If the cluster is configured to allow reads on cluster down, we
* still want to emit this error since a write will be required
Expand All @@ -1227,10 +1231,6 @@ int clusterRedirectBlockedClientIfNeeded(client *c) {
return 1;
}

/* If the client is blocked on module, but not on a specific key,
* don't unblock it (except for the CLUSTER_FAIL case above). */
if (c->bstate.btype == BLOCKED_MODULE && !moduleClientIsBlockedOnKeys(c)) return 0;

/* All keys must belong to the same slot, so check first key only. */
di = dictGetIterator(c->bstate.keys);
if ((de = dictNext(di)) != NULL) {
Expand Down
24 changes: 17 additions & 7 deletions tests/unit/moduleapi/cluster.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ tags {tls:skip external:skip cluster modules} {
set testmodule_nokey [file normalize tests/modules/blockonbackground.so]
set testmodule_blockedclient [file normalize tests/modules/blockedclient.so]
set testmodule [file normalize tests/modules/blockonkeys.so]
set testmodule_auth [file normalize tests/modules/auth.so]

set modules [list loadmodule $testmodule loadmodule $testmodule_nokey loadmodule $testmodule_blockedclient]
set modules [list loadmodule $testmodule loadmodule $testmodule_nokey loadmodule $testmodule_blockedclient loadmodule $testmodule_auth]
start_cluster 3 0 [list config_lines $modules] {

set node1 [srv 0 client]
Expand Down Expand Up @@ -146,18 +147,27 @@ start_cluster 3 0 [list config_lines $modules] {
assert_error {*CLUSTERDOWN*} {$node1_rd read}
}

test "Verify command (no keys) got unblocked after cluster failure" {
assert_error {*CLUSTERDOWN*} {$node2_rd read}

# verify there are no blocked clients
assert_equal [s 0 blocked_clients] {0}
assert_equal [s -1 blocked_clients] {0}
test "Verify command (with no keys) is not unblocked after cluster failure" {
assert_no_match {*CLUSTERDOWN*} {$node2_rd read}
# verify there are blocked clients
assert_equal [s -1 blocked_clients] {1}
}

test "Verify command RM_Call is rejected when cluster is down" {
assert_error "ERR Can not execute a command 'set' while the cluster is down" {$node1 do_rm_call set x 1}
}

test "Verify Module Auth Succeeds when cluster is down" {
r acl setuser foo >pwd on ~* &* +@all
assert_error "*CLUSTERDOWN*" {r set x 1}
# Non Blocking Module Auth
assert_equal {OK} [r testmoduleone.rm_register_auth_cb]
KarthikSubbarao marked this conversation as resolved.
Show resolved Hide resolved
assert_equal {OK} [r AUTH foo allow]
# Blocking Module Auth
assert_equal {OK} [r testmoduleone.rm_register_blocking_auth_cb]
assert_equal {OK} [r AUTH foo block_allow]
}

resume_process $node3_pid
$node1_rd close
$node2_rd close
Expand Down
Loading