Skip to content

Commit 18f6854

Browse files
meiravgrigithub-actions[bot]
meiravgri
authored and
github-actions[bot]
committed
[MOD-12627] Add Debug Support for FT.PROFILE Command (#7510)
* imp debug profile for SA: introduce in mocule.h: RSProfileCommandImp RSProfileCommand calls RSProfileCommandImp(isDebug = false) for regular execution ProfileCommandCommand_DebugWrapper mcalls it with isDebug=true and skips _FT.DEBUG introduce entrypoint for _FT.DEBUG FT.PROFILE in debug_commands: ProfileCommandCommand_DebugWrapper RSProfileCommandImp calls DEBUG_execCommandCommon is its debug _recursiveProfilePrint skips printing debug RP * pass is debug instead of extracting: module.h: replace declaration: DistAggregateCommand DistSearchCommand with Imp version that receives isDebug expose ProfileCommandHandlerImp align debug_commands introducr _FT.DEBUG _FT.PROFILE * add test for cluster * return res * augi fixes * fix spell check * fix for real * fix test * skip tests according to env * revrt test_profile changes * reove changes from internal_only (cherry picked from commit 00ca3cf)
1 parent 5c03444 commit 18f6854

10 files changed

Lines changed: 309 additions & 22 deletions

File tree

‎src/aggregate/aggregate_debug.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@
7575
* -----------------------------------------------------------------------------
7676
*
7777
* ### Limitations:
78-
* - `_FT.DEBUG` does not support `FT.PROFILE`.
7978
* - Pause debugging affects at most one query at a time (single debug pause RP at once).
8079
*
8180
* -----------------------------------------------------------------------------

‎src/aggregate/aggregate_exec.c‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ typedef struct {
4747
} blockedClientReqCtx;
4848

4949
static void runCursor(RedisModule_Reply *reply, Cursor *cursor, size_t num);
50+
static int DEBUG_execCommandCommon(RedisModuleCtx *ctx, RedisModuleString **argv, int argc,
51+
CommandType type, int execOptions);
52+
typedef int (*execCommandCommonHandler)(RedisModuleCtx *ctx, RedisModuleString **argv, int argc,
53+
CommandType type, int execOptions);
5054

5155
/**
5256
* Get the sorting key of the result. This will be the sorting key of the last
@@ -1138,18 +1142,29 @@ RedisModuleString **_profileArgsDup(RedisModuleString **argv, int argc, int para
11381142
memcpy(newArgv, argv, PROFILE_1ST_PARAM * sizeof(*newArgv));
11391143
// copy non-profile commands
11401144
memcpy(newArgv + PROFILE_1ST_PARAM, argv + PROFILE_1ST_PARAM + params,
1141-
(argc - PROFILE_1ST_PARAM - params) * sizeof(*newArgv));
1145+
(argc - PROFILE_1ST_PARAM - params) * sizeof(*newArgv));
11421146
return newArgv;
11431147
}
11441148

11451149
int RSProfileCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
1150+
return RSProfileCommandImp(ctx, argv, argc, false);
1151+
}
1152+
1153+
int RSProfileCommandImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug) {
11461154
if (argc < 5) {
11471155
return RedisModule_WrongArity(ctx);
11481156
}
11491157

11501158
CommandType cmdType;
11511159
int curArg = PROFILE_1ST_PARAM;
11521160
int withProfile = EXEC_WITH_PROFILE;
1161+
execCommandCommonHandler execCommandHandlerFunc = execCommandCommon;
1162+
1163+
// Check if this is a debug command
1164+
if (isDebug) {
1165+
execCommandHandlerFunc = DEBUG_execCommandCommon;
1166+
withProfile |= EXEC_DEBUG;
1167+
}
11531168

11541169
// Check the command type
11551170
const char *cmd = RedisModule_StringPtrLen(argv[curArg++], NULL);
@@ -1175,7 +1190,9 @@ int RSProfileCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
11751190

11761191
int newArgc = argc - curArg + PROFILE_1ST_PARAM;
11771192
RedisModuleString **newArgv = _profileArgsDup(argv, argc, curArg - PROFILE_1ST_PARAM);
1178-
execCommandCommon(ctx, newArgv, newArgc, cmdType, withProfile);
1193+
1194+
execCommandHandlerFunc(ctx, newArgv, newArgc, cmdType, withProfile);
1195+
11791196
rm_free(newArgv);
11801197
return REDISMODULE_OK;
11811198
}

‎src/debug_commands.c‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,7 +1491,7 @@ DEBUG_COMMAND(DistSearchCommand_DebugWrapper) {
14911491
return DEBUG_RSSearchCommand(ctx, ++argv, --argc);
14921492
}
14931493

1494-
return DistSearchCommand(ctx, argv, argc);
1494+
return DistSearchCommandImp(ctx, ++argv, --argc, true);
14951495
}
14961496

14971497
DEBUG_COMMAND(DistAggregateCommand_DebugWrapper) {
@@ -1509,7 +1509,7 @@ DEBUG_COMMAND(DistAggregateCommand_DebugWrapper) {
15091509
return DEBUG_RSAggregateCommand(ctx, ++argv, --argc);
15101510
}
15111511

1512-
return DistAggregateCommand(ctx, argv, argc);
1512+
return DistAggregateCommandImp(ctx, ++argv, --argc, true);
15131513
}
15141514

15151515
DEBUG_COMMAND(RSSearchCommandShard) {
@@ -1526,6 +1526,27 @@ DEBUG_COMMAND(RSAggregateCommandShard) {
15261526
return DEBUG_RSAggregateCommand(ctx, ++argv, --argc);
15271527
}
15281528

1529+
DEBUG_COMMAND(RSProfileCommandShard) {
1530+
if (!debugCommandsEnabled(ctx)) {
1531+
return RedisModule_ReplyWithError(ctx, NODEBUG_ERR);
1532+
}
1533+
return RSProfileCommandImp(ctx, ++argv, --argc, true);
1534+
}
1535+
1536+
DEBUG_COMMAND(ProfileCommandCommand_DebugWrapper) {
1537+
if (!debugCommandsEnabled(ctx)) {
1538+
return RedisModule_ReplyWithError(ctx, NODEBUG_ERR);
1539+
}
1540+
1541+
// at least one debug_param should be provided
1542+
// (1)_FT.DEBUG (2) FT.PROFILE (3) <index> (4) SEARCH | AGGREGATE [LIMITED] (6) QUERY <query> [query_options] (5) debug_params (6)DEBUG_PARAMS_COUNT (7) <debug_params_count>
1543+
if (argc < 7) {
1544+
return RedisModule_WrongArity(ctx);
1545+
}
1546+
1547+
return ProfileCommandHandlerImp(ctx, ++argv, --argc, true);
1548+
}
1549+
15291550
DEBUG_COMMAND(HybridCommand_DebugWrapper) {
15301551
if (!debugCommandsEnabled(ctx)) {
15311552
return RedisModule_ReplyWithError(ctx, NODEBUG_ERR);
@@ -2086,6 +2107,8 @@ DebugCommandType commands[] = {{"DUMP_INVIDX", DumpInvertedIndex}, // Print all
20862107
{"_FT.SEARCH", RSSearchCommandShard}, // internal use only, in SA use FT.SEARCH
20872108
{"FT.HYBRID", HybridCommand_DebugWrapper},
20882109
{"_FT.HYBRID", HybridCommand_DebugWrapper}, // internal use only, in SA use FT.HYBRID
2110+
{"FT.PROFILE", ProfileCommandCommand_DebugWrapper},
2111+
{"_FT.PROFILE", RSProfileCommandShard},
20892112
/* IMPORTANT NOTE: Every debug command starts with
20902113
* checking if redis allows this context to execute
20912114
* debug commands by calling `debugCommandsEnabled(ctx)`.

‎src/module.c‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3230,6 +3230,10 @@ void DEBUG_RSExecDistAggregate(RedisModuleCtx *ctx, RedisModuleString **argv, in
32303230
struct ConcurrentCmdCtx *cmdCtx);
32313231

32323232
int DistAggregateCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
3233+
return DistAggregateCommandImp(ctx, argv, argc, false);
3234+
}
3235+
3236+
int DistAggregateCommandImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug) {
32333237
if (NumShards == 0) {
32343238
return RedisModule_ReplyWithError(ctx, CLUSTERDOWN_ERR);
32353239
} else if (argc < 3) {
@@ -3256,10 +3260,7 @@ int DistAggregateCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc
32563260
// Coord callback
32573261
ConcurrentCmdHandler dist_callback = RSExecDistAggregate;
32583262

3259-
bool isDebug = (RMUtil_ArgIndex("_FT.DEBUG", argv, 1) != -1);
32603263
if (isDebug) {
3261-
argv++;
3262-
argc--;
32633264
dist_callback = DEBUG_RSExecDistAggregate;
32643265
}
32653266

@@ -3639,6 +3640,10 @@ static int DistSearchUnblockClient(RedisModuleCtx *ctx, RedisModuleString **argv
36393640
int RSSearchCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
36403641

36413642
int DistSearchCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
3643+
return DistSearchCommandImp(ctx, argv, argc, false);
3644+
}
3645+
3646+
int DistSearchCommandImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug) {
36423647
if (NumShards == 0) {
36433648
return RedisModule_ReplyWithError(ctx, CLUSTERDOWN_ERR);
36443649
} else if (argc < 3) {
@@ -3664,10 +3669,7 @@ int DistSearchCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
36643669
// Coord callback
36653670
void (*dist_callback)(void *) = DistSearchCommandHandler;
36663671

3667-
bool isDebug = (RMUtil_ArgIndex("_FT.DEBUG", argv, 1) != -1);
36683672
if (isDebug) {
3669-
argv++;
3670-
argc--;
36713673
dist_callback = DEBUG_DistSearchCommandHandler;
36723674
}
36733675

@@ -3714,8 +3716,11 @@ int DistSearchCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
37143716
return REDISMODULE_OK;
37153717
}
37163718

3717-
int RSProfileCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
37183719
int ProfileCommandHandler(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
3720+
return ProfileCommandHandlerImp(ctx, argv, argc, false);
3721+
}
3722+
3723+
int ProfileCommandHandlerImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug) {
37193724
if (argc < 5) {
37203725
return RedisModule_WrongArity(ctx);
37213726
}
@@ -3730,14 +3735,14 @@ int ProfileCommandHandler(RedisModuleCtx *ctx, RedisModuleString **argv, int arg
37303735
// There is only one shard in the cluster. We can handle the command locally.
37313736
// We must first check that we don't have a cursor, as the local command handler allows cursors
37323737
// for multi-shard clusters support.
3733-
return RSProfileCommand(ctx, argv, argc);
3738+
return RSProfileCommandImp(ctx, argv, argc, isDebug);
37343739
}
37353740

37363741
if (RMUtil_ArgExists("SEARCH", argv, 3, 2)) {
3737-
return DistSearchCommand(ctx, argv, argc);
3742+
return DistSearchCommandImp(ctx, argv, argc, isDebug);
37383743
}
37393744
if (RMUtil_ArgExists("AGGREGATE", argv, 3, 2)) {
3740-
return DistAggregateCommand(ctx, argv, argc);
3745+
return DistAggregateCommandImp(ctx, argv, argc, isDebug);
37413746
}
37423747
return RedisModule_ReplyWithError(ctx, "No `SEARCH` or `AGGREGATE` provided");
37433748
}

‎src/module.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,10 @@ void SpecialCaseCtx_Free(specialCaseCtx* ctx);
119119

120120
void processResultFormat(uint32_t *flags, MRReply *map);
121121

122-
int DistAggregateCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
123-
int DistSearchCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
122+
int DistAggregateCommandImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug);
123+
int DistSearchCommandImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug);
124+
int RSProfileCommandImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug);
125+
int ProfileCommandHandlerImp(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, bool isDebug);
124126

125127
void ScheduleContextCleanup(RedisModuleCtx *thctx, struct RedisSearchCtx *sctx);
126128

‎src/profile.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ static double _recursiveProfilePrint(RedisModule_Reply *reply, ResultProcessor *
7676
}
7777
double upstreamTime = _recursiveProfilePrint(reply, rp->upstream, printProfileClock);
7878

79+
if (rp->type > RP_MAX) {
80+
RS_LOG_ASSERT_FMT(rp->type < RP_MAX_DEBUG, "RPType error, type: %d", rp->type);
81+
return upstreamTime;
82+
}
83+
7984
// Array is filled backward in pair of [common, profile] result processors
8085
if (rp->type != RP_PROFILE) {
8186
RedisModule_Reply_Map(reply); // start of recursive map
@@ -106,15 +111,13 @@ static double _recursiveProfilePrint(RedisModule_Reply *reply, ResultProcessor *
106111
printProfileGILTime(rp->GILTime);
107112
break;
108113

109-
case RP_PROFILE:
110-
case RP_MAX:
114+
default:
111115
RS_ABORT("RPType error");
112116
break;
113117
}
114118

115119
return upstreamTime;
116120
}
117-
118121
double totalRPTime = rs_wall_clock_convert_ns_to_ms_d(RPProfile_GetClock(rp));
119122
if (printProfileClock) {
120123
printProfileTime(totalRPTime - upstreamTime);

‎src/result_processor.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1984,7 +1984,7 @@ static int RPHybridMerger_Yield(ResultProcessor *rp, SearchResult *r) {
19841984
}
19851985

19861986
/* Create a new Hybrid Merger processor */
1987-
ResultProcessor *RPHybridMerger_New(RedisSearchCtx *sctx,
1987+
ResultProcessor *RPHybridMerger_New(RedisSearchCtx *sctx,
19881988
HybridScoringContext *hybridScoringCtx,
19891989
ResultProcessor **upstreams,
19901990
size_t numUpstreams,

‎src/result_processor.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ typedef enum {
7676
RP_TIMEOUT,
7777
RP_CRASH,
7878
RP_PAUSE,
79+
RP_MAX_DEBUG
7980
} ResultProcessorType;
8081

8182
struct ResultProcessor;
@@ -308,7 +309,7 @@ StrongRef DepleterSync_New(unsigned int num_depleters, bool take_index_lock);
308309
* Note: RPHybridMerger takes ownership of hybridScoringCtx and is responsible for freeing it.
309310
* @param scoreKey Optional key for writing scores as fields when no LOAD step is provided
310311
*/
311-
ResultProcessor *RPHybridMerger_New(RedisSearchCtx *sctx,
312+
ResultProcessor *RPHybridMerger_New(RedisSearchCtx *sctx,
312313
HybridScoringContext *hybridScoringCtx,
313314
ResultProcessor **upstreams,
314315
size_t numUpstreams,

‎tests/pytests/common.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,17 @@ def toSortedFlatList(res):
139139
return py2sorted(finalList)
140140
return [res]
141141

142+
def countFlatElements(arr):
143+
"""Count elements without sorting (lighter than toSortedFlatList)"""
144+
if isinstance(arr, str):
145+
return 1
146+
if isinstance(arr, Iterable):
147+
count = 0
148+
for e in arr:
149+
count += countFlatElements(e)
150+
return count
151+
return 1
152+
142153
def assertInfoField(env, idx, field, expected, delta=None):
143154
d = index_info(env, idx)
144155
msg = f"field name: {field}"

0 commit comments

Comments
 (0)