Skip to content

[2.10] [MOD-12414] Add Internal cursor reads metric to cluster FT.PROFILE output#7746

Merged
meiravgri merged 4 commits into2.10from
backport-7709-to-2.10
Dec 11, 2025
Merged

[2.10] [MOD-12414] Add Internal cursor reads metric to cluster FT.PROFILE output#7746
meiravgri merged 4 commits into2.10from
backport-7709-to-2.10

Conversation

@meiravgri
Copy link
Copy Markdown
Collaborator

@meiravgri meiravgri commented Dec 11, 2025

backport #7709 to 2.10


Note

Adds an "Internal cursor reads" counter to FT.PROFILE AGGREGATE outputs (RESP2/3) and accumulates timeout state across cursor reads.

  • Profile/Metrics:
    • Add Internal cursor reads to shard profiles in FT.PROFILE AGGREGATE (RESP2/RESP3); only for internal cursor requests.
    • Accumulate has_timedout across cursor reads and use cumulative value in profile output.
  • Core:
    • Extend AREQ with cursor_reads; increment on each cursor execution (runCursor).
  • Execution:
    • Use req->has_timedout |= ... and propagate to ProfilePrinterCtx.
  • Tests:
    • New tests validate presence and correctness of Internal cursor reads under normal and timeout scenarios for RESP2/RESP3.
    • Adjust profile structure indices in existing tests to match updated output.

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

meiravgri and others added 2 commits December 11, 2025 10:16
…output (#7709)

* add cursor_reads to req struct

update in runCursor

print in Profile_Print if cursor

* Fix: Preserve timeout flag across cursor reads in cluster FT.PROFILE

Use |= instead of = to ensure has_timedout remains true once set,
preventing earlier cursor read warnings from being lost in final output.

* fix test

* fix test

* add message to the test

(cherry picked from commit 005186a)
@meiravgri meiravgri requested a review from GuyAv46 December 11, 2025 11:17
GuyAv46
GuyAv46 previously approved these changes Dec 11, 2025
Comment on lines +257 to +262
// Only internal requests can use profile with cursor.
RS_ASSERT(IsInternal(req));
RedisModule_Reply_Array(reply);
RedisModule_Reply_SimpleString(reply, "Internal cursor reads");
RedisModule_Reply_LongLong(reply, req->cursor_reads);
RedisModule_Reply_ArrayEnd(reply);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤮

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 what can i do

GuyAv46
GuyAv46 previously approved these changes Dec 11, 2025
@meiravgri meiravgri enabled auto-merge December 11, 2025 11:47
@meiravgri meiravgri added this pull request to the merge queue Dec 11, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.43%. Comparing base (4e5b0f4) to head (4dcb8c7).
⚠️ Report is 2 commits behind head on 2.10.

Additional details and impacted files
@@            Coverage Diff             @@
##             2.10    #7746      +/-   ##
==========================================
- Coverage   89.46%   89.43%   -0.03%     
==========================================
  Files         210      210              
  Lines       36087    36138      +51     
==========================================
+ Hits        32284    32319      +35     
- Misses       3803     3819      +16     
Flag Coverage Δ
flow 84.09% <100.00%> (-0.15%) ⬇️
unit 42.08% <0.00%> (-0.05%) ⬇️

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.

Merged via the queue into 2.10 with commit 1911f5b Dec 11, 2025
17 checks passed
@meiravgri meiravgri deleted the backport-7709-to-2.10 branch December 11, 2025 13:52
@meiravgri
Copy link
Copy Markdown
Collaborator Author

/backport

@redisearch-backport-pull-request
Copy link
Copy Markdown
Contributor

Backport failed for 2.8, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.8
git worktree add -d .worktree/backport-7746-to-2.8 origin/2.8
cd .worktree/backport-7746-to-2.8
git switch --create backport-7746-to-2.8
git cherry-pick -x 1911f5b89a740627ec5aeebd9153bb4b3249af01

meiravgri added a commit that referenced this pull request Dec 11, 2025
…ROFILE output (#7746)

* [MOD-12414] Add `Internal cursor reads` metric to cluster FT.PROFILE output (#7709)

* add cursor_reads to req struct

update in runCursor

print in Profile_Print if cursor

* Fix: Preserve timeout flag across cursor reads in cluster FT.PROFILE

Use |= instead of = to ensure has_timedout remains true once set,
preventing earlier cursor read warnings from being lost in final output.

* fix test

* fix test

* add message to the test

(cherry picked from commit 005186a)

* fix test

* fix test

* fix test

(cherry picked from commit 1911f5b)
github-merge-queue bot pushed a commit that referenced this pull request Dec 14, 2025
…OFILE output (#7751)

* cp master

[MOD-12414] Add `Internal cursor reads` metric to cluster FT.PROFILE output (#7709)

* add cursor_reads to req struct

update in runCursor

print in Profile_Print if cursor

* Fix: Preserve timeout flag across cursor reads in cluster FT.PROFILE

Use |= instead of = to ensure has_timedout remains true once set,
preventing earlier cursor read warnings from being lost in final output.

* fix test

* fix test

* add message to the test

(cherry picked from commit 005186a)

* [2.10] [MOD-12414] Add `Internal cursor reads` metric to cluster FT.PROFILE output (#7746)

* [MOD-12414] Add `Internal cursor reads` metric to cluster FT.PROFILE output (#7709)

* add cursor_reads to req struct

update in runCursor

print in Profile_Print if cursor

* Fix: Preserve timeout flag across cursor reads in cluster FT.PROFILE

Use |= instead of = to ensure has_timedout remains true once set,
preventing earlier cursor read warnings from being lost in final output.

* fix test

* fix test

* add message to the test

(cherry picked from commit 005186a)

* fix test

* fix test

* fix test

(cherry picked from commit 1911f5b)

* minimal test for timeout

* remove new code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants