spring-data-redis icon indicating copy to clipboard operation
spring-data-redis copied to clipboard

Add `IDLE` argument to `XPENDING` command

Open mp911de opened this issue 4 years ago • 8 comments

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 avatar Apr 19 '21 06:04 mp911de

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

SivaTharun avatar May 07 '21 17:05 SivaTharun

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

mp911de avatar May 07 '21 19:05 mp911de

@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?

SivaTharun avatar May 19 '21 18:05 SivaTharun

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 avatar Jun 11 '21 09:06 mp911de

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

mp911de avatar Jun 24 '21 13:06 mp911de

@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.

SivaTharun avatar Jun 28 '21 05:06 SivaTharun

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

matsior avatar Apr 26 '24 16:04 matsior