Close connections in case of an exception #2201
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Nusnus
requested changes
Dec 5, 2024
|
Sure, I'll also have a look at the tests that failed. |
You can run it locally with |
MZauchner
force-pushed
the
main
branch
2 times, most recently
from
82e31c9 to
8869895
Compare
|
@Nusnus Thanks a lot for your tip with the tests! I added an integration test to the py_amqp tests that should fail without the change. |
MZauchner
force-pushed
the
main
branch
2 times, most recently
from
f5f43c1 to
388b7c8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2201 +/- ##
==========================================
+ Coverage 81.49% 81.50% +0.01%
==========================================
Files 77 77
Lines 9509 9522 +13
Branches 1148 1151 +3
==========================================
+ Hits 7749 7761 +12
Misses 1569 1569
- Partials 191 192 +1 ☔ View full report in Codecov by Sentry. |
MZauchner
force-pushed
the
main
branch
3 times, most recently
from
d49e6f6 to
2c3bd2c
Compare
Nusnus
requested changes
Dec 6, 2024
See also: |
MZauchner
commented
Dec 7, 2024
MZauchner
force-pushed
the
main
branch
2 times, most recently
from
d79f9dc to
0839f26
Compare
When an exception occurs while the connection is in use, the connection may become unsuable. Such connections need to be closed, such that the connection to the broker can be reestablished at a later point.
for more information, see https://pre-commit.ci
Nusnus
approved these changes
Dec 12, 2024
zs-neo
pushed a commit
to zs-neo/kombu
that referenced
this pull request
Dec 24, 2024
* Close connections in case of an exception When an exception occurs while the connection is in use, the connection may become unsuable. Such connections need to be closed, such that the connection to the broker can be reestablished at a later point. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
zs-neo
pushed a commit
to zs-neo/kombu
that referenced
this pull request
Dec 24, 2024
* Close connections in case of an exception When an exception occurs while the connection is in use, the connection may become unsuable. Such connections need to be closed, such that the connection to the broker can be reestablished at a later point. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
zs-neo
pushed a commit
to zs-neo/kombu
that referenced
this pull request
Dec 24, 2024
* Close connections in case of an exception When an exception occurs while the connection is in use, the connection may become unsuable. Such connections need to be closed, such that the connection to the broker can be reestablished at a later point. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: celery/celery#9259
Issue
When an amqp exception occurs while a connection is in use, the connection becomes unusable. Any attempts to use the connection can potentially block indefinitely.
Currently, such broken connections are placed back into the connection pool, which causes a later
acquirecall on the pool to return a broken connection.Reproducer
See celery/celery#9259
When a celery app is created with the
confirm_publish=Trueoption, it is possible for a celery task's `apply_async' method to block indefinitely if a broken connection is retrieved from the connection pool.Fix
The
ProducerPool::acquireandConnectionPool::acquiremethod now return a wrappedConnection/Producer, that ensures the connections are properly closed in case of an exception. A reproducer for the original bug was added as an integration test, to ensure that the fix worked.Additional comments
I believe there is a similar issue with the
ChannelPool, but in this case I think it is not possible to simply close the connection, as other channels acquired from the pool may still be in use.