From cf2a68098d8a2ec7eab9966d982ff710ba0e1f74 Mon Sep 17 00:00:00 2001 From: Binbin Date: Wed, 15 Jan 2025 13:26:01 +0800 Subject: [PATCH 1/4] Add paused_purpose to INFO CLIENTS In #1519, we added paused_actions and paused_timeout_milliseconds, it would be helpful if we add the paused_purpose since users also want to know the purpose for the pause. Signed-off-by: Binbin --- src/networking.c | 22 ++++++++++++++++++++-- src/server.c | 9 +++++++-- src/server.h | 3 ++- tests/unit/pause.tcl | 26 ++++++++++++++++++++++++-- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/networking.c b/src/networking.c index 48e397e6f4..c2c4622a12 100644 --- a/src/networking.c +++ b/src/networking.c @@ -4520,12 +4520,30 @@ void flushReplicasOutputBuffers(void) { } } -mstime_t getPausedActionTimeout(uint32_t action) { +char *getPausedPurposeString(pause_purpose purpose) { + switch (purpose) { + case PAUSE_BY_CLIENT_COMMAND: + return "client_command"; + case PAUSE_DURING_SHUTDOWN: + return "during_shutdown"; + case PAUSE_DURING_FAILOVER: + return "during_failover"; + case NUM_PAUSE_PURPOSES: + return "none"; + default: + return "Unknown pause purpose"; + } +} + +mstime_t getPausedActionTimeout(uint32_t action, pause_purpose *purpose) { mstime_t timeout = 0; + *purpose = NUM_PAUSE_PURPOSES; for (int i = 0; i < NUM_PAUSE_PURPOSES; i++) { pause_event *p = &(server.client_pause_per_purpose[i]); - if (p->paused_actions & action && (p->end - server.mstime) > timeout) + if (p->paused_actions & action && (p->end - server.mstime) > timeout) { timeout = p->end - server.mstime; + *purpose = i; + } } return timeout; } diff --git a/src/server.c b/src/server.c index 8255b57e25..a95c0e4a55 100644 --- a/src/server.c +++ b/src/server.c @@ -5669,14 +5669,18 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) { getExpansiveClientsInfo(&maxin, &maxout); totalNumberOfStatefulKeys(&blocking_keys, &blocking_keys_on_nokey, &watched_keys); + pause_purpose purpose; + char *paused_purpose = "none"; char *paused_actions = "none"; long long paused_timeout = 0; if (server.paused_actions & PAUSE_ACTION_CLIENT_ALL) { paused_actions = "all"; - paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_ALL); + paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_ALL, &purpose); + paused_purpose = getPausedPurposeString(purpose); } else if (server.paused_actions & PAUSE_ACTION_CLIENT_WRITE) { paused_actions = "write"; - paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_WRITE); + paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_WRITE, &purpose); + paused_purpose = getPausedPurposeString(purpose); } if (sections++) info = sdscat(info, "\r\n"); @@ -5696,6 +5700,7 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) { "total_watched_keys:%lu\r\n", watched_keys, "total_blocking_keys:%lu\r\n", blocking_keys, "total_blocking_keys_on_nokey:%lu\r\n", blocking_keys_on_nokey, + "paused_purpose:%s\r\n", paused_purpose, "paused_actions:%s\r\n", paused_actions, "paused_timeout_milliseconds:%lld\r\n", paused_timeout)); } diff --git a/src/server.h b/src/server.h index d186d16c73..97f2fbaca3 100644 --- a/src/server.h +++ b/src/server.h @@ -2714,7 +2714,8 @@ void pauseActions(pause_purpose purpose, mstime_t end, uint32_t actions); void unpauseActions(pause_purpose purpose); uint32_t isPausedActions(uint32_t action_bitmask); uint32_t isPausedActionsWithUpdate(uint32_t action_bitmask); -mstime_t getPausedActionTimeout(uint32_t action); +char *getPausedPurposeString(pause_purpose purpose); +mstime_t getPausedActionTimeout(uint32_t action, pause_purpose *purpose); void updatePausedActions(void); void unblockPostponedClients(void); void processEventsWhileBlocked(void); diff --git a/tests/unit/pause.tcl b/tests/unit/pause.tcl index 9697c3b44e..50c192f561 100644 --- a/tests/unit/pause.tcl +++ b/tests/unit/pause.tcl @@ -1,9 +1,11 @@ start_server {tags {"pause network"}} { - test "Test check paused_actions in info stats" { + test "Test check paused info in info clients" { + assert_equal [s paused_purpose] "none" assert_equal [s paused_actions] "none" assert_equal [s paused_timeout_milliseconds] 0 r client PAUSE 10000 WRITE + assert_equal [s paused_purpose] "client_command" assert_equal [s paused_actions] "write" after 1000 set timeout [s paused_timeout_milliseconds] @@ -13,9 +15,14 @@ start_server {tags {"pause network"}} { r multi r client PAUSE 1000 ALL r info clients - assert_match "*paused_actions:all*" [r exec] + set res [r exec] + assert_match "*paused_purpose:client_command*" $res + assert_match "*paused_actions:all*" $res r client unpause + assert_equal [s paused_purpose] "none" + assert_equal [s paused_actions] "none" + assert_equal [s paused_timeout_milliseconds] 0 } test "Test read commands are not blocked by client pause" { @@ -408,3 +415,18 @@ start_server {tags {"pause network"}} { # Make sure we unpause at the end r client unpause } + +start_cluster 1 1 {tags {"external:skip cluster pause network"}} { + test "Test check paused info during the cluster failover in info clients" { + assert_equal [s 0 paused_purpose] "none" + assert_equal [s 0 paused_actions] "none" + assert_equal [s 0 paused_timeout_milliseconds] 0 + + R 1 cluster failover + wait_for_log_messages 0 {"*Manual failover requested by replica*"} 0 10 1000 + + assert_equal [s 0 paused_purpose] "during_failover" + assert_equal [s 0 paused_actions] "write" + assert_morethan [s 0 paused_timeout_milliseconds] 0 + } +} From 376fd336da5f40094418065d88d55290ffaf4847 Mon Sep 17 00:00:00 2001 From: Binbin Date: Wed, 15 Jan 2025 13:45:43 +0800 Subject: [PATCH 2/4] fix format Signed-off-by: Binbin --- src/networking.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/networking.c b/src/networking.c index c2c4622a12..98a5235930 100644 --- a/src/networking.c +++ b/src/networking.c @@ -4522,16 +4522,16 @@ void flushReplicasOutputBuffers(void) { char *getPausedPurposeString(pause_purpose purpose) { switch (purpose) { - case PAUSE_BY_CLIENT_COMMAND: - return "client_command"; - case PAUSE_DURING_SHUTDOWN: - return "during_shutdown"; - case PAUSE_DURING_FAILOVER: - return "during_failover"; - case NUM_PAUSE_PURPOSES: - return "none"; - default: - return "Unknown pause purpose"; + case PAUSE_BY_CLIENT_COMMAND: + return "client_command"; + case PAUSE_DURING_SHUTDOWN: + return "during_shutdown"; + case PAUSE_DURING_FAILOVER: + return "during_failover"; + case NUM_PAUSE_PURPOSES: + return "none"; + default: + return "Unknown pause purpose"; } } From c442b79308acd45a19cfd22adf399d334573a8b8 Mon Sep 17 00:00:00 2001 From: Binbin Date: Mon, 20 Jan 2025 13:15:23 +0800 Subject: [PATCH 3/4] update the function name to remove the string suffix Signed-off-by: Binbin --- src/networking.c | 2 +- src/server.c | 4 ++-- src/server.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/networking.c b/src/networking.c index 98a5235930..eb0d5f0534 100644 --- a/src/networking.c +++ b/src/networking.c @@ -4520,7 +4520,7 @@ void flushReplicasOutputBuffers(void) { } } -char *getPausedPurposeString(pause_purpose purpose) { +char *getPausedPurpose(pause_purpose purpose) { switch (purpose) { case PAUSE_BY_CLIENT_COMMAND: return "client_command"; diff --git a/src/server.c b/src/server.c index a95c0e4a55..8e91ec7947 100644 --- a/src/server.c +++ b/src/server.c @@ -5676,11 +5676,11 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) { if (server.paused_actions & PAUSE_ACTION_CLIENT_ALL) { paused_actions = "all"; paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_ALL, &purpose); - paused_purpose = getPausedPurposeString(purpose); + paused_purpose = getPausedPurpose(purpose); } else if (server.paused_actions & PAUSE_ACTION_CLIENT_WRITE) { paused_actions = "write"; paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_WRITE, &purpose); - paused_purpose = getPausedPurposeString(purpose); + paused_purpose = getPausedPurpose(purpose); } if (sections++) info = sdscat(info, "\r\n"); diff --git a/src/server.h b/src/server.h index 97f2fbaca3..ff284a13f9 100644 --- a/src/server.h +++ b/src/server.h @@ -2714,7 +2714,7 @@ void pauseActions(pause_purpose purpose, mstime_t end, uint32_t actions); void unpauseActions(pause_purpose purpose); uint32_t isPausedActions(uint32_t action_bitmask); uint32_t isPausedActionsWithUpdate(uint32_t action_bitmask); -char *getPausedPurposeString(pause_purpose purpose); +char *getPausedPurpose(pause_purpose purpose); mstime_t getPausedActionTimeout(uint32_t action, pause_purpose *purpose); void updatePausedActions(void); void unblockPostponedClients(void); From 00371df5cc6a0bb9c0f7cb39b71707f7730415ce Mon Sep 17 00:00:00 2001 From: Binbin Date: Tue, 21 Jan 2025 18:51:25 +0800 Subject: [PATCH 4/4] Change to paused_reason Signed-off-by: Binbin --- src/networking.c | 10 +++++----- src/server.c | 8 ++++---- src/server.h | 2 +- tests/unit/pause.tcl | 12 ++++++------ 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/networking.c b/src/networking.c index eb0d5f0534..b89cf93013 100644 --- a/src/networking.c +++ b/src/networking.c @@ -4520,18 +4520,18 @@ void flushReplicasOutputBuffers(void) { } } -char *getPausedPurpose(pause_purpose purpose) { +char *getPausedReason(pause_purpose purpose) { switch (purpose) { case PAUSE_BY_CLIENT_COMMAND: - return "client_command"; + return "client_pause"; case PAUSE_DURING_SHUTDOWN: - return "during_shutdown"; + return "shutdown_in_progress"; case PAUSE_DURING_FAILOVER: - return "during_failover"; + return "failover_in_progress"; case NUM_PAUSE_PURPOSES: return "none"; default: - return "Unknown pause purpose"; + return "Unknown pause reason"; } } diff --git a/src/server.c b/src/server.c index 8e91ec7947..44e14c5803 100644 --- a/src/server.c +++ b/src/server.c @@ -5670,17 +5670,17 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) { totalNumberOfStatefulKeys(&blocking_keys, &blocking_keys_on_nokey, &watched_keys); pause_purpose purpose; - char *paused_purpose = "none"; + char *paused_reason = "none"; char *paused_actions = "none"; long long paused_timeout = 0; if (server.paused_actions & PAUSE_ACTION_CLIENT_ALL) { paused_actions = "all"; paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_ALL, &purpose); - paused_purpose = getPausedPurpose(purpose); + paused_reason = getPausedReason(purpose); } else if (server.paused_actions & PAUSE_ACTION_CLIENT_WRITE) { paused_actions = "write"; paused_timeout = getPausedActionTimeout(PAUSE_ACTION_CLIENT_WRITE, &purpose); - paused_purpose = getPausedPurpose(purpose); + paused_reason = getPausedReason(purpose); } if (sections++) info = sdscat(info, "\r\n"); @@ -5700,7 +5700,7 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) { "total_watched_keys:%lu\r\n", watched_keys, "total_blocking_keys:%lu\r\n", blocking_keys, "total_blocking_keys_on_nokey:%lu\r\n", blocking_keys_on_nokey, - "paused_purpose:%s\r\n", paused_purpose, + "paused_reason:%s\r\n", paused_reason, "paused_actions:%s\r\n", paused_actions, "paused_timeout_milliseconds:%lld\r\n", paused_timeout)); } diff --git a/src/server.h b/src/server.h index ff284a13f9..83b52dba72 100644 --- a/src/server.h +++ b/src/server.h @@ -2714,7 +2714,7 @@ void pauseActions(pause_purpose purpose, mstime_t end, uint32_t actions); void unpauseActions(pause_purpose purpose); uint32_t isPausedActions(uint32_t action_bitmask); uint32_t isPausedActionsWithUpdate(uint32_t action_bitmask); -char *getPausedPurpose(pause_purpose purpose); +char *getPausedReason(pause_purpose purpose); mstime_t getPausedActionTimeout(uint32_t action, pause_purpose *purpose); void updatePausedActions(void); void unblockPostponedClients(void); diff --git a/tests/unit/pause.tcl b/tests/unit/pause.tcl index 50c192f561..08ded13018 100644 --- a/tests/unit/pause.tcl +++ b/tests/unit/pause.tcl @@ -1,11 +1,11 @@ start_server {tags {"pause network"}} { test "Test check paused info in info clients" { - assert_equal [s paused_purpose] "none" + assert_equal [s paused_reason] "none" assert_equal [s paused_actions] "none" assert_equal [s paused_timeout_milliseconds] 0 r client PAUSE 10000 WRITE - assert_equal [s paused_purpose] "client_command" + assert_equal [s paused_reason] "client_pause" assert_equal [s paused_actions] "write" after 1000 set timeout [s paused_timeout_milliseconds] @@ -16,11 +16,11 @@ start_server {tags {"pause network"}} { r client PAUSE 1000 ALL r info clients set res [r exec] - assert_match "*paused_purpose:client_command*" $res + assert_match "*paused_reason:client_pause*" $res assert_match "*paused_actions:all*" $res r client unpause - assert_equal [s paused_purpose] "none" + assert_equal [s paused_reason] "none" assert_equal [s paused_actions] "none" assert_equal [s paused_timeout_milliseconds] 0 } @@ -418,14 +418,14 @@ start_server {tags {"pause network"}} { start_cluster 1 1 {tags {"external:skip cluster pause network"}} { test "Test check paused info during the cluster failover in info clients" { - assert_equal [s 0 paused_purpose] "none" + assert_equal [s 0 paused_reason] "none" assert_equal [s 0 paused_actions] "none" assert_equal [s 0 paused_timeout_milliseconds] 0 R 1 cluster failover wait_for_log_messages 0 {"*Manual failover requested by replica*"} 0 10 1000 - assert_equal [s 0 paused_purpose] "during_failover" + assert_equal [s 0 paused_reason] "failover_in_progress" assert_equal [s 0 paused_actions] "write" assert_morethan [s 0 paused_timeout_milliseconds] 0 }