Skip to content

Commit

Permalink
Fix module LatencyAddSample still work when latency-monitor-threshold…
Browse files Browse the repository at this point in the history
… 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 <[email protected]>
  • Loading branch information
enjoy-binbin committed Jan 10, 2025
1 parent e60990e commit c13ce11
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/* --------------------------------------------------------------------------
Expand Down
21 changes: 21 additions & 0 deletions tests/modules/basics.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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", 1, 1, 1) == VALKEYMODULE_ERR)
return VALKEYMODULE_ERR;

return VALKEYMODULE_OK;
}
15 changes: 15 additions & 0 deletions tests/unit/moduleapi/basics.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@ 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
r test.latency 1
r test.latency 2
assert_equal 1 [llength [r latency history test]]
}

test "Unload the module - basics" {
assert_equal {OK} [r module unload test]
}
Expand Down

0 comments on commit c13ce11

Please sign in to comment.