When recovery is enabled more deliveries can be acked after recovery than the user expects

[caspermout]

I found the following code in the class RecoveryAwareChannelN:

@Override
public void basicAck(long deliveryTag, boolean multiple) throws IOException {
    long realTag = deliveryTag - activeDeliveryTagOffset;
    // 0 tag means ack all when multiple is set
    if (realTag > 0 || (multiple && realTag == 0)) {
        transmit(new Basic.Ack(realTag, multiple));
        metricsCollector.basicAck(this, deliveryTag, multiple);
    }
}

When activeDeliveryTagOffset and deliveryTag are the same (this can happen directly after a channel is reconnected/recovered) this method will ack all outstanding messages instead of acking nothing.

The output of the attached sample application contains PRECONDITION_FAILED - unknown delivery tag 1 because it is trying to ack all messages. But it was already acked by acking the first message after connection recovery.

example-output.txt
TestRabbitRecover.txt

• RabbitMQ version
  3.6.15

• Erlang version
  Erlang 19.3.6.7

• Client library version (for all libraries used)
  com.rabbitmq/amqp-client/4.5.0

[michaelklishin]

So what’s the proposed solution? However imperfect, this versioning feature has worked quite well for a number of years.

[michaelklishin]

@acogoluegnes do you have an opinion?

[caspermout]

See pull request #397

[michaelklishin]

@acogoluegnes I assume this can go into 4.8.0 safely?

[acogoluegnes]

@michaelklishin Yes, it can. We may have to release another RCs for 5.4.0 and 4.8.0, but we can also fit #398 in them.

[caspermout]

@michaelklishin I cannot find a 4.8.x version that contains this fix?

[acogoluegnes]

@caspermout Looks like it didn't make it to 4.x.x-stable. I'll backport it and release 4.8.3.

[acogoluegnes]

Didn't make it to 5.x.x-stable :-(

[caspermout]

Thank you

[acogoluegnes]

@caspermout 4.8.3 is released: https://groups.google.com/d/msg/rabbitmq-users/8HfMagQHGic/QMqvxa_MAQAJ

[caspermout]

Thanks!

[antenko]

Didn't make it to 5.x.x-stable :-(

Hi!
Are you going to release this bugfix in 5.3.x or 5.x.x?
We have the same issue..

[michaelklishin]

There were multiple 5.x releases since October 2018.

[michaelklishin]

There were multiple 5.x releases since October 2018.

2e6bf27 was cherry-picked to 5.x.x-stable and 5.6.x-stable as 1a8ffed.

[michaelklishin]

So 1a8ffed is available as of 5.5.3 and 5.7.0. I don't think it's in 5.6.0 but would be in 5.6.1 if it ships. I see no reason to stick to 5.3.x over any of the above versions, in particular 5.7.0.