Skip to content

Align Search APIs and behaviour with Redis 8.0+#4173

Merged
ggivo merged 9 commits intomasterfrom
imalinovskyi_keyless_command_execution
Mar 26, 2026
Merged

Align Search APIs and behaviour with Redis 8.0+#4173
ggivo merged 9 commits intomasterfrom
imalinovskyi_keyless_command_execution

Conversation

@uglide
Copy link
Copy Markdown
Contributor

@uglide uglide commented Jun 6, 2025

  • Introduce AggregateIterator for proper Cluster support
  • Deprecate APIs that rely on legacy RediSeach behaviour

Based on #4411


Note

Medium Risk
Touches client-side cursor iteration and connection-provider behavior; correctness depends on proper node selection and cursor lifecycle handling in both standalone and cluster deployments.

Overview
Introduces AggregateIterator, a new cursor-based iterator for FT.AGGREGATE that keeps cursor reads/deletes on the same Redis node and borrows/returns pooled connections per batch to avoid pool exhaustion/leaks; UnifiedJedis and RediSearchCommands gain ftAggregateIterator(...) to expose it.

Deprecates legacy iteration/cursor APIs (FtAggregateIteration, FtSearchIteration and UnifiedJedis.ftSearchIteration(...), plus ftCursorRead/ftCursorDel in RediSearchCommands) and extends AggregationBuilder with cursor batch size tracking plus AggregationResult.isEmpty() to support iterator semantics; adds standalone+cluster integration tests and unit tests, and fixes PooledConnectionProvider.getPrimaryNodesConnectionMap() to return the pool rather than borrowing a connection.

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

@uglide uglide force-pushed the imalinovskyi_keyless_command_execution branch from 85ff14f to f85344a Compare June 12, 2025 12:23
@uglide uglide changed the title Introduce keyless-commands execution @uglide Align Search APIs and behaviour with Redis 8.0+ Jun 12, 2025
@uglide uglide changed the title @uglide Align Search APIs and behaviour with Redis 8.0+ Align Search APIs and behaviour with Redis 8.0+ Jun 12, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 13, 2025

Test Results

   312 files  + 4     312 suites  +4   11m 55s ⏱️ -1s
11 180 tests +44  11 124 ✅ +44  56 💤 ±0  0 ❌ ±0 
 5 877 runs  +26   5 868 ✅ +26   9 💤 ±0  0 ❌ ±0 

Results for commit 464d231. ± Comparison against base commit 6d3ac4e.

♻️ This comment has been updated with latest results.

@uglide uglide requested review from Copilot and ggivo June 13, 2025 09:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aligns various Redis search APIs with Redis 8.0+ policies by introducing a new keyless command execution method, implementing round‑robin distribution for keyless commands in cluster mode, and replacing legacy search iteration APIs with a more robust AggregateIterator. Key changes include the introduction of executeKeylessCommand, removal of JedisBroadcastAndRoundRobinConfig, and the implementation of round‑robin scheduling in ClusterCommandExecutor.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test/java/redis/clients/jedis/modules/search/AggregateIteratorTest.java New tests for AggregateIterator covering basic use, cursor removal and error cases
src/main/java/redis/clients/jedis/search/aggr/AggregationBuilder.java Adds cursorCount tracking when using cursor-based aggregation
src/main/java/redis/clients/jedis/search/aggr/AggregateIterator.java Implements a new iterator for aggregation results with proper cursor management
src/main/java/redis/clients/jedis/executors/ClusterCommandExecutor.java Introduces round‑robin distribution for keyless command execution
src/main/java/redis/clients/jedis/UnifiedJedis.java Updates search commands to use the new executeKeylessCommand method and deprecates legacy APIs
src/main/java/redis/clients/jedis/CommandObjects.java Adjusts round‑robin command routing logic and removes unused configuration
Comments suppressed due to low confidence (1)

src/main/java/redis/clients/jedis/CommandObjects.java:3443

  • [nitpick] Add a comment explaining why certain SearchCommands (SUGGET, SUGADD, SUGLEN, SUGDEL, CURSOR) are excluded from round-robin routing to aid future maintainers.
private boolean isRoundRobinSearchCommand(SearchCommand sc) {

Comment thread src/main/java/redis/clients/jedis/executors/ClusterCommandExecutor.java Outdated
@ggivo ggivo added the breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes. label Jun 16, 2025
@ggivo ggivo added this to the 7.0.0 milestone Jun 16, 2025
@ggivo ggivo modified the milestones: 7.0.0, 8.0.0 Oct 10, 2025
@uglide uglide force-pushed the imalinovskyi_keyless_command_execution branch 3 times, most recently from b03ef67 to 3e38aa4 Compare October 20, 2025 11:05
@uglide uglide force-pushed the imalinovskyi_keyless_command_execution branch from 3e38aa4 to 12b7e92 Compare November 5, 2025 12:46
@a-TODO-rov a-TODO-rov force-pushed the imalinovskyi_keyless_command_execution branch from 12b7e92 to 57b101b Compare January 9, 2026 10:17
@uglide uglide force-pushed the imalinovskyi_keyless_command_execution branch from 09f3333 to 666f77c Compare January 13, 2026 13:26
Copy link
Copy Markdown
Collaborator

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

Looks good.
Added some comments that can be addressed later.
And also sharing a concern around sticky connection management, which will lock the connection from the pool for the time of iteration.

Comment thread src/main/java/redis/clients/jedis/UnifiedJedis.java
Comment thread src/main/java/redis/clients/jedis/CommandObjects.java Outdated
Comment thread src/main/java/redis/clients/jedis/executors/ClusterCommandExecutor.java Outdated
Comment thread src/main/java/redis/clients/jedis/executors/ClusterCommandExecutor.java Outdated
Comment thread src/main/java/redis/clients/jedis/executors/ClusterCommandExecutor.java Outdated
Comment thread src/main/java/redis/clients/jedis/search/aggr/AggregateIterator.java Outdated
Comment thread src/main/java/redis/clients/jedis/search/aggr/AggregateIterator.java Outdated
@uglide uglide force-pushed the imalinovskyi_keyless_command_execution branch from 666f77c to b27d847 Compare January 21, 2026 16:45
@uglide uglide force-pushed the imalinovskyi_keyless_command_execution branch from b27d847 to 3439e69 Compare March 12, 2026 10:57
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Mar 12, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Comment thread src/main/java/redis/clients/jedis/CommandArguments.java
Comment thread src/main/java/redis/clients/jedis/UnifiedJedis.java
Comment thread src/main/java/redis/clients/jedis/CommandArguments.java
Comment thread src/main/java/redis/clients/jedis/search/aggr/AggregateIterator.java Outdated
Comment thread src/main/java/redis/clients/jedis/search/aggr/AggregateIterator.java Outdated
@uglide uglide mentioned this pull request Mar 12, 2026
Copy link
Copy Markdown
Collaborator

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

LGTM,

There are comments by cursor that seems ligitime

Comment thread src/main/java/redis/clients/jedis/search/aggr/AggregateIterator.java Outdated
Comment thread src/main/java/redis/clients/jedis/UnifiedJedis.java
Comment thread src/main/java/redis/clients/jedis/UnifiedJedis.java Outdated
Comment thread src/main/java/redis/clients/jedis/search/FtSearchIteration.java
- Introduce AggregateIterator for proper Cluster support
- Deprecate APIs that rely on legacy RediSeach behaviour
@uglide uglide force-pushed the imalinovskyi_keyless_command_execution branch from 707287a to 5c40f91 Compare March 25, 2026 10:40
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Copy link
Copy Markdown
Collaborator

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

LGTM

@ggivo ggivo merged commit 83a6d81 into master Mar 26, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants