PYTHON-5517 Simplify pool backpressure behavior #2611
Conversation
| class PyMongoBaseProtocol(Protocol): | ||
| def __init__(self, timeout: Optional[float] = None): | ||
| self.transport: Transport = None # type: ignore[assignment] | ||
| self._timeout = timeout |
There was a problem hiding this comment.
I reverted these changes from #2509 since they were unrelated
blink1073
requested review from
Jibola and
ShaneHarvey
and removed request for
a team and
Jibola
| "SystemOverloadedError" | ||
| if isinstance(error, WaitQueueTimeoutError) or ( | ||
| error.has_error_label("SystemOverloadedError") | ||
| and error.has_error_label("RetryableError") |
There was a problem hiding this comment.
I'm curious. Do we need to check both error labels here?
There was a problem hiding this comment.
We're the ones adding the labels, would we want to add one but not the other?
There was a problem hiding this comment.
Right so can we simplify it to only check for "SystemOverloadedError"?
test/test_pooling.py
Outdated
| def test_pool_backoff_preserves_existing_connections(self): | ||
| def test_pool_backpressure_preserves_existing_connections(self): | ||
| client = self.rs_or_single_client() | ||
| coll = self.db.t |
There was a problem hiding this comment.
Shouldn't this line be coll = client.pymongo_test.t? Otherwise we're using two different clients.
pymongo/asynchronous/pool.py
Outdated
| # a closed connection. | ||
| # If found, set backoff and add error labels. | ||
| # Look for errors of type AutoReconnect and add error labels if appropriate. | ||
| if self.is_sdam or type(error) != AutoReconnect: |
There was a problem hiding this comment.
Since we now want to label NetworkTimeouts on connect as overload, should this be isinstance(error, AutoReconnect)?
There was a problem hiding this comment.
Added NetworkTimeout explicitly
There was a problem hiding this comment.
Oh good, this caught a logic error in the backoff network timeout test
2 participants
Replaces #2598