stream: consumer-group counters can exceed INT64_MAX and break clients decoding RESP integers as signed

[wengsht]

Summary

Several Redis-stream consumer-group counters are stored as uint64_t and emitted directly via redis::Integer (std::to_string, src/server/redis_reply.h). If any of them holds a value above INT64_MAX, the reply is a decimal like :18446744073709551615\r\n, which clients that decode RESP : integers as signed 64-bit (e.g. the Rust fred client) fail to parse — the connection is dropped/poisoned rather than returning group info.

PR #3578 fixes the two concrete instances we hit in production. This issue tracks the remaining, related hardening so it can be discussed before a broader patch (per the contributing guide's "discuss first / prefer focused patches").

Already addressed by #3578

• XAUTOCLAIM deleting a dangling PEL entry without decrementing pending_number (counter drifts up, later wraps).
• XINFO GROUPS lag wrapping when entries_read > entries_added (unsigned subtraction).

Remaining, same failure class

1. Non-saturating pending_number decrements. After fix(stream): decrement pending_number when XAUTOCLAIM removes deleted PEL entries #3578, AutoClaim is the only saturating decrement; these still subtract unconditionally, so any invariant break wraps a client-visible uint64 counter (all src/types/redis_stream.cc):

   • DeleteConsumer (XGROUP DELCONSUMER): group.pending_number -= deleted_pel — cross-level (subtracts a consumer's count from the group's count), highest risk.
   • DeletePelEntries (XACK): group.pending_number -= *acknowledged and consumer.pending_number -= ack_count.
   • ClaimPelEntries (XCLAIM): original_consumer.pending_number -= 1.

2. lag can exceed INT64_MAX even without a wrap. entries_added is settable to any uint64 via XSETID ... ENTRIESADDED n, so a correctly-computed lag = entries_added - entries_read can be a legitimate value > INT64_MAX that CheckLagValid passes through to the reply. fix(stream): decrement pending_number when XAUTOCLAIM removes deleted PEL entries #3578's guard only covers the entries_read > entries_added wrap.

3. No emit-side clamp for already-corrupt data. fix(stream): decrement pending_number when XAUTOCLAIM removes deleted PEL entries #3578 stops future drift but doesn't repair or clamp existing values, and other unsigned fields are emitted raw: group/consumer pending_number (XINFO GROUPS/CONSUMERS), entries-added (XINFO STREAM), and the idle/inactive timestamp deltas (now - stored_ts, which wrap under clock skew / a future ts injected via XCLAIM). (The XPENDING summary count appears to be recomputed live from the PEL scan — worth confirming whether it reads the cached counter on any path.)

Suggested direction

• Make all consumer-group counter decrements saturating (x >= n ? x - n : 0), matching fix(stream): decrement pending_number when XAUTOCLAIM removes deleted PEL entries #3578's AutoClaim decrement.
• Either treat lag > INT64_MAX as unknown (nil, via the existing UINT64_MAX sentinel) in CheckLagValid, or add an emit-side clamp helper (min(v, INT64_MAX)) applied to the counter/timestamp serializations so the wire value is always within i64 for strict clients. Redis already treats these as signed, so clamping is behavior-compatible.

Happy to send a PR once there's agreement on the preferred shape (per-site saturation vs. a shared emit-side clamp).

---

AI assistance: the analysis behind this issue was done with AI help; I've reviewed and verified the findings against the source.