Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add IDLE argument to XPENDING command #2046

Open
4 tasks
mp911de opened this issue Apr 19, 2021 · 8 comments · May be fixed by #3116
Open
4 tasks

Add IDLE argument to XPENDING command #2046

mp911de opened this issue Apr 19, 2021 · 8 comments · May be fixed by #3116
Assignees
@SivaTharun
Labels
status: first-timers-only An issue that can only be worked on by brand new contributors theme: Redis 6.2 type: enhancement A general enhancement

Comments

@mp911de
Copy link
Member

mp911de commented Apr 19, 2021 •
edited
Loading

Hi, this is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!

If you have contributed before, consider leaving this one for someone new, and looking through our general ideal-for-contribution issues. Thanks!

Problem

Redis 6.2 introduced a new flag, IDLE to its XPENDING command. We want to support this flag in Spring Data Redis.

Solution

We have XPendingOptions as a collector for additional arguments for the XPENDING command. Extend XPendingOptions (for the imperative API) and PendingRecordsCommand (for the reactive API). Add tests to verify this functionality.

See also https://redis.io/commands/xpending for further information.

Steps to Fix

  • Claim this issue with a comment below and ask any clarifying questions you need
  • Set up a repository locally following the Contributing Guidelines
  • Try to fix the issue following the steps above
  • Commit your changes and start a pull request
@mp911de mp911de added type: enhancement A general enhancement theme: Redis 6.2 status: first-timers-only An issue that can only be worked on by brand new contributors labels Apr 19, 2021
@SivaTharun
Copy link

@mp911de i am a newbie to open source contributions, are you fine, if i start working on this issue.

@mp911de
Copy link
Member Author

mp911de commented May 7, 2021

Thanks for reaching out. Feel free to work on this issue, I assigned it to you. Let us know if you have any questions.

@SivaTharun
Copy link

SivaTharun commented May 19, 2021 •
edited
Loading

@mp911de i have started working on this implementation, i just wanted to ask few questions, before i finalize the approach.

  1. i see that groupName and consumerName are the mandatory arguments for the XPENDING commands, i notice that there is already support for non optional arguments Range and count. And as per the documentation it is an optional argument.
  2. so there should be support for the below set of commands.
    XPENDING <Consumer group Name> <Consume Name> <IDLE TIME MilliSec>

XPENDING <Consumer group Name> <Consume Name> <Range> <IDLE TIME MilliSec>

XPENDING <Consumer group Name> <Consume Name> <Range> <Count> <IDLE TIME MilliSec>

i notice that there a methods defined in both RedisStreamCommands.xPending(....) and ReactiveStreamCommands.xPending(...), which support the current features of XPENDING command.
i am guessing i need to add overloaded methods to add support for. IDLE argument too.

  1. Also i notice that there are three implementations for blocking RedisStreamCommands.xPending method in

DefaultStringRedisConnection, DefaultedRedisConnection, JedisClusterStreamCommands , JedisStreamCommands and LettuceStreamCommands.
out of which the DefaultedRedisConnection API is deprecated, are we suppose to add the XPENDING IDLE argument support for that API too?

@mp911de
Copy link
Member Author

mp911de commented Jun 11, 2021

Idle can only be specified when there's a Range. The command allows multiple variants of arguments so we decided to encapsulate all arguments within XPendingOptions. We do not want to have more overloaded methods on the …Commands level.

If you're interested, you can take a look at StreamOperations. You could add there two overloads for pending which essentially invoke the …Commands API.

@mp911de
Copy link
Member Author

mp911de commented Jun 24, 2021

Any update here? Is there something we can help you with @SivaTharun?

@SivaTharun SivaTharun mentioned this issue Jun 28, 2021
Closed
4 tasks
@SivaTharun
Copy link

@mp911de i made few changes to support XPENDING IDLE argument for RedisStreamCommands, ReactiveStreamCommands and DefaultStringRedisConnection implementations.

Are the changes for supporting IDLE argument for JedisClusterStreamCommands, JedisStreamCommands , LettuceStreamCommands are also in the scope of this issue.

Also i see that the latest version of jedis and redis client jars, which are already being used with in the project, supports the IDLE argument only with a method, that has XPendingParams (for jedis) and XPendingArgs (for lettuce) argument. idle is an attribute with in those classes.

so there will be changes to the xPending method declared in the above three classes, essentially this method has to invoke the xPending method (from client jar), that takes in XPendingArgs(lettuce) / XPendingParams(jedis) as argument.

@matsior
Copy link

matsior commented Apr 26, 2024

Hi, is this issue still needs to be done? Can I work on it?

@whatasame whatasame linked a pull request Mar 8, 2025 that will close this issue
Open
13 tasks
@whatasame
Copy link

whatasame commented Mar 8, 2025 •
edited
Loading

It seems that no one has participated since the last activity a year ago, so I implemented it and created this PR. Please take a look.

whatasame added a commit to whatasame/spring-data-redis that referenced this issue Mar 9, 2025
@whatasame
Add IDLE argument to XPENDING command.
798b5a1 
Original pull request spring-projects#3116
Closes spring-projects#2046

Signed-off-by: Jeonggyu Choi <[email protected]>
whatasame added a commit to whatasame/spring-data-redis that referenced this issue Mar 9, 2025
@whatasame
Add IDLE argument to XPENDING command.
6b657b5 
Original pull request spring-projects#3116
Closes spring-projects#2046

Signed-off-by: Jeonggyu Choi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@SivaTharun SivaTharun

Labels
status: first-timers-only An issue that can only be worked on by brand new contributors theme: Redis 6.2 type: enhancement A general enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add IDLE argument to XPENDING command whatasame/spring-data-redis
4 participants
@mp911de @SivaTharun @matsior @whatasame