On Dec 6, 2013, at 11:15 AM, Michael Bayer <[email protected]> wrote:
>
>> Since the overflow lock seems to be to only maintain overflow count, I
>> suggest that we increment the counter *before* connection attempt, don't
>> hold the lock during connection attempt and then decrement the counter in
>> case of an error. If there is interest in doing this, I shall find time for
>> a patch and possibly a test case.
starting again, I think the part I missed is that you’re saying the creation of
a new connection is the part that hung, not the waiting for existing
connections to be available. So suppose _create_connection() is hanging, but
there’s plenty of overflow available - other threads should still be able to go
in and all access _create_connection().
the proposal is simple enough:
diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index 34681ef..219a98f 100644
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -812,11 +812,14 @@ class QueuePool(Pool):
self._overflow >= self._max_overflow:
return self._do_get()
else:
- con = self._create_connection()
self._overflow += 1
- return con
finally:
self._overflow_lock.release()
+ try:
+ return self._create_connection()
+ except:
+ self._overflow -= 1
+ raise
def recreate(self):
self.logger.info("Pool recreating”)
the hard part is producing a test case. I noticed just now that even if I
take overflow_lock out entirely, all the current tests pass, but this is
because it’s not easy for tests to catch race conditions like that. To test
the new change, it should be simpler, inject a mock connection that will hang
on one attempt and pass on another, then ensure that the second attempt
successfully connects within the overflow range before the “hanging” one does
(or errors out).
Also can you confirm the MySQL behavior here is such that only arbitrary
connection attempts are hanging? That is, a subsequent connection attempt
succeeds while the previous one continues to hang - otherwise I’m not sure how
this patch improves the situation.
signature.asc
Description: Message signed with OpenPGP using GPGMail
