Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qpid transport does not ack all messages when used with Celery's acks_late=True #563

Closed
bmbouter opened this issue Jan 25, 2016 · 2 comments

Comments

@bmbouter
Copy link
Contributor

Celery uses delivery_tag to track messages which need to be ACKed. The Qpid transport is using integers which are unique per Channel, but are not globally unique. As such, a celery worker will read in many messages and if it receives two of the same values for delivery_tag before the first one has acked, the first one will not be acked by celery.

Here is a rough recap of what I observed in celery's ack handling when receiving two messages, each with delivery_tag = 1 from two senders (A, B):

message 1 is received from dispatcher A
message 1 is received from dispatcher B
message 1 from dispatcher A completes and is acked
message 1 from dispatcher B completes but is NOT acked

I expect there to be 0 unacked messages in the queue since all messages succeeded.

@bmbouter
Copy link
Contributor Author

The resolution is to switch the Qpid transport to using UUIDs as the delivery_tags it generates during basic_publish().

@bmbouter
Copy link
Contributor Author

This fix been merged so I'm going to close this issue as resolved. I expect it to work in the 3.0.34 release.

bmbouter pushed a commit to bmbouter/kombu that referenced this issue Feb 25, 2017
Celery 4.0.2 passes the `multiple` keyword argument to `basic_ack`.
This did not used to occur with 3.1.20- so this change is only being
merged into the 4.0 branch. The desired functionality of this param is
documented here [0], but the Qpid transport uses UUIDs as the
delivery_tags so we don't have a record of the sequential messages
required to implement this. We use UUIDs as the deliver_tag to avoid
Issue celery#563.

With the functionality for the multiple parameter isn't implemented an
assertion is present to raise an AssertionError if Celery uses the
parameter with the Qpid transport. A user who encounters this
AssertionError should file a bug with Kombu.

[0] http://amqp.readthedocs.io/en/latest/reference/amqp.connection.html#amqp.connection.Connection.Channel.basic_ack

closes celery#699
bmbouter pushed a commit to bmbouter/kombu that referenced this issue Feb 25, 2017
Celery 4.0.2 passes the `multiple` keyword argument to `basic_ack`.
This did not used to occur with 3.1.20- so this change is only being
merged into the 4.0 branch. The desired functionality of this param is
documented here [0], but the Qpid transport uses UUIDs as the
delivery_tags so we don't have a record of the sequential messages
required to implement this. We use UUIDs as the deliver_tag to avoid
Issue celery#563.

With the functionality for the `multiple` parameter not implemented, an
AssertionError is raised if Celery attempts to meaningfully use the
`multiple` parameter with the Qpid transport. A developer or user who
encounters this AssertionError should file a bug with Kombu.

[0] http://amqp.readthedocs.io/en/latest/reference/amqp.connection.html#amqp.connection.Connection.Channel.basic_ack

closes celery#699
bmbouter added a commit that referenced this issue Feb 26, 2017
Celery 4.0.2 passes the `multiple` keyword argument to `basic_ack`.
This did not used to occur with 3.1.20- so this change is only being
merged into the 4.0 branch. The desired functionality of this param is
documented here [0], but the Qpid transport uses UUIDs as the
delivery_tags so we don't have a record of the sequential messages
required to implement this. We use UUIDs as the deliver_tag to avoid
Issue #563.

With the functionality for the `multiple` parameter not implemented, an
AssertionError is raised if Celery attempts to meaningfully use the
`multiple` parameter with the Qpid transport. A developer or user who
encounters this AssertionError should file a bug with Kombu.

[0] http://amqp.readthedocs.io/en/latest/reference/amqp.connection.html#amqp.connection.Connection.Channel.basic_ack

closes #699
bmbouter added a commit that referenced this issue Jun 20, 2017
Celery 4.0.2 passes the `multiple` keyword argument to `basic_ack`.
This did not used to occur with 3.1.20- so this change is only being
merged into the 4.0 branch. The desired functionality of this param is
documented here [0], but the Qpid transport uses UUIDs as the
delivery_tags so we don't have a record of the sequential messages
required to implement this. We use UUIDs as the deliver_tag to avoid
Issue #563.

With the functionality for the `multiple` parameter not implemented, an
AssertionError is raised if Celery attempts to meaningfully use the
`multiple` parameter with the Qpid transport. A developer or user who
encounters this AssertionError should file a bug with Kombu.

[0] http://amqp.readthedocs.io/en/latest/reference/amqp.connection.html#amqp.connection.Connection.Channel.basic_ack

closes #699
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant