fix(stream): align XINFO GROUPS entries-read and lag with Redis by wengsht · Pull Request #3581 · apache/kvrocks

wengsht · 2026-08-09T17:29:16Z

Follow-up to #3578, split out per review (SRP + clean cherry-pick).

XINFO GROUPS can diverge from Redis in two related ways when a group's entries-read is out of range or its cursor is behind the stream head. Fixed here in one focused commit each.

1. Clamp `entries-read` to `entries-added` on write

XGROUP CREATE/SETID stored the ENTRIESREAD value verbatim, even past entries-added. XINFO GROUPS served it back, and lag = entries_added - entries_read (unsigned) underflowed to ~2^64, overflowing the signed-64 integer clients decode the RESP reply as. Redis clamps on write (t_stream.c L3663-3665); mirrored in Stream::CreateGroup/GroupSetId.

2. Report `lag` as stream length when the group is behind the first entry

Even with an in-range entries-read, if the group's last-delivered-id is behind the first live entry, computing lag from entries_read is wrong. Redis's streamReplyWithCGLag reports lag = length in that case (and 0 for an emptied stream); CheckLagValid was missing both branches. Added them.

Result: matches Redis exactly

Same repro against reference Redis 8.10 and this patch:

127.0.0.1:PORT> XADD repro-lag 1-0 f v ; XADD repro-lag 2-0 f v ; XADD repro-lag 3-0 f v
127.0.0.1:PORT> XGROUP CREATE repro-lag grp 0 ENTRIESREAD 1000000
OK
127.0.0.1:PORT> XINFO GROUPS repro-lag
    ...
    9) "entries-read"
   10) (integer) 3
   11) "lag"
   12) (integer) 3

Both report entries-read = 3, lag = 3. Before this patch kvrocks returned entries-read = 1000000.

Test

XGROUP CREATE/SETID with ENTRIESREAD beyond entries-added, asserting XINFO GROUPS stays decodable and reports entries-read = entries-added and lag = length. The unit/type/stream gocase suite passes.

AI assistance: diagnosis and drafting were done with AI help; I've reviewed the changes and tests and understand the behavior.

github-actions · 2026-08-09T17:29:28Z

Hi @wengsht,

Thank you for your pull request. Please review our Contributing Guide.

Please make sure you understand your changes and explain your reasoning in this pull request. Low-quality pull requests may be closed.

wengsht · 2026-08-09T18:08:12Z

@LindaSummer can you look at this one when you get a chance?

` # kvrocks with PR #3581 (:6666)
127.0.0.1:6666> DEL repro-lag
(integer) 1
127.0.0.1:6666> XADD repro-lag 1-0 f v
"1-0"
127.0.0.1:6666> XADD repro-lag 2-0 f v
"2-0"
127.0.0.1:6666> XADD repro-lag 3-0 f v
"3-0"
127.0.0.1:6666> XGROUP CREATE repro-lag grp 0 ENTRIESREAD 1000000
OK
127.0.0.1:6666> XINFO GROUPS repro-lag

1. "name"
2. "grp"
3. "consumers"
4. (integer) 0
5. "pending"
6. (integer) 0
7. "last-delivered-id"
8. "0-0"
9. "entries-read"
  10) (integer) 3
  11) "lag"
  12) (integer) 3`

LindaSummer

Hi @wengsht ,

Thanks for your effort.

Left some comments, and we'd better fix the linter issue with ./x.py format before committing.

Copilot

Pull request overview

Aligns XINFO GROUPS entries-read and lag reporting with Redis.

Changes:

Clamps ENTRIESREAD to entries-added.
Handles empty streams and cursors behind the stream head.
Adds integration coverage for CREATE and SETID clamping.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/types/redis_stream.cc`	Updates entries-read clamping and lag calculation.
`tests/gocase/unit/type/stream/stream_test.go`	Tests CREATE/SETID clamping behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

jihuayu · 2026-08-11T11:25:20Z

Hi @wengsht Since I merged your previous PR, you may need to resolve the conflicts manually. Thanks!

An ENTRIESREAD larger than the stream's entries-added was stored verbatim and served back by XINFO GROUPS, and the group's lag (entries_added - entries_read, an unsigned field) underflowed to a near-2^64 value that overflows the signed 64-bit integer clients decode the RESP reply as, breaking XINFO GROUPS. Redis clamps entries-read down to entries-added on write in XGROUP CREATE and SETID (t_stream.c). Mirror that so the stored counter can never exceed entries-added: the reported entries-read now matches Redis and lag can no longer underflow.

…e stream head lag is entries_added - entries_read on an unsigned field. When the group's last-delivered-id is behind the first live entry (a group created at 0, or one whose entries_read was set inconsistently via XGROUP SETID/ENTRIESREAD), trusting entries_read yields a wrong or underflowing lag. Mirror Redis streamReplyWithCGLag: when the cursor and the max tombstone are both behind the first entry, report lag = current stream length; and report lag = 0 for an emptied stream. XINFO GROUPS lag now matches Redis exactly.

run-clang-tidy (readability-identifier-naming) requires CamelCase for functions; the helper was lowerCamelCase, which failed the Lint job. Rename to match the other file-local helpers (CheckLagValid, StreamRangeHasTombstones).

…, add tests Review follow-ups on top of the rebase onto unstable (post-apache#3578): - CheckLagValid: guard entries_read <= entries_added before the unsigned subtraction. A clamp on write (CREATE/SETID) does not stop ReadGroup from incrementing entries_read past entries_added on a subsequent XREADGROUP, which would re-underflow lag to ~2^64 and break XINFO GROUPS decoding again. Falling through to the estimate path keeps the reply decodable. (Copilot review.) - ClampEntriesRead: guard entries_read >= 0 rather than != -1, so any negative value is handled before the uint64 cast. (LindaSummer review.) - Merge the size==0 (all-entries-deleted) lag=0 case into the entries_added==0 branch. (LindaSummer review.) - Tests: add the read-after-clamp regression (XREADGROUP then XINFO GROUPS stays decodable, lag 0) and an emptied-stream lag=0 case. Restore the apache#3578 XAUTOCLAIM pending_number test dropped during conflict resolution. (Copilot review.) Ran clang-format-18 and gofmt.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/types/redis_stream.cc:1373

This guard only hides the underflow; it does not keep entries-read Redis-compatible. In the added scenario, RangeWithPending increments the clamped value from 3 to 6 while delivering the three entries, so XINFO GROUPS still returns entries-read = 6 instead of Redis's 3. With COUNT 2, the cursor lands on a middle ID, the estimate path cannot recover a distance, and lag becomes null instead of 1. Update the read path to recompute entries_read from the delivered ID when the previous cursor is behind the first entry (matching Redis's streamReplyWithRange logic), and assert both EntriesRead and partial-read lag in the regression test.

  } else if (group_metadata.entries_read != -1 &&
             group_metadata.entries_read <= static_cast<int64_t>(stream_metadata.entries_added) &&
             !StreamRangeHasTombstones(stream_metadata, group_metadata.last_delivered_id)) {
    // Guard entries_read <= entries_added: the subtraction is served as an unsigned lag, so
    // an entries_read ahead of entries_added (e.g. a post-clamp XREADGROUP still incrementing

LindaSummer · 2026-08-13T15:01:28Z

Hi @wengsht ,

Thanks very much for your effort!
I will review this patch this weekend.

LindaSummer

Hi @wengsht ,

Thanks very much for your effort!

We have one more case to be fixed below.
The entries-read and lag in last XINFO GROUPS are not aligned with redis' behavior.

Here is redis behavior.

redis-cli DEL x-entries-read \
&& redis-cli XADD x-entries-read 1-0 f v \
&& redis-cli XADD x-entries-read 2-0 f v \
&& redis-cli XADD x-entries-read 3-0 f v \
&& redis-cli XGROUP CREATE x-entries-read grp 0 ENTRIESREAD 1000000 \
&& redis-cli XINFO GROUPS x-entries-read \
&& redis-cli XREADGROUP GROUP grp c1 COUNT 2 STREAMS x-entries-read '>' \
&& redis-cli XINFO GROUPS x-entries-read
(integer) 0
"1-0"
"2-0"
"3-0"
OK
1)  1) "name"
    2) "grp"
    3) "consumers"
    4) (integer) 0
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "0-0"
    9) "entries-read"
   10) (integer) 3
   11) "lag"
   12) (integer) 3
1) 1) "x-entries-read"
   2) 1) 1) "1-0"
         2) 1) "f"
            2) "v"
      2) 1) "2-0"
         2) 1) "f"
            2) "v"
1)  1) "name"
    2) "grp"
    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) 1

Here is current patch behavior.

redis-cli -p 6666
127.0.0.1:6666> DEL x-entries-read
(integer) 0
127.0.0.1:6666> XADD x-entries-read 1-0 f v
"1-0"
127.0.0.1:6666> XADD x-entries-read 2-0 f v
"2-0"
127.0.0.1:6666> XADD x-entries-read 3-0 f v
"3-0"
127.0.0.1:6666> XGROUP CREATE x-entries-read grp 0 ENTRIESREAD 1000000
OK
127.0.0.1:6666> XINFO GROUPS x-entries-read
1)  1) "name"
    2) "grp"
    3) "consumers"
    4) (integer) 0
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "0-0"
    9) "entries-read"
   10) (integer) 3
   11) "lag"
   12) (integer) 3
127.0.0.1:6666> XREADGROUP GROUP grp c1 COUNT 2 STREAMS x-entries-read '>'
1) 1) "x-entries-read"
   2) 1) 1) "1-0"
         2) 1) "f"
            2) "v"
      2) 1) "2-0"
         2) 1) "f"
            2) "v"
127.0.0.1:6666> XINFO GROUPS x-entries-read
1)  1) "name"
    2) "grp"
    3) "consumers"
    4) (integer) 1
    5) "pending"
    6) (integer) 2
    7) "last-delivered-id"
    8) "2-0"
    9) "entries-read"
   10) (integer) 5
   11) "lag"
   12) (nil)

…s behind the stream head

wengsht · 2026-08-31T15:52:15Z

Hi @LindaSummer,

Good catch!

Your script against reference Redis 8.10 and this patch now produces identical output.

Here is redis behavior.

redis-cli DEL x-entries-read \
&& redis-cli XADD x-entries-read 1-0 f v \
&& redis-cli XADD x-entries-read 2-0 f v \
&& redis-cli XADD x-entries-read 3-0 f v \
&& redis-cli XGROUP CREATE x-entries-read grp 0 ENTRIESREAD 1000000 \
&& redis-cli XINFO GROUPS x-entries-read \
&& redis-cli XREADGROUP GROUP grp c1 COUNT 2 STREAMS x-entries-read '>' \
&& redis-cli XINFO GROUPS x-entries-read
(integer) 1
"1-0"
"2-0"
"3-0"
OK
1)  1) "name"
    2) "grp"
    3) "consumers"
    4) (integer) 0
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "0-0"
    9) "entries-read"
   10) (integer) 3
   11) "lag"
   12) (integer) 3
1) 1) "x-entries-read"
   2) 1) 1) "1-0"
         2) 1) "f"
            2) "v"
      2) 1) "2-0"
         2) 1) "f"
            2) "v"
1)  1) "name"
    2) "grp"
    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) 1

Here is current patch behavior.

redis-cli -p 6666
127.0.0.1:6666> DEL x-entries-read
(integer) 1
127.0.0.1:6666> XADD x-entries-read 1-0 f v
"1-0"
127.0.0.1:6666> XADD x-entries-read 2-0 f v
"2-0"
127.0.0.1:6666> XADD x-entries-read 3-0 f v
"3-0"
127.0.0.1:6666> XGROUP CREATE x-entries-read grp 0 ENTRIESREAD 1000000
OK
127.0.0.1:6666> XINFO GROUPS x-entries-read
1)  1) "name"
    2) "grp"
    3) "consumers"
    4) (integer) 0
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "0-0"
    9) "entries-read"
   10) (integer) 3
   11) "lag"
   12) (integer) 3
127.0.0.1:6666> XREADGROUP GROUP grp c1 COUNT 2 STREAMS x-entries-read '>'
1) 1) "x-entries-read"
   2) 1) 1) "1-0"
         2) 1) "f"
            2) "v"
      2) 1) "2-0"
         2) 1) "f"
            2) "v"
127.0.0.1:6666> XINFO GROUPS x-entries-read
1)  1) "name"
    2) "grp"
    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) 1

A following full drain also agrees on both: entries-read = 3, lag = 0.

LindaSummer · 2026-09-04T02:28:18Z

Hi @wengsht ,

Thanks very much for your effort!❤️

I will review this PR this weekend.

Have a nice day!

Copilot

🟡 Changes recommended

The read counter repair must occur independently of optional PEL insertion for NOACK reads.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

wengsht · 2026-09-14T17:00:25Z

hi @LindaSummer when you get a chance can you check if this PR ready to merge?

wengsht changed the title ~~fix(stream): clamp XGROUP CREATE/SETID entries-read to entries-added~~ fix(stream): align XINFO GROUPS entries-read and lag with Redis Aug 9, 2026

wengsht mentioned this pull request Aug 9, 2026

fix(stream): decrement pending_number when XAUTOCLAIM removes deleted PEL entries #3578

Merged

LindaSummer reviewed Aug 10, 2026

View reviewed changes

Comment thread src/types/redis_stream.cc

Comment thread src/types/redis_stream.cc Outdated

Comment thread src/types/redis_stream.cc

LindaSummer requested a balanced review from Copilot August 10, 2026 08:06

Copilot started reviewing on behalf of LindaSummer August 10, 2026 08:06 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

Comment thread src/types/redis_stream.cc Outdated

Comment thread src/types/redis_stream.cc Outdated

wengsht added 4 commits August 11, 2026 09:02

wengsht force-pushed the fix-xgroup-entries-read-clamp branch from 296df26 to 6d39abf Compare August 11, 2026 16:07

wengsht requested a review from LindaSummer August 11, 2026 16:31

Merge branch 'unstable' into fix-xgroup-entries-read-clamp

c835f05

LindaSummer requested a balanced review from Copilot August 12, 2026 06:26

Copilot started reviewing on behalf of LindaSummer August 12, 2026 06:28 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

LindaSummer reviewed Aug 18, 2026

View reviewed changes

fix(stream): match Redis entries-read on XREADGROUP when the cursor i…

7dd787e

…s behind the stream head

LindaSummer requested a balanced review from Copilot September 4, 2026 02:29

Copilot started reviewing on behalf of LindaSummer September 4, 2026 02:30 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread src/types/redis_stream.cc

wengsht requested a review from LindaSummer September 16, 2026 21:45

Merge branch 'unstable' into fix-xgroup-entries-read-clamp

f7ce5ed

Conversation

wengsht commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Clamp entries-read to entries-added on write

2. Report lag as stream length when the group is behind the first entry

Result: matches Redis exactly

Test

Uh oh!

github-actions Bot commented Aug 9, 2026

Uh oh!

wengsht commented Aug 9, 2026

Uh oh!

LindaSummer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

jihuayu commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

LindaSummer commented Aug 13, 2026

Uh oh!

LindaSummer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wengsht commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LindaSummer commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

wengsht commented Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wengsht commented Aug 9, 2026 •

edited

Loading

1. Clamp `entries-read` to `entries-added` on write

2. Report `lag` as stream length when the group is behind the first entry

jihuayu commented Aug 11, 2026 •

edited

Loading

LindaSummer left a comment •

edited

Loading

wengsht commented Aug 31, 2026 •

edited

Loading