[2.10] [MOD-11155] Create basic query pause debug command#7624
[2.10] [MOD-11155] Create basic query pause debug command#7624
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| self->remaining--; | ||
| return base->upstream->Next(base->upstream, r); |
There was a problem hiding this comment.
Bug: Unsigned integer underflow after pause in RPPauseAfterCount_Next
In RPPauseAfterCount_Next, when remaining is 0, the function pauses but then unconditionally decrements remaining afterward. Since remaining is a uint32_t, decrementing from 0 causes an unsigned integer underflow, wrapping to UINT32_MAX. This breaks the pause functionality for subsequent calls since remaining will never reach 0 again. The RPTimeoutAfterCount_Next function handles this correctly by returning early after the condition is met without decrementing.
meiravgri
changed the title
| if (internal_only) { | ||
| QueryError_SetError(status, QUERY_EPARSEARGS, "INTERNAL_ONLY is not supported without TIMEOUT_AFTER_N or PAUSE_AFTER_RP_N/PAUSE_BEFORE_RP_N"); | ||
| return REDISMODULE_ERR; | ||
| } |
There was a problem hiding this comment.
Bug: Missing return statement causes undefined behavior
The parseAndCompileDebug function has a missing return statement at the end. When no debug parameters like TIMEOUT_AFTER_N or PAUSE_*_RP_N are provided, and internal_only is false, the function falls through without returning a value. Since the function returns int, this causes undefined behavior and the caller will receive garbage or an unpredictable value instead of REDISMODULE_OK.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.10 #7624 +/- ##
==========================================
+ Coverage 89.40% 89.41% +0.01%
==========================================
Files 210 210
Lines 35841 35992 +151
==========================================
+ Hits 32043 32183 +140
- Misses 3798 3809 +11
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.10] [MOD-11155] Create basic query pause debug command #7624 * fix test * remove empty line
| } | ||
|
|
||
| bool before = AC_IsInitialized(&pauseBeforeArgs); | ||
| ArgsCursor *pauseArgs = before ? &pauseBeforeArgs : &pauseAfterArgs; |
There was a problem hiding this comment.
Bug: Second pause argument silently ignored when both provided
When both PAUSE_BEFORE_RP_N and PAUSE_AFTER_RP_N are provided in the same command, the condition at line 124 uses || to enter the block if either is initialized, but the logic inside only processes one of them. If pauseBeforeArgs is initialized, it takes priority and pauseAfterArgs is completely ignored without any error or warning. User-provided arguments for the second pause point are silently discarded, which can lead to unexpected behavior where the user believes two pause points were created but only one actually exists.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
backport #6750 to 2.10
Note
Adds a pause-capable debug result processor and QUERY_CONTROLLER commands, and extends _FT.DEBUG to pause before/after specific RP types with INTERNAL_ONLY handling.
QUERY_CONTROLLERsubcommands:SET_PAUSE_RP_RESUME,GET_IS_RP_PAUSED,PRINT_RP_STREAM.QueryDebugCtx(global) to track pause state and current debug RP.RP_PAUSEthat pauses after N results; single active debug RP enforced.StringToRPType.ResultProcessorTypeto separate normal vs debug RPs.PAUSE_BEFORE_RP_N <RP_TYPE> <N> [INTERNAL_ONLY]andPAUSE_AFTER_RP_N <RP_TYPE> <N> [INTERNAL_ONLY].INTERNAL_ONLY.INTERNAL_ONLYusage validation for all debug args.Written by Cursor Bugbot for commit 3243098. This will update automatically on new commits. Configure here.