Currently, when using XTRIM with approximate mode (~) and DELREF/ACKED deletion strategies, no messages are deleted. The code only deletes full nodes when the strategy is not DELREF/ACKED. This PR attempts to fix that issue.

However, there appears to be a fundamental conflict: approximate mode is designed to delete entire nodes from the stream radix tree to speed up deletion. But DELREF/ACKED strategies require checking each node messages before deletion. This means approximate mode and DELREF/ACKED seem inherently incompatible—the former prioritizes speed by removing full nodes, while the latter requires granular inspection that defeats that optimization.

Perhaps we should ignore the approximate mode when DELREF/ACKED is used (and clarify this in the documentation)?

@LiorKogan agree with this way.
@sggeorgiev If the process is not continued, unless the client checks this reply, it is possible that the client will never discover that no entry has been deleted forever.

@LiorKogan @sundb @sggeorgiev if it's not too late, maybe we should block the combo "~" + "DELREF/ACKED"?
i.e. reply with error

that way it'll be very clear to the user that if he wants DELREF/ACKED he can't use "~"

@guybe7 - It is probably too late because we already released it in 8.2 in August. Anyway, ignoring the approximate flag causes no harm.

OK, in that case let's merge this PR and add a note in the documentation that the approximate flag is ignored when using DELREF/ACKED strategies.

However, there appears to be a fundamental conflict: approximate mode is designed to delete entire nodes from the stream radix tree to speed up deletion. But DELREF/ACKED strategies require checking each node messages before deletion. This means approximate mode and DELREF/ACKED seem inherently incompatible—the former prioritizes speed by removing full nodes, while the latter requires granular inspection that defeats that optimization.

I came upon this PR after running into the issue that it fixes, but I wonder about the above. I am not familiar with the internals of redis, but I would think that there are two parts in doing an ACKED delete:

1. Checking all entries in a node to see if they are acked, as indicated above
2. If you do non-approximate deletion, then you have to actually remove individual entries from the node, which (I imagine) requires moving data around.

It would seem that an approximate ACKED trim would still need to do 1, but could prevent having to do 2, which might still make it more efficient than a non-approximate ACKED trim (and more featureful than a KEEPREF delete, since AFAICS that will also delete items that are never seen by all consumer groups. Or am I misunderstanding this?).

1. Checking all entries in a node to see if they are acked, as indicated above
2. If you do non-approximate deletion, then you have to actually remove individual entries from the node, which (I imagine) requires moving data around.

@matthijskooijman Maybe you have misunderstood. These two ways are the same. It's not about checking first and then deleting, but checking and deleting simultaneously. Moreover, the memory won't be moved; it's just set to the deletion flag.

@matthijskooijman Maybe you have misunderstood. These two ways are the same. It's not about checking first and then deleting, but checking and deleting simultaneously. Moreover, the memory won't be moved; it's just set to the deletion flag.

Ah, thanks for clarifying. So then there is some extra bookkeeping to know that all deletion flags in a node are set and the entire node can be deleted?

And then in KEEPREF mode, approximate is useful because based on the MINID or MAXLEN you can just decide what nodes to delete based on their ID ranges, without having to look into the nodes themselves?

What I am looking for is an efficient way to delete old items, but prevent deleting items that are not seen/processed yet. Apparently ~ ACKED is not the solution here (works, but not efficient).

How would you achieve this then? Thinking on it, and looking at KEEPREF, would it make sense to run XTRIM ~ KEEPREF with MINID based on the oldest message not yet received by all consumer groups? This MINID could be determined by looking at the consumer group metadata only, without having to look at individual entries. This might end up deleting entries that were received but not ACKed yet, but IIUC then KEEPREF ensures thee entries are kept around in the pel list until they are ACKed, right? So this would effectively be the same as doing ~ ACKED?

Is this a correct interpretation? If so, is there a way to do this automatically, like a special entry id to pass to MINID to mean "the highest ID seen by all consumer groups", or would I need to parse XINFO output for this?

Maybe this is a bit off-topic for this PR, but hopefully you'll help me understand how this works :-)