From ac6ed4687f5934cee8fba83977c2d1038e2a97ae Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 19:37:19 -0500 Subject: [PATCH 1/9] PYTHON-6040 Preserve 1:1 index correspondence in client metadata --- pymongo/driver_info.py | 2 + pymongo/pool_options.py | 91 +++++++++++++++++++++++++++-------------- 2 files changed, 62 insertions(+), 31 deletions(-) diff --git a/pymongo/driver_info.py b/pymongo/driver_info.py index 18a51ae638..d4c444ce44 100644 --- a/pymongo/driver_info.py +++ b/pymongo/driver_info.py @@ -42,5 +42,7 @@ def __new__( raise TypeError( f"Wrong type for DriverInfo {key} option, value must be an instance of str, not {type(value)}" ) + if value and "|" in value: + raise ValueError(f"DriverInfo {key} must not contain the '|' delimiter") return self diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 8b26b4baf2..68d24ef61e 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -235,25 +235,51 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: if encoded_size <= _MAX_METADATA_SIZE: return # 5. Truncate driver info. - overflow = encoded_size - _MAX_METADATA_SIZE driver = metadata.get("driver", {}) if driver: - # Truncate driver version. - driver_version = driver.get("version")[:-overflow] - if len(driver_version) >= len(_METADATA["driver"]["version"]): - metadata["driver"]["version"] = driver_version - else: - metadata["driver"]["version"] = _METADATA["driver"]["version"] - encoded_size = len(bson.encode(metadata)) - if encoded_size <= _MAX_METADATA_SIZE: - return - # Truncate driver name. - overflow = encoded_size - _MAX_METADATA_SIZE - driver_name = driver.get("name")[:-overflow] - if len(driver_name) >= len(_METADATA["driver"]["name"]): - metadata["driver"]["name"] = driver_name - else: - metadata["driver"]["name"] = _METADATA["driver"]["name"] + # Truncate the driver name and version in lockstep so that the + # pipe-delimited name and version entries remain index-aligned (1:1). + # Trimming never removes a delimiter from one side alone, so the number + # of "|" in the name always matches the version. Under a large overflow + # the appended |c / |async / wrapped-driver entries are dropped together. + while True: + encoded_size = len(bson.encode(metadata)) + if encoded_size <= _MAX_METADATA_SIZE: + break + overflow = encoded_size - _MAX_METADATA_SIZE + previous = (metadata["driver"].get("name"), metadata["driver"].get("version")) + + name = metadata["driver"].get("name", "") + if len(name) > len(_METADATA["driver"]["name"]): + # Trim the tail of the name, never dropping below the base name. + name = name[:-overflow] + if len(name) < len(_METADATA["driver"]["name"]): + name = _METADATA["driver"]["name"] + metadata["driver"]["name"] = name + else: + # Name is already minimal; trim the version's trailing content. + # Trimming to "" is fine: keeping the delimiter preserves the + # index alignment. + parts = metadata["driver"].get("version", "").split("|") + if len(parts) > 1: + last = parts[-1] + parts[-1] = last[:-overflow] + metadata["driver"]["version"] = "|".join(parts) + else: + break + + # Rebuild the version to match the name's delimiter count so the + # entries stay 1:1 aligned. + parts = metadata["driver"].get("version", "").split("|") + target = metadata["driver"]["name"].count("|") + 1 + if len(parts) > target: + parts = parts[:target] + elif len(parts) < target: + parts += [""] * (target - len(parts)) + metadata["driver"]["version"] = "|".join(parts) + + if previous == (metadata["driver"].get("name"), metadata["driver"].get("version")): + break # If the first getaddrinfo call of this interpreter's life is on a thread, @@ -277,6 +303,7 @@ class PoolOptions: """ __slots__ = ( + "__appended_drivers", "__appname", "__compression_settings", "__connect_timeout", @@ -336,6 +363,7 @@ def __init__( self.__load_balanced = load_balanced self.__credentials = credentials self.__metadata = copy.deepcopy(_METADATA) + self.__appended_drivers: list[DriverInfo] = [] if appname: self.__metadata["application"] = {"name": appname} @@ -353,11 +381,19 @@ def __init__( self.__metadata["driver"]["name"], "c", ) + self.__metadata["driver"]["version"] = "{}|{}".format( + self.__metadata["driver"]["version"], + "", + ) if not is_sync: self.__metadata["driver"]["name"] = "{}|{}".format( self.__metadata["driver"]["name"], "async", ) + self.__metadata["driver"]["version"] = "{}|{}".format( + self.__metadata["driver"]["version"], + "", + ) if driver: self._update_metadata(driver) @@ -368,28 +404,21 @@ def __init__( _truncate_metadata(self.__metadata) def _update_metadata(self, driver: DriverInfo) -> None: - """Updates the client's metadata""" - if driver.name and driver.name.lower() in self.__metadata["driver"]["name"].lower().split( - "|" - ): + """Updates the client's metadata.""" + if driver in self.__appended_drivers: return metadata = copy.deepcopy(self.__metadata) - if driver.name: - metadata["driver"]["name"] = "{}|{}".format( - metadata["driver"]["name"], - driver.name, - ) - if driver.version: - metadata["driver"]["version"] = "{}|{}".format( - metadata["driver"]["version"], - driver.version, - ) + metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name or "") + metadata["driver"]["version"] = "{}|{}".format( + metadata["driver"]["version"], driver.version or "" + ) if driver.platform: metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) self.__metadata = metadata + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: From 5f2e74cc269878f43e8cbf13d3d9db055cd1728f Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 19:37:24 -0500 Subject: [PATCH 2/9] PYTHON-6040 Update tests for index-aligned client metadata --- test/asynchronous/test_client.py | 40 +++++++- test/asynchronous/test_client_metadata.py | 114 +++++++++++++++++++--- test/mockupdb/test_handshake.py | 4 +- test/test_client.py | 40 +++++++- test/test_client_metadata.py | 114 +++++++++++++++++++--- 5 files changed, 277 insertions(+), 35 deletions(-) diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 90a2d33a45..e9d426c045 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -134,6 +134,19 @@ _IS_SYNC = False +def _driver_version(base_version: str, name: str, last_version: str | None = None) -> str: + """Build a metadata driver version aligned 1:1 with ``name`` segments. + + The ``|c`` and ``|async`` name segments always have an empty version entry, + so the version string has one delimiter per name delimiter. ``last_version`` + is used when the final segment carries a wrapped driver's version. + """ + segments = [""] * name.count("|") + if last_version is not None: + segments[-1] = last_version + return "|".join([base_version, *segments]) + + class AsyncClientUnitTest(AsyncUnitTest): """AsyncMongoClient tests that don't require a server.""" @@ -386,6 +399,9 @@ async def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c|async" else: metadata["driver"]["name"] = "PyMongo|async" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) metadata["application"] = {"name": "foobar"} client = self.simple_client("mongodb://foo:27017/?appname=foobar&connect=false") options = client.options @@ -412,7 +428,9 @@ async def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c|async|FooDriver" else: metadata["driver"]["name"] = "PyMongo|async|FooDriver" - metadata["driver"]["version"] = "{}|1.2.3".format(_METADATA["driver"]["version"]) + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) client = self.simple_client( "foo", 27017, @@ -422,6 +440,13 @@ async def test_metadata(self): ) options = client.options self.assertEqual(options.pool_options.metadata, metadata) + if has_c(): + metadata["driver"]["name"] = "PyMongo|c|async|FooDriver" + else: + metadata["driver"]["name"] = "PyMongo|async|FooDriver" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) metadata["platform"] = "{}|FooPlatform".format(_METADATA["platform"]) client = self.simple_client( "foo", @@ -438,19 +463,29 @@ async def test_metadata(self): connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) client = self.simple_client( driver=DriverInfo(name="s" * _MAX_METADATA_SIZE, version="s" * _MAX_METADATA_SIZE), connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): @@ -2224,6 +2259,9 @@ async def _test_handshake(self, env_vars, expected_env): metadata["driver"]["name"] = "PyMongo|c|async" else: metadata["driver"]["name"] = "PyMongo|async" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) if expected_env is not None: metadata["env"] = expected_env diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 1a07e835a8..4615317c5b 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -99,20 +99,14 @@ async def check_metadata_added( new_name, new_version, new_platform, new_metadata = await self.send_ping_and_get_metadata( client, True ) - if add_name is not None and add_name.lower() in name.lower().split("|"): - self.assertEqual(name, new_name) - self.assertEqual(version, new_version) - self.assertEqual(platform, new_platform) - else: - self.assertEqual(new_name, f"{name}|{add_name}" if add_name is not None else name) - self.assertEqual( - new_version, - f"{version}|{add_version}" if add_version is not None else version, - ) - self.assertEqual( - new_platform, - f"{platform}|{add_platform}" if add_platform is not None else platform, - ) + # Name and version always get a delimiter (empty string if None) to + # preserve 1:1 index correspondence. + self.assertEqual(new_name, f"{name}|{add_name or ''}") + self.assertEqual(new_version, f"{version}|{add_version or ''}") + self.assertEqual( + new_platform, + f"{platform}|{add_platform}" if add_platform is not None else platform, + ) metadata.pop("driver") metadata.pop("platform") @@ -216,8 +210,42 @@ async def test_duplicate_driver_name_no_op(self): await self.check_metadata_added(client, "framework", None, None) # wait for connection to become idle await asyncio.sleep(0.005) - # add same metadata again - await self.check_metadata_added(client, "Framework", None, None) + # Append the exact same DriverInfo again: no-op. + name, version, platform, _ = await self.send_ping_and_get_metadata(client, True) + await asyncio.sleep(0.005) + client.append_metadata(DriverInfo("framework", None, None)) + new_name, new_version, new_platform, _ = await self.send_ping_and_get_metadata(client, True) + self.assertEqual(new_name, name) + self.assertEqual(new_version, version) + self.assertEqual(new_platform, platform) + + async def test_append_metadata_rejects_delimiter(self): + cases = [ + ("frame|work", "2.0", "Framework Platform"), + ("framework", "2|0", "Framework Platform"), + ("framework", "2.0", "Framework|Platform"), + ] + for name, version, platform in cases: + with self.subTest(name=name, version=version, platform=platform): + client = await self.async_rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + # Send initial handshake. + name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( + client, True + ) + await asyncio.sleep(0.005) + # Appending metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = await self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + await client.close() async def test_handshake_documents_include_backpressure(self): # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of @@ -232,6 +260,60 @@ async def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") + async def test_index_correspondence(self): + cases = [ + ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), + ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), + ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), + ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), + ( + "Non-adjacent duplicate", + [("F1", "1.0"), ("F2", "2.0"), ("F1", "1.0")], + "|F1|F2", + "|1.0|2.0", + ), + ( + "Platform-only difference is not a duplicate", + [("F1", "1.0", "P1"), ("F1", "1.0", "P2")], + "|F1|F1", + "|1.0|1.0", + ), + ("Wrapper matching the driver's own identity", [("PyMongo", None)], "|PyMongo", "|"), + ] + for ( + description, + appended, + expected_name_suffix, + expected_version_suffix, + ) in cases: + with self.subTest(description=description): + client = await self.async_rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True) + await asyncio.sleep(0.005) + + # Append each DriverInfoOptions in order. + for opts in appended: + d_name = opts[0] if len(opts) > 0 else None + assert d_name is not None + d_version = opts[1] if len(opts) > 1 else None + d_platform = opts[2] if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name, d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) + + assert name0 is not None + assert version0 is not None + self.assertEqual(name1, name0 + expected_name_suffix) + self.assertEqual(version1, version0 + expected_version_suffix) + await client.close() + if __name__ == "__main__": unittest.main() diff --git a/test/mockupdb/test_handshake.py b/test/mockupdb/test_handshake.py index 2772e6f77a..e3a3fc0562 100644 --- a/test/mockupdb/test_handshake.py +++ b/test/mockupdb/test_handshake.py @@ -49,9 +49,11 @@ def _check_handshake_data(request): assert data["application"] == {"name": "my app"} if has_c(): name = "PyMongo|c" + version = pymongo_version + "|" else: name = "PyMongo" - assert data["driver"] == {"name": name, "version": pymongo_version} + version = pymongo_version + assert data["driver"] == {"name": name, "version": version} # Keep it simple, just check these fields exist. assert "os" in data diff --git a/test/test_client.py b/test/test_client.py index 249f95d8fc..eb0022c9e8 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -133,6 +133,19 @@ _IS_SYNC = True +def _driver_version(base_version: str, name: str, last_version: str | None = None) -> str: + """Build a metadata driver version aligned 1:1 with ``name`` segments. + + The ``|c`` and ``|async`` name segments always have an empty version entry, + so the version string has one delimiter per name delimiter. ``last_version`` + is used when the final segment carries a wrapped driver's version. + """ + segments = [""] * name.count("|") + if last_version is not None: + segments[-1] = last_version + return "|".join([base_version, *segments]) + + class ClientUnitTest(UnitTest): """MongoClient tests that don't require a server.""" @@ -379,6 +392,9 @@ def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c" else: metadata["driver"]["name"] = "PyMongo" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) metadata["application"] = {"name": "foobar"} client = self.simple_client("mongodb://foo:27017/?appname=foobar&connect=false") options = client.options @@ -405,7 +421,9 @@ def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c|FooDriver" else: metadata["driver"]["name"] = "PyMongo|FooDriver" - metadata["driver"]["version"] = "{}|1.2.3".format(_METADATA["driver"]["version"]) + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) client = self.simple_client( "foo", 27017, @@ -415,6 +433,13 @@ def test_metadata(self): ) options = client.options self.assertEqual(options.pool_options.metadata, metadata) + if has_c(): + metadata["driver"]["name"] = "PyMongo|c|FooDriver" + else: + metadata["driver"]["name"] = "PyMongo|FooDriver" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) metadata["platform"] = "{}|FooPlatform".format(_METADATA["platform"]) client = self.simple_client( "foo", @@ -431,19 +456,29 @@ def test_metadata(self): connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) client = self.simple_client( driver=DriverInfo(name="s" * _MAX_METADATA_SIZE, version="s" * _MAX_METADATA_SIZE), connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): @@ -2177,6 +2212,9 @@ def _test_handshake(self, env_vars, expected_env): metadata["driver"]["name"] = "PyMongo|c" else: metadata["driver"]["name"] = "PyMongo" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) if expected_env is not None: metadata["env"] = expected_env diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index f5ec92f2f3..68b132646c 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -99,20 +99,14 @@ def check_metadata_added( new_name, new_version, new_platform, new_metadata = self.send_ping_and_get_metadata( client, True ) - if add_name is not None and add_name.lower() in name.lower().split("|"): - self.assertEqual(name, new_name) - self.assertEqual(version, new_version) - self.assertEqual(platform, new_platform) - else: - self.assertEqual(new_name, f"{name}|{add_name}" if add_name is not None else name) - self.assertEqual( - new_version, - f"{version}|{add_version}" if add_version is not None else version, - ) - self.assertEqual( - new_platform, - f"{platform}|{add_platform}" if add_platform is not None else platform, - ) + # Name and version always get a delimiter (empty string if None) to + # preserve 1:1 index correspondence. + self.assertEqual(new_name, f"{name}|{add_name or ''}") + self.assertEqual(new_version, f"{version}|{add_version or ''}") + self.assertEqual( + new_platform, + f"{platform}|{add_platform}" if add_platform is not None else platform, + ) metadata.pop("driver") metadata.pop("platform") @@ -216,8 +210,42 @@ def test_duplicate_driver_name_no_op(self): self.check_metadata_added(client, "framework", None, None) # wait for connection to become idle time.sleep(0.005) - # add same metadata again - self.check_metadata_added(client, "Framework", None, None) + # Append the exact same DriverInfo again: no-op. + name, version, platform, _ = self.send_ping_and_get_metadata(client, True) + time.sleep(0.005) + client.append_metadata(DriverInfo("framework", None, None)) + new_name, new_version, new_platform, _ = self.send_ping_and_get_metadata(client, True) + self.assertEqual(new_name, name) + self.assertEqual(new_version, version) + self.assertEqual(new_platform, platform) + + def test_append_metadata_rejects_delimiter(self): + cases = [ + ("frame|work", "2.0", "Framework Platform"), + ("framework", "2|0", "Framework Platform"), + ("framework", "2.0", "Framework|Platform"), + ] + for name, version, platform in cases: + with self.subTest(name=name, version=version, platform=platform): + client = self.rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + # Send initial handshake. + name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( + client, True + ) + time.sleep(0.005) + # Appending metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + client.close() def test_handshake_documents_include_backpressure(self): # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of @@ -232,6 +260,60 @@ def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") + def test_index_correspondence(self): + cases = [ + ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), + ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), + ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), + ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), + ( + "Non-adjacent duplicate", + [("F1", "1.0"), ("F2", "2.0"), ("F1", "1.0")], + "|F1|F2", + "|1.0|2.0", + ), + ( + "Platform-only difference is not a duplicate", + [("F1", "1.0", "P1"), ("F1", "1.0", "P2")], + "|F1|F1", + "|1.0|1.0", + ), + ("Wrapper matching the driver's own identity", [("PyMongo", None)], "|PyMongo", "|"), + ] + for ( + description, + appended, + expected_name_suffix, + expected_version_suffix, + ) in cases: + with self.subTest(description=description): + client = self.rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = self.send_ping_and_get_metadata(client, True) + time.sleep(0.005) + + # Append each DriverInfoOptions in order. + for opts in appended: + d_name = opts[0] if len(opts) > 0 else None + assert d_name is not None + d_version = opts[1] if len(opts) > 1 else None + d_platform = opts[2] if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name, d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) + + assert name0 is not None + assert version0 is not None + self.assertEqual(name1, name0 + expected_name_suffix) + self.assertEqual(version1, version0 + expected_version_suffix) + client.close() + if __name__ == "__main__": unittest.main() From b3d18d12f1c3ba51841abe23a686a98df143db2e Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 20:14:01 -0500 Subject: [PATCH 3/9] PYTHON-6040 Address PR review feedback Trim the _truncate_metadata comments and number/label the handshake prose tests (backpressure no. 9, delimiter no. 10, index no. 11). --- pymongo/pool_options.py | 9 ++----- test/asynchronous/test_client_metadata.py | 29 +++++++++++++---------- test/test_client_metadata.py | 29 +++++++++++++---------- 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 68d24ef61e..5a322d741a 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -237,11 +237,8 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: # 5. Truncate driver info. driver = metadata.get("driver", {}) if driver: - # Truncate the driver name and version in lockstep so that the - # pipe-delimited name and version entries remain index-aligned (1:1). - # Trimming never removes a delimiter from one side alone, so the number - # of "|" in the name always matches the version. Under a large overflow - # the appended |c / |async / wrapped-driver entries are dropped together. + # Truncate the driver name and version in lockstep so the pipe-delimited + # entries stay 1:1 index-aligned. while True: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: @@ -258,8 +255,6 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: metadata["driver"]["name"] = name else: # Name is already minimal; trim the version's trailing content. - # Trimming to "" is fine: keeping the delimiter preserves the - # index alignment. parts = metadata["driver"].get("version", "").split("|") if len(parts) > 1: last = parts[-1] diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 4615317c5b..e740140859 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -219,6 +219,21 @@ async def test_duplicate_driver_name_no_op(self): self.assertEqual(new_version, version) self.assertEqual(new_platform, platform) + # Prose test no. 9 + async def test_handshake_documents_include_backpressure(self): + # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of + # connection establishment. + client = await self.async_rs_or_single_client("mongodb://" + self.server.address_string) + + # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is + # established on all topologies. Note: MockupDB only supports standalone servers. + await client.admin.command("ping") + + # Assert that for every handshake document intercepted: + # the document has a field `backpressure` whose value is `"2"`. + self.assertEqual(self.handshake_req["backpressure"], "2") + + # Prose test no. 10 async def test_append_metadata_rejects_delimiter(self): cases = [ ("frame|work", "2.0", "Framework Platform"), @@ -247,19 +262,7 @@ async def test_append_metadata_rejects_delimiter(self): self.assertEqual(platform1, platform0) await client.close() - async def test_handshake_documents_include_backpressure(self): - # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of - # connection establishment. - client = await self.async_rs_or_single_client("mongodb://" + self.server.address_string) - - # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is - # established on all topologies. Note: MockupDB only supports standalone servers. - await client.admin.command("ping") - - # Assert that for every handshake document intercepted: - # the document has a field `backpressure` whose value is `"2"`. - self.assertEqual(self.handshake_req["backpressure"], "2") - + # Prose test no. 11 async def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 68b132646c..998353c8ba 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -219,6 +219,21 @@ def test_duplicate_driver_name_no_op(self): self.assertEqual(new_version, version) self.assertEqual(new_platform, platform) + # Prose test no. 9 + def test_handshake_documents_include_backpressure(self): + # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of + # connection establishment. + client = self.rs_or_single_client("mongodb://" + self.server.address_string) + + # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is + # established on all topologies. Note: MockupDB only supports standalone servers. + client.admin.command("ping") + + # Assert that for every handshake document intercepted: + # the document has a field `backpressure` whose value is `"2"`. + self.assertEqual(self.handshake_req["backpressure"], "2") + + # Prose test no. 10 def test_append_metadata_rejects_delimiter(self): cases = [ ("frame|work", "2.0", "Framework Platform"), @@ -247,19 +262,7 @@ def test_append_metadata_rejects_delimiter(self): self.assertEqual(platform1, platform0) client.close() - def test_handshake_documents_include_backpressure(self): - # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of - # connection establishment. - client = self.rs_or_single_client("mongodb://" + self.server.address_string) - - # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is - # established on all topologies. Note: MockupDB only supports standalone servers. - client.admin.command("ping") - - # Assert that for every handshake document intercepted: - # the document has a field `backpressure` whose value is `"2"`. - self.assertEqual(self.handshake_req["backpressure"], "2") - + # Prose test no. 11 def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), From b035cfbbed58b2cde8135fc9b493e3528d2ea87a Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 20:29:25 -0500 Subject: [PATCH 4/9] PYTHON-6040 Address Copilot review feedback Reapply the 512-byte handshake limit after append_metadata, guard the check/update/record sequence with a lock for thread-safe clients, and document the reserved '|' delimiter on DriverInfo. --- pymongo/driver_info.py | 4 ++++ pymongo/pool_options.py | 30 +++++++++++++++++++----------- test/asynchronous/test_client.py | 15 +++++++++++++++ test/test_client.py | 15 +++++++++++++++ 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/pymongo/driver_info.py b/pymongo/driver_info.py index d4c444ce44..54905e8872 100644 --- a/pymongo/driver_info.py +++ b/pymongo/driver_info.py @@ -31,6 +31,10 @@ class DriverInfo(namedtuple("DriverInfo", ["name", "version", "platform"])): can add its own info to this log message. Initialize with three strings like 'MyDriver', '1.2.3', 'some platform info'. Any of these strings may be None to accept PyMongo's default. + + The ``|`` character is the reserved delimiter used to join appended + metadata, so it must not appear in any of the fields. A + :class:`ValueError` is raised if it does. """ def __new__( diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 5a322d741a..d8107e8017 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -23,6 +23,7 @@ import os import platform import sys +import threading from collections.abc import MutableMapping from pathlib import Path from typing import TYPE_CHECKING, Any, Optional @@ -310,6 +311,7 @@ class PoolOptions: "__max_idle_time_seconds", "__max_pool_size", "__metadata", + "__metadata_lock", "__min_pool_size", "__pause_enabled", "__server_api", @@ -359,6 +361,7 @@ def __init__( self.__credentials = credentials self.__metadata = copy.deepcopy(_METADATA) self.__appended_drivers: list[DriverInfo] = [] + self.__metadata_lock = threading.Lock() if appname: self.__metadata["application"] = {"name": appname} @@ -400,20 +403,25 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata.""" - if driver in self.__appended_drivers: - return + with self.__metadata_lock: + if driver in self.__appended_drivers: + return - metadata = copy.deepcopy(self.__metadata) + metadata = copy.deepcopy(self.__metadata) - metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name or "") - metadata["driver"]["version"] = "{}|{}".format( - metadata["driver"]["version"], driver.version or "" - ) - if driver.platform: - metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + metadata["driver"]["name"] = "{}|{}".format( + metadata["driver"]["name"], driver.name or "" + ) + metadata["driver"]["version"] = "{}|{}".format( + metadata["driver"]["version"], driver.version or "" + ) + if driver.platform: + metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + + _truncate_metadata(metadata) - self.__metadata = metadata - self.__appended_drivers.append(driver) + self.__metadata = metadata + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index e9d426c045..32ce6dc152 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -486,6 +486,21 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Successive appends must also stay within the limit and keep name and + # version index-aligned after truncation. + client = self.simple_client(connect=False) + for i in range(80): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + options = client.options + truncated = options.pool_options.metadata["driver"] + self.assertLessEqual( + len(bson.encode(options.pool_options.metadata)), + _MAX_METADATA_SIZE, + ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index eb0022c9e8..4f2c9e7a7d 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -479,6 +479,21 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Successive appends must also stay within the limit and keep name and + # version index-aligned after truncation. + client = self.simple_client(connect=False) + for i in range(80): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + options = client.options + truncated = options.pool_options.metadata["driver"] + self.assertLessEqual( + len(bson.encode(options.pool_options.metadata)), + _MAX_METADATA_SIZE, + ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): From 61ba3a20552d4d632267d0abde302feed65d26f3 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 20:36:45 -0500 Subject: [PATCH 5/9] PYTHON-6040 Use fork-aware lock for metadata updates Use _create_lock() so the metadata lock is registered with pymongo.lock and reset after a fork, avoiding a deadlock in the child process. --- pymongo/pool_options.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index d8107e8017..22ed73cc9d 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -23,7 +23,6 @@ import os import platform import sys -import threading from collections.abc import MutableMapping from pathlib import Path from typing import TYPE_CHECKING, Any, Optional @@ -38,6 +37,7 @@ WAIT_QUEUE_TIMEOUT, has_c, ) +from pymongo.lock import _create_lock if TYPE_CHECKING: from pymongo.auth_shared import MongoCredential @@ -361,7 +361,7 @@ def __init__( self.__credentials = credentials self.__metadata = copy.deepcopy(_METADATA) self.__appended_drivers: list[DriverInfo] = [] - self.__metadata_lock = threading.Lock() + self.__metadata_lock = _create_lock() if appname: self.__metadata["application"] = {"name": appname} From 7e863c9011d607359a2c5168cb3cdee1eecea531 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 05:20:32 -0500 Subject: [PATCH 6/9] PYTHON-6040 Fix truncation order and platform recreation Trim wrapper version content before dropping name/version segments so driver identity is preserved, and recreate the platform field when a platform append follows truncation that removed it. --- pymongo/pool_options.py | 57 ++++++++++------------- test/asynchronous/test_client_metadata.py | 7 ++- test/test_client_metadata.py | 7 ++- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 22ed73cc9d..b2a1d24f75 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -235,46 +235,34 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: return - # 5. Truncate driver info. + # 5. Truncate driver info, keeping name and version 1:1 index-aligned. driver = metadata.get("driver", {}) if driver: - # Truncate the driver name and version in lockstep so the pipe-delimited - # entries stay 1:1 index-aligned. + # Trim the trailing wrapper version's content first so the driver's + # name identity is preserved for as long as possible. Only when no + # version content remains to trim do we drop the last name/version + # segment pair; both actions keep the entries 1:1 aligned. while True: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: break overflow = encoded_size - _MAX_METADATA_SIZE - previous = (metadata["driver"].get("name"), metadata["driver"].get("version")) - - name = metadata["driver"].get("name", "") - if len(name) > len(_METADATA["driver"]["name"]): - # Trim the tail of the name, never dropping below the base name. - name = name[:-overflow] - if len(name) < len(_METADATA["driver"]["name"]): - name = _METADATA["driver"]["name"] - metadata["driver"]["name"] = name + previous = (driver.get("name"), driver.get("version")) + n_parts = driver.get("name", "").split("|") + v_parts = driver.get("version", "").split("|") + + if len(v_parts) > 1 and v_parts[-1]: + v_parts[-1] = v_parts[-1][:-overflow] + driver["version"] = "|".join(v_parts) + elif len(n_parts) > 1: + n_parts.pop() + v_parts.pop() + driver["name"] = "|".join(n_parts) + driver["version"] = "|".join(v_parts) else: - # Name is already minimal; trim the version's trailing content. - parts = metadata["driver"].get("version", "").split("|") - if len(parts) > 1: - last = parts[-1] - parts[-1] = last[:-overflow] - metadata["driver"]["version"] = "|".join(parts) - else: - break - - # Rebuild the version to match the name's delimiter count so the - # entries stay 1:1 aligned. - parts = metadata["driver"].get("version", "").split("|") - target = metadata["driver"]["name"].count("|") + 1 - if len(parts) > target: - parts = parts[:target] - elif len(parts) < target: - parts += [""] * (target - len(parts)) - metadata["driver"]["version"] = "|".join(parts) - - if previous == (metadata["driver"].get("name"), metadata["driver"].get("version")): + break + + if previous == (driver.get("name"), driver.get("version")): break @@ -416,7 +404,10 @@ def _update_metadata(self, driver: DriverInfo) -> None: metadata["driver"]["version"], driver.version or "" ) if driver.platform: - metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + if "platform" in metadata: + metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + else: + metadata["platform"] = driver.platform _truncate_metadata(metadata) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index e740140859..a723fca8a4 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -267,7 +267,12 @@ async def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ( + "Equal versions do not collapse", + [("F1", "1.0"), ("F2", "1.0")], + "|F1|F2", + "|1.0|1.0", + ), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 998353c8ba..6dc2c7f4af 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -267,7 +267,12 @@ def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ( + "Equal versions do not collapse", + [("F1", "1.0"), ("F2", "1.0")], + "|F1|F2", + "|1.0|1.0", + ), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), From d45d98752f27a7f5e210fc1b4df52d17915fb6af Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 05:38:41 -0500 Subject: [PATCH 7/9] PYTHON-6040 Reverse prose test case and trim truncation comment Revert the 'Equal versions do not collapse' prose test case to the specification and shorten the truncation comment. --- pymongo/pool_options.py | 6 ++---- test/asynchronous/test_client_metadata.py | 7 +------ test/test_client_metadata.py | 7 +------ 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index b2a1d24f75..46dcb96a90 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -238,10 +238,8 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: # 5. Truncate driver info, keeping name and version 1:1 index-aligned. driver = metadata.get("driver", {}) if driver: - # Trim the trailing wrapper version's content first so the driver's - # name identity is preserved for as long as possible. Only when no - # version content remains to trim do we drop the last name/version - # segment pair; both actions keep the entries 1:1 aligned. + # Trim wrapper version content first, dropping paired segments only as + # a last resort, so name and version stay 1:1 aligned. while True: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index a723fca8a4..e740140859 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -267,12 +267,7 @@ async def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ( - "Equal versions do not collapse", - [("F1", "1.0"), ("F2", "1.0")], - "|F1|F2", - "|1.0|1.0", - ), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 6dc2c7f4af..998353c8ba 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -267,12 +267,7 @@ def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ( - "Equal versions do not collapse", - [("F1", "1.0"), ("F2", "1.0")], - "|F1|F2", - "|1.0|1.0", - ), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), From 0194f4742d6898f323d11bddaa95f3b090d0c023 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 06:08:28 -0500 Subject: [PATCH 8/9] PYTHON-6040 Bound appended-driver tracking Only record drivers that remain representable in the truncated metadata, so __appended_drivers cannot grow without bound and the dedup membership check stays fast. Add a regression test. --- pymongo/pool_options.py | 12 +++++++++++- test/asynchronous/test_client.py | 6 ++++++ test/test_client.py | 6 ++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 46dcb96a90..1187770086 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -410,7 +410,17 @@ def _update_metadata(self, driver: DriverInfo) -> None: _truncate_metadata(metadata) self.__metadata = metadata - self.__appended_drivers.append(driver) + + # Keep the dedup list bounded: a driver that truncation dropped + # from the published metadata can't be re-appended anyway. + if driver.name: + represented = metadata["driver"]["name"].split("|")[-1] == driver.name + elif driver.version: + represented = metadata["driver"]["version"].split("|")[-1] == driver.version + else: + represented = True + if represented: + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 32ce6dc152..9cf5b2a8c4 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -501,6 +501,12 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Truncated-away drivers must not be retained, so the dedup list stays + # bounded instead of growing one entry per append. + self.assertLess( + len(options.pool_options._PoolOptions__appended_drivers), + 80, + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index 4f2c9e7a7d..f9c5a839f2 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -494,6 +494,12 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Truncated-away drivers must not be retained, so the dedup list stays + # bounded instead of growing one entry per append. + self.assertLess( + len(options.pool_options._PoolOptions__appended_drivers), + 80, + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): From b652b78cf32be0461e7f899af2f125a363d844e8 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 06:25:02 -0500 Subject: [PATCH 9/9] PYTHON-6040 Bound dedup tracking for platform-only appends Use the name delimiter count before/after the update to decide whether an appended pair survived truncation, instead of a name/version branch that always recorded platform-only (empty name/version) drivers. --- pymongo/pool_options.py | 14 ++++-------- test/asynchronous/test_client.py | 39 ++++++++++++++++++-------------- test/test_client.py | 39 ++++++++++++++++++-------------- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 1187770086..ce61c8e62b 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -393,6 +393,7 @@ def _update_metadata(self, driver: DriverInfo) -> None: if driver in self.__appended_drivers: return + name_delims = self.__metadata["driver"]["name"].count("|") metadata = copy.deepcopy(self.__metadata) metadata["driver"]["name"] = "{}|{}".format( @@ -411,15 +412,10 @@ def _update_metadata(self, driver: DriverInfo) -> None: self.__metadata = metadata - # Keep the dedup list bounded: a driver that truncation dropped - # from the published metadata can't be re-appended anyway. - if driver.name: - represented = metadata["driver"]["name"].split("|")[-1] == driver.name - elif driver.version: - represented = metadata["driver"]["version"].split("|")[-1] == driver.version - else: - represented = True - if represented: + # Only track drivers whose appended name/version pair survived + # truncation (i.e. the name gained a segment), so __appended_drivers + # stays bounded and the dedup membership check stays fast. + if metadata["driver"]["name"].count("|") > name_delims: self.__appended_drivers.append(driver) @property diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 9cf5b2a8c4..2a81e3188b 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -486,27 +486,32 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) - # Successive appends must also stay within the limit and keep name and - # version index-aligned after truncation. + # Successive appends must stay within the limit and keep name and + # version index-aligned after truncation. Once the metadata saturates, + # further appends must not grow the dedup tracking list. client = self.simple_client(connect=False) - for i in range(80): + for i in range(300): client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) - options = client.options - truncated = options.pool_options.metadata["driver"] - self.assertLessEqual( - len(bson.encode(options.pool_options.metadata)), - _MAX_METADATA_SIZE, - ) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) self.assertEqual( - truncated["name"].count("|"), - truncated["version"].count("|"), - ) - # Truncated-away drivers must not be retained, so the dedup list stays - # bounded instead of growing one entry per append. - self.assertLess( - len(options.pool_options._PoolOptions__appended_drivers), - 80, + pool.metadata["driver"]["name"].count("|"), + pool.metadata["driver"]["version"].count("|"), ) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + # Platform-only appends (empty name/version) stay bounded the same way. + client = self.simple_client(connect=False) + for i in range(300): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index f9c5a839f2..21217c8f34 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -479,27 +479,32 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) - # Successive appends must also stay within the limit and keep name and - # version index-aligned after truncation. + # Successive appends must stay within the limit and keep name and + # version index-aligned after truncation. Once the metadata saturates, + # further appends must not grow the dedup tracking list. client = self.simple_client(connect=False) - for i in range(80): + for i in range(300): client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) - options = client.options - truncated = options.pool_options.metadata["driver"] - self.assertLessEqual( - len(bson.encode(options.pool_options.metadata)), - _MAX_METADATA_SIZE, - ) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) self.assertEqual( - truncated["name"].count("|"), - truncated["version"].count("|"), - ) - # Truncated-away drivers must not be retained, so the dedup list stays - # bounded instead of growing one entry per append. - self.assertLess( - len(options.pool_options._PoolOptions__appended_drivers), - 80, + pool.metadata["driver"]["name"].count("|"), + pool.metadata["driver"]["version"].count("|"), ) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + # Platform-only appends (empty name/version) stay bounded the same way. + client = self.simple_client(connect=False) + for i in range(300): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self):