[Bug] XINFO overflow when deleting pending message and consumer

[LindaSummer]

Search before asking

• I had searched in the issues and found no similar issues.

Version

version: unstable
kvrocks_version: unstable
redis_version:4.0.0
git_sha1: d2e0feb
kvrocks_git_sha1: d2e0feb
redis_mode: standalone
kvrocks_mode: standalone
os: Linux 5.14.0-162.6.1.el9_1.0.1.x86_64 x86_64
gcc_version: 11.2.1
arch_bits: 64
process_id: 1
tcp_port: 6666
uptime_in_seconds: 1444
uptime_in_days: 0

Minimal reproduce step

Here is the step using redis-cli:

~ redis-cli -h kvrocks-server -p 6666
kvrocks-server:6666> xgroup create overflow-test info-count-test $ MKSTREAM
OK
kvrocks-server:6666> xgroup createconsumer overflow-test info-count-test consumer-1
(integer) 1
kvrocks-server:6666> xinfo groups overflow-test
1)  1) "name"
    2) "info-count-test"
    3) "consumers"
    4) (integer) 1
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "0-0"
    9) "entries-read"
   10) (nil)
   11) "lag"
   12) (integer) 0
kvrocks-server:6666> xadd overflow-test * testing overflow
"1717248597227-0"
kvrocks-server:6666> xinfo groups overflow-test
1)  1) "name"
    2) "info-count-test"
    3) "consumers"
    4) (integer) 1
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "0-0"
    9) "entries-read"
   10) (nil)
   11) "lag"
   12) (integer) 1
kvrocks-server:6666> xreadgroup group info-count-test consumer-1 count 1 streams overflow-test >
1) 1) "overflow-test"
   2) 1) 1) "1717248597227-0"
         2) 1) "testing"
            2) "overflow"
kvrocks-server:6666> xinfo groups overflow-test
1)  1) "name"
    2) "info-count-test"
    3) "consumers"
    4) (integer) 1
    5) "pending"
    6) (integer) 1
    7) "last-delivered-id"
    8) "1717248597227-0"
    9) "entries-read"
   10) (integer) 0
   11) "lag"
   12) (integer) 1
kvrocks-server:6666> xack overflow-test info-count-test 1717248597227-0
(integer) 1
kvrocks-server:6666> xinfo groups overflow-test
1)  1) "name"
    2) "info-count-test"
    3) "consumers"
    4) (integer) 1
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "1717248597227-0"
    9) "entries-read"
   10) (integer) 0
   11) "lag"
   12) (integer) 1
kvrocks-server:6666> xgroup delconsumer overflow-test info-count-test consumer-1
(integer) 1
kvrocks-server:6666> xinfo groups overflow-test
Error: Bad integer value

The redis-cli refuses to display the response value.

So, I use telnet to run the command and got response below.

xinfo groups overflow-test
*1
*12
$4
name
$15
info-count-test
$9
consumers
:0
$7
pending
:18446744073709551615
$17
last-delivered-id
$15
1717248597227-0
$12
entries-read
:0
$3
lag
:1

What did you expect to see?

This is output from redis-cli.

redis-server:6379>xinfo groups overflow-test
1)  1) "name"
    2) "info-count-test"
    3) "consumers"
    4) (integer) 0
    5) "pending"
    6) (integer) 0
    7) "last-delivered-id"
    8) "1717249449175-0"
    9) "entries-read"
   10) (integer) 1
   11) "lag"
   12) (integer) 0

This output from telnet.

xinfo groups overflow-test
*1
*12
$4
name
$15
info-count-test
$9
consumers
:0
$7
pending
:0
$17
last-delivered-id
$15
1717249449175-0
$12
entries-read
:1
$3
lag
:0

What did you see instead?

redis-cli refuses to display the response.

kvrocks-server:6666> xinfo groups overflow-test
Error: Bad integer value

telnet get the response from kvrocks-server.

xinfo groups overflow-test
*1
*12
$4
name
$15
info-count-test
$9
consumers
:0
$7
pending
:18446744073709551615
$17
last-delivered-id
$15
1717248597227-0
$12
entries-read
:0
$3
lag
:1

Anything Else?

Hi kvrocks team,

I find the way to produce the bug during reading the PR of XPENDING #2190 .

In conversations of the PR, it's suggested to use a separate PR to solve this problem.

I want to fix the bug and add related unit tests with it.

Best Regards,

Edward

Are you willing to submit a PR?

• I'm willing to submit a PR!

[git-hulk]

@LindaSummer Thanks for your detailed report.

[LindaSummer]

Hi @git-hulk ,

This should be forget to update the pending_number in consumer meta.

kvrocks/src/types/redis_stream.cc

Lines 351 to 356 in d2e0feb

	if (*acknowledged > 0) {
	StreamConsumerGroupMetadata group_metadata = decodeStreamConsumerGroupMetadataValue(get_group_value);
	group_metadata.pending_number -= *acknowledged;
	std::string group_value = encodeStreamConsumerGroupMetadataValue(group_metadata);
	batch->Put(stream_cf_handle_, group_key, group_value);
	}

When remove consumer, the group will remove the pending count from the deleted consumer, the inconsistency emerged and may lead to overflow.

kvrocks/src/types/redis_stream.cc

Lines 687 to 688 in d2e0feb

	StreamConsumerMetadata consumer_metadata = decodeStreamConsumerMetadataValue(get_consumer_value);
	deleted_pel = consumer_metadata.pending_number;

kvrocks/src/types/redis_stream.cc

Lines 717 to 720 in d2e0feb

	StreamConsumerGroupMetadata group_metadata = decodeStreamConsumerGroupMetadataValue(get_group_value);
	group_metadata.consumer_number -= 1;
	group_metadata.pending_number -= deleted_pel;
	batch->Put(stream_cf_handle_, group_key, encodeStreamConsumerGroupMetadataValue(group_metadata));

[PragmaTwice]

Thank you for your report!

If you know how to fix it, feel free to open a PR : )

[LindaSummer]

Hi @PragmaTwice ,

I have opened a PR for this bugfix with related test case.

Please take a look.

Thanks very much.

[git-hulk]

@LindaSummer Thanks for your nice catch and fix.