Skip to content

fix: dispatch AMQP acknowledgements to the owning connection - #146

Draft
robjarawan wants to merge 2 commits into
developmentfrom
fix/ack-owning-connection
Draft

fix: dispatch AMQP acknowledgements to the owning connection#146
robjarawan wants to merge 2 commits into
developmentfrom
fix/ack-owning-connection

Conversation

@robjarawan

Copy link
Copy Markdown
Owner

Closes #145.

I changed Gather.ack() to check the existing connection_id before handing an AMQP acknowledgement to a consumer. Previously, the first consumer on the same broker erased the second consumer's ack_id, so the owning connection never acknowledged that delivery.

I reproduced this with a disposable RabbitMQ 4.3.5, parsed subscriptions, real gather/acknowledgement calls and a queue check after closing the consumers. Both amqp and amqpconsumer redelivered subscription 1's message on the baseline. With the fix, neither backend redelivers either message. The single-subscription controls pass before and after.

Check Baseline Fix
Local RabbitMQ, two subscriptions per backend Second delivery returned on both backends No redelivery
Local RabbitMQ, one subscription per backend Pass Pass
Transport-isolated regressions and recovery controls 4 failed, 7 passed 11 passed
Broader unit suite, excluding Azure's external-emulator tests See existing baseline evidence 364 passed, 1 skipped, 1 failed

The remaining broader failure is the existing clock-dependent test_schedules. I kept the stale-connection, changed-channel and transport-error controls so the fix preserves the recovery behavior from upstream #1155 and #1278. No ACK retry loop or reconnect was added. Protocols without connection_id keep their existing dispatch behavior.

python -m pytest -q tests/sarracenia/flowcb/gather/ack_owner_test.py

For the real-broker reproduction, start a disposable RabbitMQ on loopback with the standard local guest fixture account, then run:

python tests/sarracenia/flowcb/gather/ack_owner_broker.py

The script creates uniquely named queues/exchanges, publishes synthetic messages, reports actual redeliveries and removes only its own resources. The same script exits 1 on the baseline and 0 with the fix.

The runtime fix and tests are in 11ac81734. I carried the separate CI repair from #140 for local maintenance fixtures and exclusion of live-feed dynamic_flow jobs. This remains a draft pending the broader flow matrix and independent review.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Test Results

385 tests   384 ✅  1m 42s ⏱️
  1 suites    1 💤
  1 files      0 ❌

Results for commit 24141ec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Route AMQP acknowledgements to the receiving connection

1 participant