-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-5536 Avoid clearing the connection pool when the server connection rate limiter triggers #2509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PYTHON-5536 Avoid clearing the connection pool when the server connection rate limiter triggers #2509
Changes from 61 commits
d24b4a5
3a26119
db3d3c7
f7b94be
9a9a65c
5e96353
e08284b
ddf9508
3ebd934
cd4e5db
9892e1b
1179c5c
bc91967
8c361be
0d4c84e
f51e8a5
c1fe2e3
7584d2d
f1544aa
9d34e52
bb5ac35
957a87d
9d0af17
da0c0e5
70b4113
c974d36
845f17a
09fc66d
84478d0
532c1b8
6890c73
6f38a9b
0997248
320cb54
7734af7
452cdd6
1f44c48
fa5c151
0ab78e4
07d0233
771570d
6623261
f602d4c
64aa0af
7f6335e
6db793d
8c2eb91
a84a181
c5ce8dd
679807e
7e9f19f
b0b5800
a033c58
ded90b0
2cd3c18
0b4b265
7548f7b
5e34bdc
f1294dc
6a47fc6
6d57549
9fa3a6f
47e55b4
4f06bb6
3c760e7
4cc6619
eaab484
65832a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -256,6 +256,7 @@ def __init__(self, timeout: Optional[float] = None): | |
self._timeout = timeout | ||
self._closed = asyncio.get_running_loop().create_future() | ||
self._connection_lost = False | ||
self._closing_exception = None | ||
|
||
def settimeout(self, timeout: float | None) -> None: | ||
self._timeout = timeout | ||
|
@@ -269,9 +270,11 @@ def close(self, exc: Optional[Exception] = None) -> None: | |
self.transport.abort() | ||
self._resolve_pending(exc) | ||
self._connection_lost = True | ||
self._closing_exception = exc # type:ignore[assignment] | ||
|
||
def connection_lost(self, exc: Optional[Exception] = None) -> None: | ||
self._resolve_pending(exc) | ||
self._closing_exception = exc # type:ignore[assignment] | ||
if not self._closed.done(): | ||
self._closed.set_result(None) | ||
|
||
|
@@ -335,8 +338,11 @@ async def read(self, request_id: Optional[int], max_message_size: int) -> tuple[ | |
if self._done_messages: | ||
message = await self._done_messages.popleft() | ||
else: | ||
if self.transport and self.transport.is_closing(): | ||
raise OSError("connection is already closed") | ||
if self._closed.done(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm let me try that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it is ambiguous as to whether |
||
if self._closing_exception: | ||
raise self._closing_exception | ||
else: | ||
raise OSError("connection closed") | ||
read_waiter = asyncio.get_running_loop().create_future() | ||
self._pending_messages.append(read_waiter) | ||
try: | ||
|
@@ -474,6 +480,7 @@ def _resolve_pending(self, exc: Optional[Exception] = None) -> None: | |
else: | ||
msg.set_exception(exc) | ||
self._done_messages.append(msg) | ||
self._pending_messages.clear() | ||
|
||
|
||
class PyMongoKMSProtocol(PyMongoBaseProtocol): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we remove this print now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to log it somehow? In the Server Tech Design they call out "A message will be logged and the function will be returned from."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we'll add a CMAP log message and/or an a new event. Feel free to add a log message now or we can defer to the spec PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added