Skip to content

[8.4] Register Cursor Sub-Commands as such - [MOD-12807, MOD-12808]#7611

Closed
redisearch-backport-pull-request[bot] wants to merge 1 commit into8.4from
backport-7571-to-8.4
Closed

[8.4] Register Cursor Sub-Commands as such - [MOD-12807, MOD-12808]#7611
redisearch-backport-pull-request[bot] wants to merge 1 commit into8.4from
backport-7571-to-8.4

Conversation

@redisearch-backport-pull-request
Copy link
Contributor

@redisearch-backport-pull-request redisearch-backport-pull-request bot commented Dec 2, 2025

Description

Backport of #7571 to 8.4.


Note

Registers FT.CURSOR subcommands (READ/PROFILE/DEL/GC), refactors cursor reply handling and coordinator routing, and updates tests and metrics accordingly.

  • Commands & Routing:
    • Register FT.CURSOR subcommands: READ, PROFILE, DEL, GC (external and internal _FT.CURSOR).
    • Replace monolithic RSCursorCommand with dedicated handlers: RSCursorReadCommand, RSCursorProfileCommand, RSCursorDelCommand, RSCursorGCCommand.
    • Integrate subcommand-aware coordinator distribution (CursorCommand wrapper and per-subcommand internals).
  • Reply/Execution Refactor:
    • Change sendChunk_ReplyOnly_EmptyResults to accept RedisModuleCtx * and self-create RedisModule_Reply.
    • Update cursor read path to use RedisModuleCtx and manage reply lifecycle (cursorRead, bg variant, error on dropped index).
  • Behavioral Tweaks:
    • FT.CURSOR GC now aggregates local/coord results and returns -1 only when both quick-checks return -1.
    • Error messages/arity validation aligned per subcommand; commandstats now reported per subcommand (e.g., cmdstat__FT.CURSOR|READ).
  • Registration Changes:
    • Register subcommands for both single-shard and coordinator (RediSearch_InitModuleInternal, RedisModule_OnLoad).
  • Tests:
    • Adjust existing tests for subcommand semantics and errors; add coverage for PROFILE, GC edge-cases, dropped-index during cursor read/profile, and updated commandstats keys.

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

* split cursor command

* fix and improve tests

* cover error cases

* fix cursor leaks

* Add "TODO: run hybrid cursor" back

* remove new empty line

* small test improvement

* fix FT.CURSOR GC

* de-flake test

* make CURSOR PROFILE internal only

(cherry picked from commit 6945fd6)
}

static int CursorCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
static inline int CursorCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, RedisModuleCmdFunc subcmd, ConcurrentCmdHandler dist_callback) {
Copy link

Choose a reason for hiding this comment

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

Bug: GC subcommand incorrectly requires 4 arguments in cluster mode

The CursorCommand wrapper, used for distributed cursor subcommands, incorrectly checks argc < 4. This prevents FT.CURSOR GC {index} from working in cluster mode, as it only requires 3 arguments. The internal RSCursorGCCommand correctly handles 3 arguments, but the wrapper rejects valid calls.

Fix in Cursor Fix in Web

@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

❌ Patch coverage is 97.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.93%. Comparing base (4d9f181) to head (8931a09).
⚠️ Report is 11 commits behind head on 8.4.

Files with missing lines Patch % Lines
src/aggregate/aggregate_exec.c 98.57% 1 Missing ⚠️
src/module.c 94.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              8.4    #7611      +/-   ##
==========================================
+ Coverage   85.91%   85.93%   +0.01%     
==========================================
  Files         331      331              
  Lines       52792    52805      +13     
  Branches    12004    12004              
==========================================
+ Hits        45357    45377      +20     
+ Misses       7268     7261       -7     
  Partials      167      167              
Flag Coverage Δ
flow 84.50% <97.77%> (-0.34%) ⬇️
unit 52.58% <5.55%> (-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.

@GuyAv46 GuyAv46 closed this Dec 7, 2025
auto-merge was automatically disabled December 7, 2025 13:06

Pull request was closed

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.

1 participant