-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add support for XREADGROUP CLAIM arg #3486
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
afc4947
84438de
2e04494
085d857
79c7439
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||||||||||||
| package io.lettuce.core; | ||||||||||||||||
|
|
||||||||||||||||
| import java.time.Duration; | ||||||||||||||||
| import java.util.Map; | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Stream message returned by XREADGROUP when entries were claimed from the PEL using CLAIM min-idle-time. Contains additional | ||||||||||||||||
| * metadata: milliseconds since last delivery and redelivery count. | ||||||||||||||||
| */ | ||||||||||||||||
| public class ClaimedStreamMessage<K, V> extends StreamMessage<K, V> { | ||||||||||||||||
|
|
||||||||||||||||
| private final long msSinceLastDelivery; | ||||||||||||||||
|
|
||||||||||||||||
| private final long redeliveryCount; | ||||||||||||||||
|
|
||||||||||||||||
| public ClaimedStreamMessage(K stream, String id, Map<K, V> body, long msSinceLastDelivery, long redeliveryCount) { | ||||||||||||||||
| super(stream, id, body); | ||||||||||||||||
| this.msSinceLastDelivery = msSinceLastDelivery; | ||||||||||||||||
| this.redeliveryCount = redeliveryCount; | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+16
to
+20
|
||||||||||||||||
|
|
||||||||||||||||
|
||||||||||||||||
| /** | |
| * Returns the number of milliseconds since the last delivery of this message. | |
| * | |
| * @return milliseconds since the last delivery of this message | |
| */ |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing JavaDoc for the public method. Should include:
- A description of what the method returns
@returnDuration since the last delivery of this message
| /** | |
| * Returns the duration since the last delivery of this message. | |
| * | |
| * @return Duration since the last delivery of this message. | |
| */ |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing JavaDoc for the public method. Should include:
- A description of what the method returns
@returnthe number of times this message has been delivered (redelivery count)
| /** | |
| * Returns the number of times this message has been delivered (redelivery count). | |
| * | |
| * @return the number of times this message has been delivered (redelivery count) | |
| */ |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,8 @@ public class XReadArgs implements CompositeArgument { | |||||||||||||||||||
|
|
||||||||||||||||||||
| private boolean noack; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private Long claimMinIdleTime; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * Builder entry points for {@link XReadArgs}. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
|
|
@@ -90,6 +92,21 @@ public static XReadArgs noack(boolean noack) { | |||||||||||||||||||
| return new XReadArgs().noack(noack); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * Create a new {@link XReadArgs} and set CLAIM min-idle-time (milliseconds). Only valid for XREADGROUP. | ||||||||||||||||||||
|
||||||||||||||||||||
| * Create a new {@link XReadArgs} and set CLAIM min-idle-time (milliseconds). Only valid for XREADGROUP. | |
| * Create a new {@link XReadArgs} and set CLAIM min-idle-time (milliseconds). Only valid for XREADGROUP. | |
| * | |
| * @param milliseconds the minimum idle time in milliseconds | |
| * @return new {@link XReadArgs} with CLAIM set | |
| * @see XReadArgs#claim(long) |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing JavaDoc parameters and return value documentation. The method should document:
@param timeoutthe minimum idle time as a Duration@returnnew {@link XReadArgs} with CLAIM set- Consider adding
@see XReadArgs#claim(Duration)for consistency with other Builder methods
| */ | |
| */ | |
| /** | |
| * Create a new {@link XReadArgs} and set CLAIM min-idle-time. Only valid for XREADGROUP. | |
| * | |
| * @param timeout the minimum idle time as a {@link Duration} | |
| * @return new {@link XReadArgs} with CLAIM set | |
| * @see XReadArgs#claim(Duration) | |
| */ |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing JavaDoc parameters and return value documentation. The method should document:
@param millisecondsthe minimum idle time in milliseconds@return{@code this} XReadArgs instance for method chaining
| * | |
| * | |
| * @param milliseconds the minimum idle time in milliseconds | |
| * @return {@code this} XReadArgs instance for method chaining |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing JavaDoc parameters and return value documentation. The method should document:
@param timeoutthe minimum idle time as a Duration@return{@code this} XReadArgs instance for method chaining
| * | |
| * | |
| * @param timeout the minimum idle time as a Duration | |
| * @return {@code this} XReadArgs instance for method chaining |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.lettuce.core.StreamMessage; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.lettuce.core.codec.RedisCodec; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.lettuce.core.ClaimedStreamMessage; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.lettuce.core.internal.LettuceAssert; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -31,6 +33,10 @@ public class StreamReadOutput<K, V> extends CommandOutput<K, V, List<StreamMessa | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private Map<K, V> body; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private Long msSinceLastDelivery; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private Long redeliveryCount; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private boolean bodyReceived = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public StreamReadOutput(RedisCodec<K, V> codec) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -51,6 +57,20 @@ public void set(ByteBuffer bytes) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle extra metadata for claimed entries that may arrive as bulk strings (RESP2/RESP3) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (id != null && bodyReceived && key == null && bytes != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use a duplicate so decoding doesn't advance the original buffer position. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String s = decodeString(bytes.duplicate()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (msSinceLastDelivery == null && isDigits(s)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| msSinceLastDelivery = Long.parseLong(s); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (redeliveryCount == null && isDigits(s)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redeliveryCount = Long.parseLong(s); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+65
to
+69
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| msSinceLastDelivery = Long.parseLong(s); | |
| return; | |
| } | |
| if (redeliveryCount == null && isDigits(s)) { | |
| redeliveryCount = Long.parseLong(s); | |
| try { | |
| msSinceLastDelivery = Long.parseLong(s); | |
| } catch (NumberFormatException e) { | |
| msSinceLastDelivery = null; // or handle as appropriate | |
| } | |
| return; | |
| } | |
| if (redeliveryCount == null && isDigits(s)) { | |
| try { | |
| redeliveryCount = Long.parseLong(s); | |
| } catch (NumberFormatException e) { | |
| redeliveryCount = null; // or handle as appropriate | |
| } |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
| msSinceLastDelivery = Long.parseLong(s); | |
| return; | |
| } | |
| if (redeliveryCount == null && isDigits(s)) { | |
| redeliveryCount = Long.parseLong(s); | |
| try { | |
| msSinceLastDelivery = Long.parseLong(s); | |
| } catch (NumberFormatException e) { | |
| // Optionally log or handle the error here | |
| } | |
| return; | |
| } | |
| if (redeliveryCount == null && isDigits(s)) { | |
| try { | |
| redeliveryCount = Long.parseLong(s); | |
| } catch (NumberFormatException e) { | |
| // Optionally log or handle the error here | |
| } |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential logic issue: If a non-numeric bulk string is received when extras are expected (id != null && bodyReceived && key == null), the code will skip the extras handling (lines 64-71) because isDigits(s) returns false, but then fall through to line 79 where it would incorrectly interpret this data as a body key. This could cause incorrect parsing.
Consider restructuring the logic to handle unexpected data more explicitly, or add an else clause to log/handle non-numeric extras:
if (id != null && bodyReceived && key == null && bytes != null) {
String s = decodeString(bytes.duplicate());
if (msSinceLastDelivery == null && isDigits(s)) {
msSinceLastDelivery = Long.parseLong(s);
return;
}
if (redeliveryCount == null && isDigits(s)) {
redeliveryCount = Long.parseLong(s);
return;
}
// If we reach here with non-numeric data in the extras position,
// it should be handled or logged, not allowed to fall through
}| } | |
| } | |
| // If we reach here, non-numeric data was received in the extras position. | |
| // Log or handle the unexpected data. For now, we ignore and do not fall through. | |
| // System.err.println("Unexpected non-numeric extras data: " + s); | |
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly unused code. Seems server sends string instead the specified integer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing JavaDoc for the class. The class documentation should include:
@param <K>the key type@param <V>the value type@authortag@sincetag (appears to be version 7.0 based on the XReadArgs documentation)