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