PYTHON-4636 - Avoid blocking I/O calls in async code paths #1870
Conversation
|
We should use the loop.recv_into approach in #1871 to avoid any blocking calls. |
NoahStapp
force-pushed
the
PYTHON-4636
branch
from
7fa5d47
to
7f71430
Compare
pymongo/network_layer.py
Outdated
| finished = next(iter(result[0])) | ||
| next(iter(result[1])).cancel() | ||
| if finished == read_task: | ||
| return finished.result() # type: ignore[return-value] |
There was a problem hiding this comment.
If we used read_task.result() would that avoid the type error?
pymongo/network_layer.py
Outdated
| read += conn.recv_into(mv[read:]) | ||
| if read == 0: | ||
| raise OSError("connection closed") |
There was a problem hiding this comment.
This should be something like:
read = conn.recv_into(mv[read:])
if read == 0:
raise OSError("connection closed")
total_read += read
pymongo/network_layer.py
Outdated
| try: | ||
| read = conn.recv_into(mv[total_read:]) | ||
| except BLOCKING_IO_ERRORS: | ||
| await asyncio.sleep(0.5) |
There was a problem hiding this comment.
RIP windows perf. Can you schedule some of the windows tasks to see the impact?
For anyone else following along we'll fix this when we migrate to asyncio streams in: https://jira.mongodb.org/browse/PYTHON-4493
There was a problem hiding this comment.
This isn't a new change, it's already in master:
mongo-python-driver/pymongo/network_layer.py
Line 127 in 0f84ad6
There was a problem hiding this comment.
Right but the existing code was only for send() whereas this new code is for recv so the perf impact could be different.
There was a problem hiding this comment.
Good point, I'll schedule some Windows tasks.
|
Looks like there are still a few bugs:
|
|
The test_find_raw_retryable_reads error persists. |
Our theory is that #1875 will fix it, the error is caused by the retried command timing out on server selection. We think that's due to the condition lock deadlocking across background threads. |
|
Cool, let's merge that one first then to verify. |
|
I think we have to defer this PR, we're seeing failures in |
All tests passing after merging master in. |
|
Looks like there are still some test failures. I just scheduled some Windows and macOS tasks as well. |
Yeah they seem to be flakey, none consistently fail so far. |
|
I fixed PYTHON-4798 which will make MacOS and Windows run again in PRs. |
|
|
Sure any one task may be flaky at a low probability but looking at the whole patch about 10% of tasks are failing. That's something we need to fix. Also on Windows test_network_error_message seems to block forever. |
Agreed that we need to fix them, I meant that there wasn't a consistent pattern in which tests fail and how often. |
…writer PYTHON-4636 Always remove reader/writer callbacks
PYTHON-4636 Properly check for socket close on windows
PYTHON-4636 Fix windows send/recv perf
| self._run() | ||
| else: | ||
| asyncio.run(self._run()) | ||
| self._run() |
There was a problem hiding this comment.
How was this test failing in async?
There was a problem hiding this comment.
It was trying to call asyncio.run while in an active loop since it's an async test, which isn't supported.
No description provided.