fix(stream): align XINFO GROUPS entries-read and lag with Redis - #3581
Open
wengsht wants to merge 2 commits into
Open
fix(stream): align XINFO GROUPS entries-read and lag with Redis#3581wengsht wants to merge 2 commits into
wengsht wants to merge 2 commits into
Conversation
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.
|
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. |
…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.
Author
|
@LindaSummer can you look at this one when you get a chance? ` # kvrocks with PR #3581 (:6666)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #3578, split out per review (SRP + clean cherry-pick).
XINFO GROUPScan diverge from Redis in two related ways when a group'sentries-readis out of range or its cursor is behind the stream head. Fixed here in one focused commit each.1. Clamp
entries-readtoentries-addedon writeXGROUP CREATE/SETIDstored theENTRIESREADvalue verbatim, even pastentries-added.XINFO GROUPSserved it back, andlag = 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 inStream::CreateGroup/GroupSetId.2. Report
lagas stream length when the group is behind the first entryEven with an in-range
entries-read, if the group'slast-delivered-idis behind the first live entry, computinglagfromentries_readis wrong. Redis'sstreamReplyWithCGLagreportslag = lengthin that case (and0for an emptied stream);CheckLagValidwas missing both branches. Added them.Result: matches Redis exactly
Same repro against reference Redis 8.10 and this patch:
Both report
entries-read = 3,lag = 3. Before this patch kvrocks returnedentries-read = 1000000.Test
XGROUP CREATE/SETIDwithENTRIESREADbeyondentries-added, assertingXINFO GROUPSstays decodable and reportsentries-read = entries-addedandlag = length. Theunit/type/streamgocase suite passes.AI assistance: diagnosis and drafting were done with AI help; I've reviewed the changes and tests and understand the behavior.