127.0.0.1:6379> XADD x 1 f1 v1
"1-0"
127.0.0.1:6379> XADD x 2 f1 v1
"2-0"
127.0.0.1:6379> XADD x 3 f1 v1
"3-0"
127.0.0.1:6379> XGROUP CREATE x grp 0
OK
127.0.0.1:6379> XREADGROUP GROUP grp Alice COUNT 2 STREAMS x >
1) 1) "x"
2) 1) 1) "1-0"
2) 1) "f1"
2) "v1"
2) 1) "2-0"
2) 1) "f1"
2) "v1"
127.0.0.1:6379> XDEL x 1 2
(integer) 2
127.0.0.1:6379> XCLAIM x grp Bob 0 1 2
1) (nil)
2) (nil)
XCLAIM replies with null for each deleted entry... is this by design? or by lack of choice? (fixing it means we can't always call streamReplyWithRange in xclaimCommand: maybe it's not worth the effort?)
Comment From: antirez
Hi @guybe7, yeah, basically the idea here is that this behavior is both simple, and clear for the user: in this way the caller can understand that entries it is trying to fetch were deleted by someone, and can ignore them, at the same time we don't have to do any cleanup in the PEL, that may be in many many consumer groups. That's ll. So it's by design. Maybe we should document this better?
Comment From: gkorland
@guybe7 I think the more confusing behavior here is that XPENDING still returns the IDs
127.0.0.1:6379> XPENDING x grp
1) (integer) 2
2) "1-0"
3) "2-0"
4) 1) 1) "Alice"
2) "2"
Comment From: guybe7
@gkorland yes... just don't know if it's worth fixing... i'm guessing not a lot of people use XDEL (i wonder if it happens also with XTRIM... then maybe we have some justification)
Comment From: kandy
Have the same unexpected behavior with XPENDING
127.0.0.1:6379> XRANGE product_action_attribute.update - +
(empty list or set)
127.0.0.1:6379> XPENDING product_action_attribute.update test
1) (integer) 1
2) "1603691795580-0"
3) "1603691795580-0"
4) 1) 1) "app-xdebug:c:597"
2) "1"
Comment From: guybe7
if we want to fix this strange behavior we will have to pay in complexity: every time we trim or delete an entry from a stream we will have to scan each PEL of each cgroup and mark the NACK as "invalid" and make sure we don't account "invalid" NACKs in XPENDING, XCLAIM etc. otherwise we can scan each PEL of each cgroup + each PEL of each consumer and just delete NACKs of a certain range. anyway, it seems like it's not worth it... unless it really bothers people and they have some good reasoning... @gkorland @kandy is this more than an annoying cosmetic issue?
in theory there should be a GC mechanism for streams (to actually free memory of XDELeted entries), maybe it can also do some CG-related house cleaning? not sure it's a good idea because it's a bit weird a cron task will affect the result of a command
Comment From: itamarhaber
Mailing list related: https://groups.google.com/g/redis-db/c/wT8MUKQ-ehg/m/r5ffTv7yBgAJ
Comment From: asalabaev
I agree the existing behavior makes sense in some scenarios, e.g. the consumer seeing a NULL body may fire an alert about the message/information got lost, you may need to be re-submitted to the queue, etc. On the other hand the client can ignore NULL message body. A documentation would help preventing sudden NullPointerExceptions when processing message body of a missing entry
Comment From: monotykamary
@gkorland yes... just don't know if it's worth fixing... i'm guessing not a lot of people use XDEL (i wonder if it happens also with XTRIM... then maybe we have some justification)
I've found that this also happens with XTRIM and with XADD at a fixed threshold with tons of writes and it accumulates a lot of garbage entries. With XADD threshold at ~1,000 and an ingestion rate of 10,000+ and XACK failing 0.1% of the time leads to about 20 null entries every minute with occasional spikes. At the very least, it's definitely not a cosmetic issue if consumers are unexpectedly able to consume null entries or loop through trash.
To avoid this, I just call XPENDING and XCLAIM: if an XCLAIM item returns null, XACK it and throw an error and essentially clean it from the pending list.
Comment From: guybe7
@gkorland @kandy @asalabaev @monotykamary this issue was partly fixed in https://github.com/redis/redis/pull/10227
Intro
the main problem with using XACK on all ids for which "nil" was returned is that there's no guarantee for any correlation between the index of the "id" arguments and the array indices. for example:
127.0.0.1:6379> XADD x 1 f1 v1
"1-0"
127.0.0.1:6379> XADD x 2 f1 v1
"2-0"
127.0.0.1:6379> XADD x 3 f1 v1
"3-0"
127.0.0.1:6379> XGROUP CREATE x grp 0
OK
127.0.0.1:6379> XREADGROUP GROUP grp Alice COUNT 2 STREAMS x >
1) 1) "x"
2) 1) 1) "1-0"
2) 1) "f1"
2) "v1"
2) 1) "2-0"
2) 1) "f1"
2) "v1"
127.0.0.1:6379> XDEL x 1 2
(integer) 2
127.0.0.1:6379> XCLAIM x grp Bob 0 0-99 1-0 1-99 2-0
1) (nil)
2) (nil)
the PR mentioned above makes X[AUTO]CLAIM clear the NACK of a deleted entry from wherever it found it. So it will never reply with "nil" for any entry.
Knowing which entries were cleared from the PEL
XCLAIM
the set {XCLAIM input ids} - {XCLAIM returned ids} contains all the entry ids that were not claimed which means they were deleted (assuming the input contains only entries from some PEL). The user doesn't need to XACK them because XCLAIM had already deleted them from the source PEL.
XAUTOCLAIM
XAUTOCLAIM has a new element added to its reply: it's an array of all the deleted stream IDs it stumbled upon.
please LMK if you see an issue with that approach