From a901e6406581c6996e93cb105dbedf6809159d1f Mon Sep 17 00:00:00 2001 From: Binbin Date: Fri, 10 Jan 2025 11:55:13 +0800 Subject: [PATCH 1/2] Fix module LatencyAddSample still work when latency-monitor-threshold is 0 When latency-monitor-threshold is set to 0, it means the latency monitor is disabled, and in VM_LatencyAddSample, we wrote the condition incorrectly, causing us to record latency when latency was turned off. Signed-off-by: Binbin --- src/module.c | 3 ++- tests/modules/basics.c | 21 +++++++++++++++++++++ tests/unit/moduleapi/basics.tcl | 17 +++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/module.c b/src/module.c index 58555839f2..d256dc7a3d 100644 --- a/src/module.c +++ b/src/module.c @@ -7680,7 +7680,8 @@ void VM__Assert(const char *estr, const char *file, int line) { * command. The call is skipped if the latency is smaller than the configured * latency-monitor-threshold. */ void VM_LatencyAddSample(const char *event, mstime_t latency) { - if (latency >= server.latency_monitor_threshold) latencyAddSample(event, latency); + if (server.latency_monitor_threshold && latency >= server.latency_monitor_threshold) + latencyAddSample(event, latency); } /* -------------------------------------------------------------------------- diff --git a/tests/modules/basics.c b/tests/modules/basics.c index 36f88becbe..c5b5565686 100644 --- a/tests/modules/basics.c +++ b/tests/modules/basics.c @@ -669,6 +669,24 @@ int TestNotifications(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) return ValkeyModule_ReplyWithSimpleString(ctx, "ERR"); } +/* test.latency latency_ms */ +int TestLatency(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { + if (argc != 2) { + ValkeyModule_WrongArity(ctx); + return VALKEYMODULE_OK; + } + + long long latency_ms; + if (ValkeyModule_StringToLongLong(argv[1], &latency_ms) != VALKEYMODULE_OK) { + ValkeyModule_ReplyWithError(ctx, "Invalid integer value"); + return VALKEYMODULE_OK; + } + + ValkeyModule_LatencyAddSample("test", latency_ms); + ValkeyModule_ReplyWithSimpleString(ctx, "OK"); + return VALKEYMODULE_OK; +} + /* TEST.CTXFLAGS -- Test GetContextFlags. */ int TestCtxFlags(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { VALKEYMODULE_NOT_USED(argc); @@ -1048,5 +1066,8 @@ int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int arg TestNotifications,"write deny-oom",1,1,1) == VALKEYMODULE_ERR) return VALKEYMODULE_ERR; + if (ValkeyModule_CreateCommand(ctx, "test.latency", TestLatency, "readonly", 0, 0, 0) == VALKEYMODULE_ERR) + return VALKEYMODULE_ERR; + return VALKEYMODULE_OK; } diff --git a/tests/unit/moduleapi/basics.tcl b/tests/unit/moduleapi/basics.tcl index 733e8e1962..0696e30050 100644 --- a/tests/unit/moduleapi/basics.tcl +++ b/tests/unit/moduleapi/basics.tcl @@ -39,6 +39,23 @@ start_server {tags {"modules"}} { verify_log_message 0 "*Module name is busy*" 0 } + test "test latency" { + r config set latency-monitor-threshold 0 + r latency reset + r test.latency 0 + r test.latency 1 + assert_equal {} [r latency latest] + assert_equal {} [r latency history test] + + r config set latency-monitor-threshold 1 + r test.latency 0 + assert_equal 0 [llength [r latency history test]] + r test.latency 1 + assert_match {*test * 1 1*} [r latency latest] + r test.latency 2 + assert_match {*test * 2 2*} [r latency latest] + } + test "Unload the module - basics" { assert_equal {OK} [r module unload test] } From f645e30d43e10d0696428b0a77aaf0e6b8882bf1 Mon Sep 17 00:00:00 2001 From: Binbin Date: Sat, 11 Jan 2025 10:27:58 +0800 Subject: [PATCH 2/2] use helper function Signed-off-by: Binbin --- src/module.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/module.c b/src/module.c index d256dc7a3d..76c362b758 100644 --- a/src/module.c +++ b/src/module.c @@ -7680,8 +7680,7 @@ void VM__Assert(const char *estr, const char *file, int line) { * command. The call is skipped if the latency is smaller than the configured * latency-monitor-threshold. */ void VM_LatencyAddSample(const char *event, mstime_t latency) { - if (server.latency_monitor_threshold && latency >= server.latency_monitor_threshold) - latencyAddSample(event, latency); + latencyAddSampleIfNeeded(event, latency); } /* --------------------------------------------------------------------------