Skip to content

[8.4] [MOD-12640] Handle warnings in empty FT.AGGREGATE replies (cluster) #7872

Merged
meiravgri merged 1 commit into8.4from
backport-7855-to-8.4
Dec 24, 2025
Merged

[8.4] [MOD-12640] Handle warnings in empty FT.AGGREGATE replies (cluster) #7872
meiravgri merged 1 commit into8.4from
backport-7855-to-8.4

Conversation

@meiravgri
Copy link
Collaborator

@meiravgri meiravgri commented Dec 23, 2025

backport #7855 to 8.4


Note

Implements reliable coordinator handling of shard warnings when replies are empty in cluster FT.AGGREGATE.

  • Add processWarningsAndCleanup in rpnet.c; invoked on empty shard replies and at batch end to extract warning from RESP3 meta, set coordinator warnings (timeout/OOM/max-prefix/indexing OOM), return RS_RESULT_TIMEDOUT when appropriate, free reply, and reset state
  • Call warning processing in getNextReply (empty rows) and rpnetNext (end-of-batch)
  • In aggregate_debug.c, only force coordinator timeout for TIMEOUT_AFTER_N 0 INTERNAL_ONLY under RESP2; RESP3 propagates shard timeout warnings without forcing
  • Update aggregate_debug.h docs to clarify RESP3 vs RESP2 behavior
  • Tests: new test_empty_reply_warnings.py; adjust test_info_modules.py and test_query_oom.py to expect propagated warnings and updated coordinator metrics (RESP3), and verify RESP2 fallback behavior

Written by Cursor Bugbot for commit 5dc9064. This will update automatically on new commits. Configure here.

…7855)

* introduce setWarnings

use at the end of the reply
or when empty reply is recieved

* fixes

* cleanups

fix test

* fix force timeout in debug

* add tests:
testEmptyReplyTimeoutWarningAggregate get stuck if we remove enforcing coord timeout

testEmptyReplyTimeoutWarningProfileAggregate
we dont get warning

* more tests

* move to file

* fix type

* remove comment

* rename test file

* add testEmptyReplyQueryOomWarning
address todos

* trmobr skip duplication

* check waning exact

(cherry picked from commit 701f719)
@codecov
Copy link

codecov bot commented Dec 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.02%. Comparing base (9501de5) to head (5dc9064).
⚠️ Report is 2 commits behind head on 8.4.

Additional details and impacted files
@@            Coverage Diff             @@
##              8.4    #7872      +/-   ##
==========================================
- Coverage   86.07%   86.02%   -0.06%     
==========================================
  Files         331      331              
  Lines       53362    53368       +6     
  Branches    11998    11998              
==========================================
- Hits        45933    45910      -23     
- Misses       7262     7291      +29     
  Partials      167      167              
Flag Coverage Δ
flow 84.64% <100.00%> (-0.18%) ⬇️
unit 52.34% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@meiravgri meiravgri added this pull request to the merge queue Dec 24, 2025
Merged via the queue into 8.4 with commit dd8f901 Dec 24, 2025
32 checks passed
@meiravgri meiravgri deleted the backport-7855-to-8.4 branch December 24, 2025 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants