PYTHON-5369 - Re-raise socket.timeout errors if the deadline has alre… #2326
Conversation
…ady been execeeded
|
On second thought, I intentionally added the non-blocking read to avoid spurious reconnects on SDAM connections when using the driver in FaaS services that pause/resume the process, see https://jira.mongodb.org/browse/PYTHON-3186. I'm not sure we can accept this change. Although, if the test_sigstop_sigcont regression test still passes, perhaps we can? |
Looks like that regression test isn't passing... |
|
Can we not use the FaaS detection logic and preserve the current behavior in that case? |
|
Since |
My vote is yes |
test/test_client.py
Outdated
| @@ -1967,6 +1967,7 @@ def test_srv_max_hosts_kwarg(self): | |||
| "loadBalanced clients do not run SDAM", | |||
| ) | |||
| @unittest.skipIf(sys.platform == "win32", "Windows does not support SIGSTOP") | |||
| @unittest.skipUnless(_is_faas(), "Non-FaaS environments raise timeouts faster") | |||
There was a problem hiding this comment.
We can't skip this test like this because we don't run the test suite on faas platforms. We want this test to run locally.
There was a problem hiding this comment.
By "run locally", what exactly do you mean? As in we can't do this change because we need this test to run in our regular test suite?
There was a problem hiding this comment.
Perhaps instead we can force the metadata to be faas through monkeypatching
test/asynchronous/test_client.py
Outdated
| @@ -117,7 +117,7 @@ | |||
| WriteConcernError, | |||
| ) | |||
| from pymongo.monitoring import ServerHeartbeatListener, ServerHeartbeatStartedEvent | |||
| from pymongo.pool_options import _MAX_METADATA_SIZE, _METADATA, ENV_VAR_K8S, PoolOptions | |||
| from pymongo.pool_options import _MAX_METADATA_SIZE, _METADATA, ENV_VAR_K8S, PoolOptions, _is_faas | |||
I'm running some patches right now to confirm, but so far it appears to fix the flakes on non-PyPy variants. PyPy still has this issue, so I propose we skip |
I have not seen any failures of these tests flaking on sync + CPython with this fix. |
| or not _is_faas() | ||
| and deadline is not None | ||
| and deadline - time.monotonic() < 0 | ||
| ): |
There was a problem hiding this comment.
I think we want sigstop/sigcont to work the same regardless of what env variables are defined. What if we take an alternative approach here and replace not _is_faas() with not conn.is_sdam? IE only do the extra non-blocking read on SDAM connections.
Will that still fix the flaky tests?
…ady been execeeded