PYTHON-5517 Simplify pool backpressure behavior#2611
PYTHON-5517 Simplify pool backpressure behavior#2611blink1073 merged 49 commits intomongodb:backpressurefrom
Conversation
| @@ -256,7 +256,6 @@ def __init__(self, timeout: Optional[float] = None): | |||
| 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
pymongo/synchronous/topology.py
Outdated
| "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"?
There was a problem hiding this comment.
Okay fair enough, done
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
ShaneHarvey
left a comment
ShaneHarvey
left a comment
There was a problem hiding this comment.
This looks good to me besides the print statement.
pymongo/asynchronous/pool.py
Outdated
| # End of file errors are excluded, because the server may have disconnected | ||
| # during the handshake. | ||
| if not isinstance(error.__cause__, (ssl.SSLEOFError, ssl.SSLZeroReturnError)): | ||
| print("Ignoring root error", error.__cause__, type(error.__cause__)) # noqa: T201 |
There was a problem hiding this comment.
Will have to remember to remove this print once we're ready to merge.
|
@NoahStapp can you please give one more approval? I just removed the print statement that Shane asked for. |
…tion rate limiter triggers (#2509) Co-authored-by: Iris <58442094+sleepyStick@users.noreply.github.com> Co-authored-by: Noah Stapp <noah.stapp@mongodb.com> Co-authored-by: Shane Harvey <shnhrv@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> PYTHON-5629 Increase max overload retries from 3 to 5 and initial delay from 50ms to 100ms (#2599) PYTHON-5517 Simplify pool backpressure behavior (#2611) synchro update network_layer update pool shared update pool shared update run-tests
3 participants
Replaces #2598
PYTHON-5517