celery / kombu

Messaging library for Python.
http://kombu.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
2.87k stars 928 forks source link

ConnectionPool can't be used after .resize(..., reset=True) (resolves #2018) #2024

Closed FrankK-1234 closed 3 months ago

FrankK-1234 commented 3 months ago

Hi, can you please have a look at this pull request. It addresses the issue #2018.

What did I do to tackle the issue. First I added several test case for the resize method (in test_connection.py), as they were not available. Basically the test test_acquire_resize_reset() triggered the given issue.

In short the use of force_close_all() (used also in other cases) caused the pool to be set on _closed = True. In the acquire method this resulted in the RuntimeError('Acquire on closed pool').

I've added a parameter close_pool in force_close_all with default of True (old behaviour as used by other callers), which allows the pool to be kept open if set to False. In the resize method, reset branch I've added force_close_all(close_pool=False).

PLEASE NOTE: While testing the resize I've found 2 issues related to resizing which I think are a problem. They are addressed as well.

Please have a look at the changes, Regards,

Frank

thedrow commented 3 months ago

Thank you for the contribution!

spumer commented 3 months ago

Good news!

Guys, only one question: why method setup() exists? Looks like pool can create new entry by calling .new() when it's required and limit not reached.

Due setup() calling we create "lazy" objects, then we check it "lazy" or not (in acquire()), and evaluate in prepare(). Looks like removing pre-filling pool should simplify Resource class and all related logic.