• Binbin's avatar
    Fix propagation of entries_read by calling streamPropagateGroupID unconditionally (#12898) · f17381a3
    Binbin authored
    In XREADGROUP ACK, because streamPropagateXCLAIM does not propagate
    entries-read, entries-read will be inconsistent between master and
    replicas.
    I.e. if no entries were claimed, it would have propagated correctly, but
    if some
    were claimed, then the entries-read field would be inconsistent on the
    replica.
    
    The fix was suggested by guybe7, call streamPropagateGroupID
    unconditionally,
    so that we will normalize entries_read on the replicas. In the past, we
    would
    only set propagate_last_id when NOACK was specified. And in #9127,
    XCLAIM did
    not propagate entries_read in ACK, which would cause entries_read to be
    inconsistent between master and replicas.
    
    Another approach is add another arg to XCLAIM and let it propagate
    entries_read,
    but we decided not to use it. Because we want minimal damage in case
    there's an
    old target and new source (in the worst case scenario, the new source
    doesn't
    recognize XGROUP SETID ... ENTRIES READ and the lag is lost. If we
    change XCLAIM,
    the damage is much more severe).
    
    In this patch, now if the user uses XREADGROUP .. COUNT 1 there will be
    an additional
    overhead of MULTI, EXEC and XGROUPSETID. We assume the extra command in
    case of
    COUNT 1 (4x factor, changing from one XCLAIM to
    MULTI+XCLAIM+XSETID+EXEC), is probably
    ok since reading just one entry is in any case very inefficient (a
    client round trip
    per record), so we're hoping it's not a common case.
    
    Issue was introduced in #9127.
    f17381a3
t_stream.c 157 KB