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

オープン
  • 何か問題がありますか？
#3,579 コメント 1 件 リアクション 1 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
45/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
静か
技術スタック
cpp, redis
領域
backend-api-design, databases

調査の方向性

src/types/redis_stream.cc から開始し、記載されている consumer-group の減算パスを追跡してから、src/server/redis_reply.h と XINFO の stream/group/consumer serializer を調査します。XPENDING が summary count をどのように取得するかを確認し、可能な saturation の方法と emit 側での処理方法を比較します。残っている unsigned counters と timestamp deltas が signed 64-bit range 外の RESP integers を生成できず、実装前に動作について合意できていれば完了です。

索引モデルが issue の本文から書いたものです。

説明

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 #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. #3578's guard only covers the entries_read > entries_added wrap.

  3. No emit-side clamp for already-corrupt data. #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 #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.

主要言語
C++
スター
4.5k
フォーク
689
平均マージ
1日 22時間
マージ済み PR（30日）
37

環境構築

Codespaces で開く

このプロジェクトの開発コンテナを、あなたの GitHub アカウントでブラウザ上に起動します。

  • Dockerfile または Docker Compose ファイルあり
  • プルリクエストのテンプレートなし
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

apache/kvrocks のほかの issue

apache/kvrocks の issue をすべて見る

似ている issue

C++ の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。