Improve timeout check for FT.AGGREGATE in cluster mode - [MOD-12434]#7359
Improve timeout check for FT.AGGREGATE in cluster mode - [MOD-12434]#7359
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves timeout handling for FT.AGGREGATE operations in cluster mode. When a shard times out, it may send an empty reply; previously, these empty replies were not added to the query channel. Now, empty replies are allowed to wake up the coordinator thread, which can then check for timeouts before going back to sleep, improving timeout enforcement.
Key Changes:
- Modified callback conditions to allow empty replies to be added to the query channel (lines 133, 143)
- Refactored empty reply validation logic with a structured do-while pattern (lines 268-296)
- Changed log level for empty replies from "warning" to "verbose" (line 302)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 2.10 #7359 +/- ##
==========================================
- Coverage 89.27% 89.26% -0.01%
==========================================
Files 207 207
Lines 35307 35309 +2
==========================================
- Hits 31520 31519 -1
- Misses 3787 3790 +3
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:
|
|
Successfully created backport PR for |
…2434] (#7393) Improve timeout check for FT.AGGREGATE in cluster mode - [MOD-12434] (#7359) * allow empty replies to be pushed to the query channel * fix empty reply check * actually fix empty reply check * address review comment (cherry picked from commit 78329f6) Co-authored-by: GuyAv46 <47632673+GuyAv46@users.noreply.github.com>
Allow empty replies to be added to the query channel
This change makes empty replies received from some shard to be added to the query channel.
It will cause the (maybe) blocked coordinator thread to wake up, check (and discard) the reply, and perform a rutine before waiting for the next reply.
This improves timeout behavior, as in some cases, when a shard times out, it will reply an empty reply to the coordinator. Allowing the empty reply to wake up the coordinator thread will cause it to check the time before going back to sleep, and handle timeouts if the time is already up, improving the timeout enforcement.
Mark if applicable
Note
Empty shard replies are now forwarded (RESP2/RESP3) to wake the coordinator for timeout checks, with protocol-specific emptiness detection and reduced log level.
coord/src/dist_aggregate.c):map["results"]arrays regardless of length.resultsarrays with length >= 1 (was > 1).getNextReplyand discard empty replies after waking coordinator:map["results"]length == 0.rowslength == 1.Written by Cursor Bugbot for commit 031529d. This will update automatically on new commits. Configure here.