Skip to content

Commit 8e0b375

Browse files
authored
Fix cluster slot stats for scripts with cross-slot keys (#2835)
This commit fixes the cluster slot stats for scripts executed by scripting engines when the scripts access cross-slot keys. This was not a bug in Lua scripting engine, but `VM_Call` was missing a call to invalidate the script caller client slot to prevent the accumulation of stats. Signed-off-by: Ricardo Dias <[email protected]>
1 parent 01a7657 commit 8e0b375

File tree

5 files changed

+12
-11
lines changed

5 files changed

+12
-11
lines changed

src/cluster_slot_stats.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -226,14 +226,6 @@ void clusterSlotStatsAddCpuDuration(client *c, ustime_t duration) {
226226
server.cluster->slot_stats[c->slot].cpu_usec += duration;
227227
}
228228

229-
/* For cross-slot scripting, its caller client's slot must be invalidated,
230-
* such that its slot-stats aggregation is bypassed. */
231-
void clusterSlotStatsInvalidateSlotIfApplicable(scriptRunCtx *ctx) {
232-
if (!(ctx->flags & SCRIPT_ALLOW_CROSS_SLOT)) return;
233-
234-
ctx->original_client->slot = -1;
235-
}
236-
237229
static int canAddNetworkBytesIn(client *c) {
238230
/* First, cluster mode must be enabled.
239231
* Second, command should target a specific slot.

src/cluster_slot_stats.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
#include "server.h"
22
#include "cluster.h"
3-
#include "script.h"
43
#include "cluster_legacy.h"
54

65
/* General use-cases. */
@@ -10,7 +9,6 @@ int clusterSlotStatsEnabled(int slot);
109

1110
/* cpu-usec metric. */
1211
void clusterSlotStatsAddCpuDuration(client *c, ustime_t duration);
13-
void clusterSlotStatsInvalidateSlotIfApplicable(scriptRunCtx *ctx);
1412

1513
/* network-bytes-in metric. */
1614
void clusterSlotStatsAddNetworkBytesInForUserClient(client *c);

src/module.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6773,6 +6773,9 @@ ValkeyModuleCallReply *VM_Call(ValkeyModuleCtx *ctx, const char *cmdname, const
67736773

67746774
if (reply) autoMemoryAdd(ctx, VALKEYMODULE_AM_REPLY, reply);
67756775
if (ctx->module) ctx->module->in_call--;
6776+
if (is_running_script) {
6777+
scriptClusterSlotStatsInvalidateSlotIfApplicable();
6778+
}
67766779
if (c) moduleReleaseTempClient(c);
67776780
return reply;
67786781
}

src/script.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ void scriptCall(scriptRunCtx *run_ctx, sds *err) {
587587
}
588588
call(c, call_flags);
589589
serverAssert(c->flag.blocked == 0);
590-
clusterSlotStatsInvalidateSlotIfApplicable(run_ctx);
590+
scriptClusterSlotStatsInvalidateSlotIfApplicable();
591591
return;
592592

593593
error:
@@ -644,3 +644,10 @@ sds scriptGetRunningEngineName(void) {
644644
serverAssert(scriptIsRunning());
645645
return scriptingEngineGetName(curr_run_ctx->engine);
646646
}
647+
648+
/* For cross-slot scripting, its caller client's slot must be invalidated,
649+
* such that its slot-stats aggregation is bypassed. */
650+
void scriptClusterSlotStatsInvalidateSlotIfApplicable(void) {
651+
if (!scriptAllowsCrossSlot()) return;
652+
curr_run_ctx->original_client->slot = -1;
653+
}

src/script.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ int scriptAllowsCrossSlot(void);
127127
int scriptGetSlot(void);
128128
void scriptSetSlot(int slot);
129129
void scriptSetOriginalClientSlot(int slot);
130+
void scriptClusterSlotStatsInvalidateSlotIfApplicable(void);
130131

131132
sds scriptGetRunningEngineName(void);
132133

0 commit comments

Comments
 (0)