[MOD-12320] Ensure full profile output on timeout with RETURN policy#7345
Merged
[MOD-12320] Ensure full profile output on timeout with RETURN policy#7345
Conversation
req->has_timedout FT.CURSOR PROFILE while (getNextReply(rpnet)) to printAggProfile expose getNextReply set timeout in netCursor Callabck send ft.cursor profile it timeout
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
getNextReply read profile when cursor is depleted use sendChunk_ReplyOnly_EmptyResults in cursor PROFILE
There was a problem hiding this comment.
Bug: RESP3 Timeout State Tracking Inconsistency
In sendChunk_Resp3, the has_timedout local variable is computed but req->has_timedout is never set, unlike in sendChunk_Resp2 where it's set on line 467. This inconsistency means timeout state won't be properly tracked for RESP3 queries, potentially affecting cursor profile commands that rely on req->has_timedout.
src/aggregate/aggregate_exec.c#L606-L611
RediSearch/src/aggregate/aggregate_exec.c
Lines 606 to 611 in d6e1ef8
…allback netCursorCallback: fix set timeout from reply expose MRIterator_GetChannelSize done check if its not md->rootCommand = C_PROFILE
meiravgri
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7345 +/- ##
==========================================
- Coverage 85.17% 85.15% -0.03%
==========================================
Files 346 346
Lines 53027 53106 +79
Branches 13734 13773 +39
==========================================
+ Hits 45167 45220 +53
- Misses 7666 7692 +26
Partials 194 194
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:
|
meiravgri
changed the title
use it->len for num shards
GuyAv46
previously approved these changes
Nov 18, 2025
GuyAv46
reviewed
Nov 18, 2025
Co-authored-by: GuyAv46 <[email protected]>
GuyAv46
approved these changes
Nov 18, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 tasks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.8
git worktree add -d .worktree/backport-7345-to-2.8 origin/2.8
cd .worktree/backport-7345-to-2.8
git switch --create backport-7345-to-2.8
git cherry-pick -x 38408d7d4623b4a2401874a18f6a044e670a0a87 |
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.10
git worktree add -d .worktree/backport-7345-to-2.10 origin/2.10
cd .worktree/backport-7345-to-2.10
git switch --create backport-7345-to-2.10
git cherry-pick -x 38408d7d4623b4a2401874a18f6a044e670a0a87 |
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 8.2
git worktree add -d .worktree/backport-7345-to-8.2 origin/8.2
cd .worktree/backport-7345-to-8.2
git switch --create backport-7345-to-8.2
git cherry-pick -x 38408d7d4623b4a2401874a18f6a044e670a0a87 |
redisearch-backport-pull-request bot
pushed a commit
that referenced
this pull request
Nov 19, 2025
…7345) * initial imp req->has_timedout FT.CURSOR PROFILE while (getNextReply(rpnet)) to printAggProfile expose getNextReply set timeout in netCursor Callabck send ft.cursor profile it timeout * don't send C_DEL if profile getNextReply read profile when cursor is depleted use sendChunk_ReplyOnly_EmptyResults in cursor PROFILE * sendChunk_ReplyOnly_EmptyResults: move createProfilePrinterCtx to a callback netCursorCallback: fix set timeout from reply expose MRIterator_GetChannelSize done check if its not md->rootCommand = C_PROFILE * cleanups * address review: Cursors_Purge->Cursors_Free free reply netCursorCallback: move timeout check after the profile validation add test_cursor_profile * fix * fix test * address review: use it->len for num shards * take spec ref * remove return statement * Update src/aggregate/aggregate_exec.c Co-authored-by: GuyAv46 <[email protected]> --------- Co-authored-by: GuyAv46 <[email protected]> (cherry picked from commit 38408d7)
Contributor
|
Successfully created backport PR for |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 20, 2025
…olicy (#7431) * [MOD-12320] Ensure full profile output on timeout with RETURN policy (#7345) * initial imp req->has_timedout FT.CURSOR PROFILE while (getNextReply(rpnet)) to printAggProfile expose getNextReply set timeout in netCursor Callabck send ft.cursor profile it timeout * don't send C_DEL if profile getNextReply read profile when cursor is depleted use sendChunk_ReplyOnly_EmptyResults in cursor PROFILE * sendChunk_ReplyOnly_EmptyResults: move createProfilePrinterCtx to a callback netCursorCallback: fix set timeout from reply expose MRIterator_GetChannelSize done check if its not md->rootCommand = C_PROFILE * cleanups * address review: Cursors_Purge->Cursors_Free free reply netCursorCallback: move timeout check after the profile validation add test_cursor_profile * fix * fix test * address review: use it->len for num shards * take spec ref * remove return statement * Update src/aggregate/aggregate_exec.c Co-authored-by: GuyAv46 <[email protected]> --------- Co-authored-by: GuyAv46 <[email protected]> (cherry picked from commit 38408d7) * fix error code --------- Co-authored-by: meiravgri <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: meiravgri <[email protected]>
This was referenced Nov 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Previously, shard profile data was omitted on timeout because it's only sent in the final
CURSOR READresponse. This PR ensures that profile data from all shards is collected and returned even when a timeout occurs during profiling withRETURNpolicy.Implementation: Introduces a new internal
FT.CURSOR PROFILEcommand that is sent to shards (instead ofCURSOR READ) when a timeout occurs with RETURN policy during profiling, allowing the coordinator to retrieve profile data from all shards.Execution Flow
Normal profiling flow (no timeout):
FT.AGGREGATEwith PROFILE to shardsCURSOR READresponsesCURSOR READ(cursor_id=0) includes profile dataNew timeout handling flow:
Shard execution (
sendChunk):req->has_timedout = trueto preserve timeout state for laterFT.CURSOR PROFILEcallCoordinator thread (
rpnetNext→sendChunk):RS_RESULT_TIMEDOUTMRIteratorCallback_SetTimedOut()on iterator context only if detected by coordinator timerUV thread (
netCursorCallback):MRIteratorCallback_SetTimedOut()getCursorCommand()to determine next commandUV thread (
getCursorCommand):CURSOR READ→CURSOR PROFILEFT.CURSOR PROFILE {index} {CID}to shardShard execution (
RSCursorCommandwith PROFILE):has_timedoutstatesendChunk_ReplyOnly_EmptyResults()which usescreateProfilePrinterCtx()Coordinator thread (
printAggProfile):CURSOR PROFILEresponses)Note
Ensures complete profile output on timeout by adding FT.CURSOR PROFILE and wiring timeout propagation across shards, coordinator, and reply handling.
AREQ::has_timedoutand set it during execution; use newcreateProfilePrinterCtx()everywhere profile is printed (incl. empty-results replies).FT.CURSOR PROFILE {index} {CID}to fetch profile after timeout and purge cursor; RESP2/RESP3 supported.CURSOR READtoCURSOR PROFILE(MRRootCommandaddsC_PROFILE).printAggProfiledrains pending replies and logs shard/profile count; uses iterator helpersMRIterator_GetChannelSize()andMRIterator_GetNumShards().getNextReply(RPNet*)for draining.MRIterator_GetChannelSize,MRIterator_GetNumShardsand export them; minor command mutation/reset fixes when switching to PROFILE.Written by Cursor Bugbot for commit f8845f0. This will update automatically on new commits. Configure here.