-
Notifications
You must be signed in to change notification settings - Fork 791
Fix incorrect lag reported in XINFO GROUPS #1952
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #1952 +/- ##
============================================
- Coverage 70.98% 70.86% -0.13%
=========
8000
===================================
Files 123 123
Lines 65768 65941 +173
============================================
+ Hits 46687 46730 +43
- Misses 19081 19211 +130
🚀 New features to boost your workflow:
|
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.
LGTM.
@hpatro Shall we backport it?
@zuiderkwast @hpatro #685 try to fix others issues as well and it's more dangerous to handle all of them in a single fix |
Right. I agree we shouldn't conflate the PR. I just want to ensure the correctness. @artikell would you mind reviewing this ? |
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.
The overall change looks good to me. @nesty92 Could you share the before and after behavior (preferrable valkey-cli output) ?
@zuiderkwast @hpatro Yes, this PR is a simple fix that also aligns redis. There is another fix on redis, and this pr is also dealt with together? @nesty92 When using the XTRIM command to trim a stream, it does not update the maximal tombstone (max_deleted_entry_id). This leads to an issue where the lag calculation incorrectly assumes that there are no tombstones after the consumer group's last_id, resulting in an inaccurate lag. |
@hpatro the full output is in #1951 description, here is just the last result 127.0.0.1:6379> XINFO GROUPS mystream
1) 1) "name"
2) "g1"
3) "consumers"
4) (integer) 1
5) "pending"
6) (integer) 3
7) "last-delivered-id"
8) "4-0"
9) "entries-read"
10) (integer) 3
11) "lag"
12) (integer) 1 # it should be 0 the message was deleted and there is not lag this is the new output with the fix > ./src/valkey-cli
127.0.0.1:6379> xadd mystream 1-0 data a
"1-0"
127.0.0.1:6379> xadd mystream 2-0 data b
"2-0"
127.0.0.1:6379> xadd mystream 3-0 data c
"3-0"
127.0.0.1:6379> xadd mystream 4-0 data d
"4-0"
127.0.0.1:6379> XGROUP CREATE mystream g1 0
OK
127.0.0.1:6379> XREADGROUP GROUP g1 c1 COUNT 2 STREAMS mystream >
1) 1) "mystream"
2) 1) 1) "1-0"
2) 1) "data"
2) "a"
2) 1) "2-0"
2) 1) "data"
2) "b"
127.0.0.1:6379> XINFO GROUPS mystream
1) 1) "name"
2) "g1"
3) "consumers"
4) (integer) 1
5) "pending"
6) (integer) 2
7) "last-delivered-id"
8) "2-0"
9) "entries-read"
10) (integer) 2
11) "lag"
12) (integer) 2
127.0.0.1:6379> xdel mystream 3-0
(integer) 1
127.0.0.1:6379> XINFO GROUPS mystream
1) 1) "name"
2) "g1"
3) "consumers"
4) (integer) 1
5) "pending"
6) (integer) 2
7) "last-delivered-id"
8) "2-0"
9) "entries-read"
10) (integer) 2
11) "lag"
12) (nil)
127.0.0.1:6379> XREADGROUP GROUP g1 c1 COUNT 2 STREAMS mystream >
1) 1) "mystream"
2) 1) 1) "4-0"
2) 1) "data"
2) "d"
127.0.0.1:6379> XINFO GROUPS mystream
1) 1) "name"
2) "g1"
3) "consumers"
4) (integer) 1
5) "pending"
6) (integer) 3
7) "last-delivered-id"
8) "4-0"
9) "entries-read"
10) (integer) 4
11) "lag"
12) (integer) 0
127.0.0.1:6379> |
@artikell Oh, I didn't know about this one, but yes you are right I was able to replicate the issue with XTRIM but not sure if we should handle it together. What do you think @hpatro @zuiderkwast @artikell ? It's my first time trying to contribute here so any feedback is more than welcome |
Yes, thank you, it's great! I am not very familiar with consumer groups logic so I need some help and advice. I don't care which PR we will merge, as long as it fixes the problems. If the #685 doesn't affect |
@zuiderkwast @artikell I included a fix for the XTRIM, left the details also in the commit message The issue arises when This fix updates the trimming logic to reset In this way we are not changing the behavior of the lag all the existing test continue passing and a new one was added. note: the problem with the XTRIM, could be also with XADD if trimming is set. That case is cover as well |
272f7dd
to
a2c0ff4
Compare
Thanks for the help @zuiderkwast @hpatro @artikell |
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Nitai Caro <caronita@amazon.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: #1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: #1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: #1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When a tombstone is created after the last_id of a consumer group. The consumer group lag is reported in the reply of `XINFO GROUPS mystream`. Close: valkey-io#1951 Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com> Signed-off-by: hwware <wen.hui.ware@gmail.com>
When a tombstone is created after the last_id of a consumer group.
The consumer group lag is reported in the reply of
XINFO GROUPS mystream
.Close: #1951