[2.8] [MOD-11155] Create basic query pause debug command#7653
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| return REDISMODULE_ERR; | ||
| } | ||
| return REDISMODULE_OK; | ||
| } |
There was a problem hiding this comment.
Bug: Missing return statement causes undefined behavior
The parseAndCompileDebug function is missing a return statement at the end. If none of the debug argument types (TIMEOUT_AFTER_N, PAUSE_AFTER_RP_N, PAUSE_BEFORE_RP_N) are initialized after successful parsing, the function falls through without returning a value. Since the function returns int, this leads to undefined behavior.
| } | ||
|
|
||
| self->remaining--; | ||
| return base->upstream->Next(base->upstream, r); |
There was a problem hiding this comment.
Bug: Unsigned counter underflow after pause causes incorrect behavior
In RPPauseAfterCount_Next, when remaining reaches 0 and the pause occurs, the function then unconditionally decrements remaining on line 1274. Since remaining is a uint32_t, this causes an integer underflow from 0 to 4294967295. The pause will only trigger once and never again until this maximum value is exhausted by subsequent calls. The decrement at line 1274 appears intended to run only when remaining is greater than 0, not after the pause completes.
meiravgri
changed the title
src/result_processor.c
Outdated
| * the Server Side Public License v1 (SSPLv1). | ||
| */ | ||
|
|
||
|
|
| // (rpidnext for SA) | ||
| // Take this into account when adding more debug types that are modifying the rp pipeline. | ||
| PipelineAddTimeoutAfterCount(&debug_req->r, results_count); | ||
| return REDISMODULE_OK; |
There was a problem hiding this comment.
Bug: Multiple debug parameters silently ignored
When users specify multiple debug parameters in the same command (e.g., TIMEOUT_AFTER_N with a pause parameter, or both PAUSE_BEFORE_RP_N and PAUSE_AFTER_RP_N), only one parameter is processed and the others are silently ignored. The early return after handling TIMEOUT_AFTER_N skips any pause parameters. Similarly, when both pause parameters are specified, the || condition enters the block but only processes whichever parameter the before variable selects, ignoring the other. Users receive no error or warning that their parameters were not applied.
Additional Locations (1)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.8 #7653 +/- ##
==========================================
+ Coverage 82.15% 87.55% +5.40%
==========================================
Files 203 203
Lines 34954 35102 +148
==========================================
+ Hits 28715 30734 +2019
+ Misses 6239 4368 -1871
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
backport #7624 to 2.8
Note: This version doesn't implement support for query debug on cluster env.
Note
Introduces pause-after-N debug processors (before/after specific RP), plus a QUERY_CONTROLLER to resume and inspect paused queries, with supporting APIs and tests.
PAUSE_BEFORE_RP_N <RP_TYPE> <N>andPAUSE_AFTER_RP_N <RP_TYPE> <N>parsing insrc/aggregate/aggregate_debug.c(workers-only), callingPipelineAddPauseRPcount.RP_PAUSE) insrc/result_processor.{c,h}withRPPauseAfterCount_*, insertion helpers (before/after targetRP_TYPE), andStringToRPType.ResultProcessorTypeenum to separate non-debug (RP_MAX) from debug types.QueryDebugCtxtoDebugCTX(src/debug_commands.{c,h}) with pause state and current debug RP tracking.FT.DEBUG QUERY_CONTROLLERsubcommands:SET_PAUSE_RP_RESUME(resume paused query)GET_IS_RP_PAUSED(returns pause status)PRINT_RP_STREAM(lists RP stream; showsDEBUG_RP).QUERY_CONTROLLERin debug command table.src/aggregate/aggregate_debug.h.Written by Cursor Bugbot for commit 5e1e744. This will update automatically on new commits. Configure here.