From a4bba9dd5c60842c5cd69900a552f7c1288e5149 Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Tue, 29 Mar 2022 13:45:27 -0700 Subject: [PATCH] Revert "PYTHON-2970 Prioritize electionId over setVersion for stale primary check (#845)" This reverts commit 225d131c2d3f6f0b4c46c130abb3e1452010ad40. --- doc/changelog.rst | 41 ++--- pymongo/topology_description.py | 29 ++-- .../rs/electionId_precedence_setVersion.json | 92 ----------- .../rs/null_election_id.json | 30 ++-- .../rs/secondary_ignore_ok_0.json | 2 +- .../rs/set_version_can_rollback.json | 149 ------------------ ...tversion_equal_max_without_electionid.json | 84 ---------- ...on_greaterthan_max_without_electionid.json | 84 ---------- .../rs/setversion_without_electionid.json | 12 +- .../rs/use_setversion_without_electionid.json | 32 ++-- 10 files changed, 67 insertions(+), 488 deletions(-) delete mode 100644 test/discovery_and_monitoring/rs/electionId_precedence_setVersion.json delete mode 100644 test/discovery_and_monitoring/rs/set_version_can_rollback.json delete mode 100644 test/discovery_and_monitoring/rs/setversion_equal_max_without_electionid.json delete mode 100644 test/discovery_and_monitoring/rs/setversion_greaterthan_max_without_electionid.json diff --git a/doc/changelog.rst b/doc/changelog.rst index 0bacb1bb7..d263d4534 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -4,40 +4,33 @@ Changelog Changes in Version 4.1 ---------------------- +.. warning:: PyMongo 4.1 drops support for Python 3.6.0 and 3.6.1, Python 3.6.2+ is now required. + PyMongo 4.1 brings a number of improvements including: -- :meth:`pymongo.collection.Collection.update_one`, - :meth:`pymongo.collection.Collection.update_many`, - :meth:`pymongo.collection.Collection.delete_one`, - :meth:`pymongo.collection.Collection.delete_many`, - :meth:`pymongo.collection.Collection.aggregate`, - :meth:`pymongo.collection.Collection.find_one_and_delete`, - :meth:`pymongo.collection.Collection.find_one_and_replace`, - :meth:`pymongo.collection.Collection.find_one_and_update`, - :meth:`pymongo.collection.Collection.find`, - and :meth:`pymongo.collection.Collection.replace_one `all support a new - keyword argument ``let`` which is a map of parameter names and values. +- Added support for the ``let`` parameter to + :meth:`~pymongo.collection.Collection.update_one`, + :meth:`~pymongo.collection.Collection.update_many`, + :meth:`~pymongo.collection.Collection.delete_one`, + :meth:`~pymongo.collection.Collection.delete_many`, + :meth:`~pymongo.collection.Collection.replace_one`, + :meth:`~pymongo.collection.Collection.aggregate`, + :meth:`~pymongo.collection.Collection.find_one_and_delete`, + :meth:`~pymongo.collection.Collection.find_one_and_replace`, + :meth:`~pymongo.collection.Collection.find_one_and_update`, + :meth:`~pymongo.collection.Collection.find`, + :meth:`~pymongo.collection.Collection.find_one`, + and :meth:`~pymongo.collection.Collection.bulk_write`. + ``let`` is a map of parameter names and values. Parameters can then be accessed as variables in an aggregate expression context. - :meth:`~pymongo.collection.Collection.aggregate` now supports $merge and $out executing on secondaries on MongoDB >=5.0. aggregate() now always obeys the collection's :attr:`read_preference` on MongoDB >= 5.0. -- :meth:`gridfs.GridOut.seek` now returns the new position in the file, to +- :meth:`gridfs.grid_file.GridOut.seek` now returns the new position in the file, to conform to the behavior of :meth:`io.IOBase.seek`. -Breaking Changes in 4.1 -....................... -- Removed support for Python 3.6.0 and 3.6.1, Python 3.6.2+ is now required. - -Bug fixes -......... - -- Fixed a bug where the client could be unable to discover the new primary - after a simultaneous replica set election and reconfig (`PYTHON-2970`_). - -.. _PYTHON-2970: https://jira.mongodb.org/browse/PYTHON-2970 - Issues Resolved ............... diff --git a/pymongo/topology_description.py b/pymongo/topology_description.py index 9f718376e..b3dd60680 100644 --- a/pymongo/topology_description.py +++ b/pymongo/topology_description.py @@ -17,7 +17,6 @@ from random import sample from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple -from bson.min_key import MinKey from bson.objectid import ObjectId from pymongo import common from pymongo.errors import ConfigurationError @@ -532,16 +531,24 @@ def _update_rs_from_primary( sds.pop(server_description.address) return (_check_has_primary(sds), replica_set_name, max_set_version, max_election_id) - new_election_tuple = server_description.election_id, server_description.set_version - max_election_tuple = max_election_id, max_set_version - new_election_safe = tuple(MinKey() if i is None else i for i in new_election_tuple) - max_election_safe = tuple(MinKey() if i is None else i for i in max_election_tuple) - if new_election_safe >= max_election_safe: - max_election_id, max_set_version = new_election_tuple - else: - # Stale primary, set to type Unknown. - sds[server_description.address] = server_description.to_unknown() - return _check_has_primary(sds), replica_set_name, max_set_version, max_election_id + max_election_tuple = max_set_version, max_election_id + if None not in server_description.election_tuple: + if ( + None not in max_election_tuple + and max_election_tuple > server_description.election_tuple + ): + + # Stale primary, set to type Unknown. + sds[server_description.address] = server_description.to_unknown() + return (_check_has_primary(sds), replica_set_name, max_set_version, max_election_id) + + max_election_id = server_description.election_id + + if server_description.set_version is not None and ( + max_set_version is None or server_description.set_version > max_set_version + ): + + max_set_version = server_description.set_version # We've heard from the primary. Is it the same primary as before? for server in sds.values(): diff --git a/test/discovery_and_monitoring/rs/electionId_precedence_setVersion.json b/test/discovery_and_monitoring/rs/electionId_precedence_setVersion.json deleted file mode 100644 index a7b49e2b9..000000000 --- a/test/discovery_and_monitoring/rs/electionId_precedence_setVersion.json +++ /dev/null @@ -1,92 +0,0 @@ -{ - "description": "ElectionId is considered higher precedence than setVersion", - "uri": "mongodb://a/?replicaSet=rs", - "phases": [ - { - "responses": [ - [ - "a:27017", - { - "ok": 1, - "helloOk": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000001" - }, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ], - [ - "b:27017", - { - "ok": 1, - "helloOk": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 2, - "electionId": { - "$oid": "000000000000000000000001" - }, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ], - [ - "a:27017", - { - "ok": 1, - "helloOk": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - }, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - } - }, - "b:27017": { - "type": "Unknown", - "setName": null, - "setVersion": null, - "electionId": null - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 1, - "maxElectionId": { - "$oid": "000000000000000000000002" - } - } - } - ] -} diff --git a/test/discovery_and_monitoring/rs/null_election_id.json b/test/discovery_and_monitoring/rs/null_election_id.json index 8eb519595..62120e844 100644 --- a/test/discovery_and_monitoring/rs/null_election_id.json +++ b/test/discovery_and_monitoring/rs/null_election_id.json @@ -123,18 +123,15 @@ "outcome": { "servers": { "a:27017": { - "type": "Unknown", - "setName": null, - "setVersion": null, - "electionId": null - }, - "b:27017": { "type": "RSPrimary", "setName": "rs", "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - } + "electionId": null + }, + "b:27017": { + "type": "Unknown", + "setName": null, + "electionId": null }, "c:27017": { "type": "Unknown", @@ -177,18 +174,15 @@ "outcome": { "servers": { "a:27017": { - "type": "Unknown", - "setName": null, - "setVersion": null, - "electionId": null - }, - "b:27017": { "type": "RSPrimary", "setName": "rs", "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - } + "electionId": null + }, + "b:27017": { + "type": "Unknown", + "setName": null, + "electionId": null }, "c:27017": { "type": "Unknown", diff --git a/test/discovery_and_monitoring/rs/secondary_ignore_ok_0.json b/test/discovery_and_monitoring/rs/secondary_ignore_ok_0.json index ee9519930..4c1cb011a 100644 --- a/test/discovery_and_monitoring/rs/secondary_ignore_ok_0.json +++ b/test/discovery_and_monitoring/rs/secondary_ignore_ok_0.json @@ -1,5 +1,5 @@ { - "description": "Secondary ignored when ok is zero", + "description": "New primary", "uri": "mongodb://a,b/?replicaSet=rs", "phases": [ { diff --git a/test/discovery_and_monitoring/rs/set_version_can_rollback.json b/test/discovery_and_monitoring/rs/set_version_can_rollback.json deleted file mode 100644 index 28ecbeefc..000000000 --- a/test/discovery_and_monitoring/rs/set_version_can_rollback.json +++ /dev/null @@ -1,149 +0,0 @@ -{ - "description": "Set version rolls back after new primary with higher election Id", - "uri": "mongodb://a/?replicaSet=rs", - "phases": [ - { - "responses": [ - [ - "a:27017", - { - "ok": 1, - "hello": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 2, - "electionId": { - "$oid": "000000000000000000000001" - }, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 2, - "electionId": { - "$oid": "000000000000000000000001" - } - }, - "b:27017": { - "type": "Unknown", - "setName": null, - "electionId": null - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 2, - "maxElectionId": { - "$oid": "000000000000000000000001" - } - } - }, - { - "_comment": "Response from new primary with newer election Id", - "responses": [ - [ - "b:27017", - { - "ok": 1, - "hello": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - }, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "Unknown", - "setName": null, - "electionId": null - }, - "b:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - } - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 1, - "maxElectionId": { - "$oid": "000000000000000000000002" - } - } - }, - { - "_comment": "Response from stale primary", - "responses": [ - [ - "a:27017", - { - "ok": 1, - "hello": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 2, - "electionId": { - "$oid": "000000000000000000000001" - }, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "Unknown", - "setName": null, - "electionId": null - }, - "b:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - } - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 1, - "maxElectionId": { - "$oid": "000000000000000000000002" - } - } - } - ] -} diff --git a/test/discovery_and_monitoring/rs/setversion_equal_max_without_electionid.json b/test/discovery_and_monitoring/rs/setversion_equal_max_without_electionid.json deleted file mode 100644 index 91e84d4fa..000000000 --- a/test/discovery_and_monitoring/rs/setversion_equal_max_without_electionid.json +++ /dev/null @@ -1,84 +0,0 @@ -{ - "description": "setVersion version that is equal is treated the same as greater than if there is no electionId", - "uri": "mongodb://a/?replicaSet=rs", - "phases": [ - { - "responses": [ - [ - "a:27017", - { - "ok": 1, - "helloOk": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 1, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": null - }, - "b:27017": { - "type": "Unknown", - "setName": null, - "electionId": null - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 1 - } - }, - { - "responses": [ - [ - "b:27017", - { - "ok": 1, - "helloOk": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 1, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "Unknown", - "setName": null, - "electionId": null - }, - "b:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": null - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 1 - } - } - ] -} diff --git a/test/discovery_and_monitoring/rs/setversion_greaterthan_max_without_electionid.json b/test/discovery_and_monitoring/rs/setversion_greaterthan_max_without_electionid.json deleted file mode 100644 index b15fd5c1a..000000000 --- a/test/discovery_and_monitoring/rs/setversion_greaterthan_max_without_electionid.json +++ /dev/null @@ -1,84 +0,0 @@ -{ - "description": "setVersion that is greater than maxSetVersion is used if there is no electionId", - "uri": "mongodb://a/?replicaSet=rs", - "phases": [ - { - "responses": [ - [ - "a:27017", - { - "ok": 1, - "helloOk": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 1, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": null - }, - "b:27017": { - "type": "Unknown", - "setName": null, - "electionId": null - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 1 - } - }, - { - "responses": [ - [ - "b:27017", - { - "ok": 1, - "helloOk": true, - "isWritablePrimary": true, - "hosts": [ - "a:27017", - "b:27017" - ], - "setName": "rs", - "setVersion": 2, - "minWireVersion": 0, - "maxWireVersion": 6 - } - ] - ], - "outcome": { - "servers": { - "a:27017": { - "type": "Unknown", - "setName": null, - "electionId": null - }, - "b:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 2, - "electionId": null - } - }, - "topologyType": "ReplicaSetWithPrimary", - "logicalSessionTimeoutMinutes": null, - "setName": "rs", - "maxSetVersion": 2 - } - } - ] -} diff --git a/test/discovery_and_monitoring/rs/setversion_without_electionid.json b/test/discovery_and_monitoring/rs/setversion_without_electionid.json index f59c162ae..2f68287f1 100644 --- a/test/discovery_and_monitoring/rs/setversion_without_electionid.json +++ b/test/discovery_and_monitoring/rs/setversion_without_electionid.json @@ -1,5 +1,5 @@ { - "description": "setVersion that is less than maxSetVersion is ignored if there is no electionId", + "description": "setVersion is ignored if there is no electionId", "uri": "mongodb://a/?replicaSet=rs", "phases": [ { @@ -63,14 +63,14 @@ "outcome": { "servers": { "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 2, + "type": "Unknown", + "setName": null, "electionId": null }, "b:27017": { - "type": "Unknown", - "setName": null, + "type": "RSPrimary", + "setName": "rs", + "setVersion": 1, "electionId": null } }, diff --git a/test/discovery_and_monitoring/rs/use_setversion_without_electionid.json b/test/discovery_and_monitoring/rs/use_setversion_without_electionid.json index 6dd753d5d..421ff57c8 100644 --- a/test/discovery_and_monitoring/rs/use_setversion_without_electionid.json +++ b/test/discovery_and_monitoring/rs/use_setversion_without_electionid.json @@ -71,23 +71,20 @@ "outcome": { "servers": { "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000001" - } - }, - "b:27017": { "type": "Unknown", "setName": null, "electionId": null + }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 2 } }, "topologyType": "ReplicaSetWithPrimary", "logicalSessionTimeoutMinutes": null, "setName": "rs", - "maxSetVersion": 1, + "maxSetVersion": 2, "maxElectionId": { "$oid": "000000000000000000000001" } @@ -118,25 +115,22 @@ "outcome": { "servers": { "a:27017": { - "type": "RSPrimary", - "setName": "rs", - "setVersion": 1, - "electionId": { - "$oid": "000000000000000000000002" - } - }, - "b:27017": { "type": "Unknown", "setName": null, "electionId": null + }, + "b:27017": { + "type": "RSPrimary", + "setName": "rs", + "setVersion": 2 } }, "topologyType": "ReplicaSetWithPrimary", "logicalSessionTimeoutMinutes": null, "setName": "rs", - "maxSetVersion": 1, + "maxSetVersion": 2, "maxElectionId": { - "$oid": "000000000000000000000002" + "$oid": "000000000000000000000001" } } }