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

Fix assertion error for messages_on_hold._activate_ordering_keys #913

Closed
acocuzzo opened this issue May 5, 2023 · 0 comments · Fixed by #911
Closed

Fix assertion error for messages_on_hold._activate_ordering_keys #913

acocuzzo opened this issue May 5, 2023 · 0 comments · Fixed by #911
Assignees
Labels
api: pubsub Issues related to the googleapis/python-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@acocuzzo
Copy link
Contributor

acocuzzo commented May 5, 2023

It is possible to call activate_ordering_keys with keys which are not present in messages_on_hold._pending_ordered_messages. This can happen if:

  1. A message (or messages) lease exceeds max_lease_duration, it is dropped from maintain_leases

  2. We call drop in the dispatcher

  3. If there are no messages remaining in messages_on_hold._pending_ordered_messages, therefore the pending_ordered_messages queue for the ordering key is removed:
    https://github.com/googleapis/python-pubsub/blob/main/google/cloud/pubsub_v1/subscriber/_protocol/messages_on_hold.py#L154

  4. The message is acked from the user callback, the ack completes and drop is called in the dispatcher:

  5. We call _activate_ordering_keys with a key that does not exist in messages_on_hold._pending_ordered_messages:
    https://github.com/googleapis/python-pubsub/blob/main/google/cloud/pubsub_v1/subscriber/_protocol/messages_on_hold.py#L122

Reproduction:

  1. Publish a single message with an ordering key:
  2. Subscribe with flow control max_lease_duration < time to ack the message, to ensure that the library will drop leasing prior to the message being acked.

Results:

Listening for messages...
Received b'message1'.
Dropping 1 items because they were leased too long.
Acking message with ordering key: key1 
AssertionError: A message queue should exist for every ordered message in flight.
@acocuzzo acocuzzo self-assigned this May 5, 2023
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/python-pubsub API. label May 5, 2023
@acocuzzo acocuzzo added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/python-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant