Skip to content

PYTHON-4298 Fix _apply_local_threshold TypeError #1566

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

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

Ale-Cas
Copy link
Contributor

@Ale-Cas Ale-Cas commented Mar 29, 2024

Fix for TypeError caused by substraction between nulls when round_trip_time is None

    return [
>       s for s in selection.server_descriptions if (s.round_trip_time - fastest) <= threshold
    ]
E   TypeError: unsupported operand type(s) for -: 'NoneType' and 'NoneType'

Jira ticket: https://jira.mongodb.org/browse/PYTHON-4298
Linked to previous PR #1361

@blink1073 blink1073 requested a review from NoahStapp March 29, 2024 12:23
Copy link
Member

@ShaneHarvey ShaneHarvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Ale-Cas, thanks for the PR. Instead of silently ignoring this case, it would be great to detect when round_trip_time is None and raise an informative error. Something like:

for server in selection.server_descriptions:
    if server.round_trip_time is None:
        raise ConfigurationError(f"round_trip_time for server {server.address} is unexpectedly None: {self}, servers: {selection.server_descriptions}")

The goal is to help us isolate the underlying problem. Are you able to reproduce this issue?

@ShaneHarvey
Copy link
Member

Based on this traceback:

    return [
>       s for s in selection.server_descriptions if (s.round_trip_time - fastest) <= threshold
    ]
E   TypeError: unsupported operand type(s) for -: 'NoneType' and 'NoneType'

This means that both s.round_trip_time and fastest are None. The only way fastest could be None is if there is only a single server in selection.server_descriptions, otherwise fastest = min(...) line would have raised the TypeError:

>>> print(min([None, 1.0]))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '<' not supported between instances of 'float' and 'NoneType'
>>> print(min([None]))
None

@ShaneHarvey
Copy link
Member

Is your application using the "server_selector" argument to MongoClient? If so, could you share how it's defined?

          - `server_selector`: (callable or None) Optional, user-provided
            function that augments server selection rules. The function should
            accept as an argument a list of
            :class:`~pymongo.server_description.ServerDescription` objects and
            return a list of server descriptions that should be considered
            suitable for the desired operation.

@Ale-Cas
Copy link
Contributor Author

Ale-Cas commented Apr 3, 2024

Hi @ShaneHarvey thanks for the review, I'll address your suggestion and raise that exception.
I wasn't able to reproduce the issue locally (only happens during the CI) but maybe I can share more info on the client and replica set configuration so we can troubleshoot it.

Is your application using the "server_selector" argument to MongoClient? If so, could you share how it's defined?

          - `server_selector`: (callable or None) Optional, user-provided
            function that augments server selection rules. The function should
            accept as an argument a list of
            :class:`~pymongo.server_description.ServerDescription` objects and
            return a list of server descriptions that should be considered
            suitable for the desired operation.

No, we're not passing the server_selector argument in the client.

This is the rest of the client configuration:

pymongo.MongoClient(
            self.config.connection_string,
            connect=True,
            tz_aware=True,
            retryWrites=False,
            retryReads=True,
            minPoolSize=4,
            maxPoolSize=100,
            maxIdleTimeMS=3 * 60 * 1000,
            socketTimeoutMS=8 * 1000,
            connectTimeoutMS=8 * 1000,
            serverSelectionTimeoutMS=29 * 1000,
        )

the application is also using a different session for each thread to perform any type of mongodb operation.

That type error usually pops up during the CI, which is using this image ghcr.io/zcube/bitnami-compat/mongodb:6.0-debian-11-r64 to spin up a basic replica set (I can also share the mongo services we're using in docker compose if you think it's helpful).
Should we explicitly describe the mongo services using the server_selector argument?

@ShaneHarvey
Copy link
Member

Should we explicitly describe the mongo services using the server_selector argument?

No, server_selector is a power-user type feature only useful in very niche circumstances.

I'll address your suggestion and raise that exception.

Thanks!

@Ale-Cas
Copy link
Contributor Author

Ale-Cas commented Apr 4, 2024

Hi @ShaneHarvey,
I addressed your suggestion and I'm raising the configuration error now, can you take a second look, please?
Also linting should be ok now.

I had again that error in the CI today, here's the full pymongo traceback:

for doc in collection.find(
  File "/opt/venv/lib/python3.8/site-packages/pymongo/cursor.py", line 1264, in next
    if len(self.__data) or self._refresh():
  File "/opt/venv/lib/python3.8/site-packages/pymongo/cursor.py", line 1181, in _refresh
    self.__send_message(q)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/cursor.py", line 1060, in __send_message
    response = client._run_operation(
  File "/opt/venv/lib/python3.8/site-packages/pymongo/_csot.py", line 107, in csot_wrapper
    return func(self, *args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/mongo_client.py", line 1394, in _run_operation
    return self._retryable_read(
  File "/opt/venv/lib/python3.8/site-packages/pymongo/mongo_client.py", line 1492, in _retryable_read
    return self._retry_internal(
  File "/opt/venv/lib/python3.8/site-packages/pymongo/_csot.py", line 107, in csot_wrapper
    return func(self, *args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/mongo_client.py", line 1453, in _retry_internal
    return _ClientConnectionRetryable(
  File "/opt/venv/lib/python3.8/site-packages/pymongo/mongo_client.py", line 2315, in run
    return self._read() if self._is_read else self._write()
  File "/opt/venv/lib/python3.8/site-packages/pymongo/mongo_client.py", line 2437, in _read
    self._server = self._get_server()
  File "/opt/venv/lib/python3.8/site-packages/pymongo/mongo_client.py", line 2400, in _get_server
    return self._client._select_server(
  File "/opt/venv/lib/python3.8/site-packages/pymongo/mongo_client.py", line 1303, in _select_server
    server = topology.select_server(server_selector)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/topology.py", line 302, in select_server
    server = self._select_server(selector, server_selection_timeout, address)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/topology.py", line 286, in _select_server
    servers = self.select_servers(selector, server_selection_timeout, address)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/topology.py", line 237, in select_servers
    server_descriptions = self._select_servers_loop(selector, server_timeout, address)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/topology.py", line 252, in _select_servers_loop
    server_descriptions = self._description.apply_selector(
  File "/opt/venv/lib/python3.8/site-packages/pymongo/topology_description.py", line 330, in apply_selector
    return self._apply_local_threshold(selection)
  File "/opt/venv/lib/python3.8/site-packages/pymongo/topology_description.py", line 272, in _apply_local_threshold
    return [
  File "/opt/venv/lib/python3.8/site-packages/pymongo/topology_description.py", line 275, in <listcomp>
    if (cast(float, s.round_trip_time) - fastest) <= threshold
TypeError: unsupported operand type(s) for -: 'NoneType' and 'NoneType'

@ShaneHarvey
Copy link
Member

ShaneHarvey commented Apr 4, 2024

@Ale-Cas I'm going to merge this after the test suite completes. Could you install this branch in your CI and provide us an example of the new exception?

The new error will look something like:

pymongo.errors.ConfigurationError: round_trip_time for server ('localhost', 27019) is unexpectedly None: <TopologyDescription id: 660ef6814d5b0d196ee0f1e4, topology_type: ReplicaSetWithPrimary, servers: [<ServerDescription ('localhost', 27017) server_type: RSSecondary, rtt: 0.004299714891240001>, <ServerDescription ('localhost', 27018) server_type: RSSecondary, rtt: 0.007958880392834544>, <ServerDescription ('localhost', 27019) server_type: RSPrimary, rtt: 0.0067411982174962765>]>, servers: [<ServerDescription ('localhost', 27019) server_type: Unknown, rtt: None>]

@ShaneHarvey ShaneHarvey merged commit 167b964 into mongodb:master Apr 4, 2024
@Ale-Cas
Copy link
Contributor Author

Ale-Cas commented Apr 4, 2024

@ShaneHarvey thanks for your help!
Is there a plan to release this fix as part of pymongo 4.6.3?
Because, if possible, I'd prefer to bump the package using the new semantic version rather than the master branch.

And, as soon as I get the configuration error in the CI, I'll send it to you.

ShaneHarvey pushed a commit that referenced this pull request Apr 4, 2024
…me is None in server selection (#1566)

Co-authored-by: Alessio <alessio.castrica@investsuite.com>
(cherry picked from commit 167b964)
@ShaneHarvey
Copy link
Member

ShaneHarvey commented Apr 4, 2024

Ideally we can debug this issue and fix the underlying bug before we release. I backported this commit to the v4.6 branch here: d6248e9

Can you install the backported commit like this? python3 -m pip install https://github.com/mongodb/mongo-python-driver/archive/d6248e9cf1f22f71b1986a3e5c17ad43574ecc0d.tar.gz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants