From bfa5aafb34130677956fd39a8da3b67855b9636d Mon Sep 17 00:00:00 2001 From: Julius Park Date: Tue, 26 Oct 2021 14:47:51 -0700 Subject: [PATCH] PYTHON-2832 Provide options to limit number of mongos servers used in connecting to sharded clusters (#754) --- doc/changelog.rst | 5 + pymongo/common.py | 3 +- pymongo/mongo_client.py | 10 +- pymongo/settings.py | 5 +- pymongo/srv_resolver.py | 9 +- pymongo/topology_description.py | 20 ++- pymongo/uri_parser.py | 18 ++- ...-conflicts_with_loadBalanced-true-txt.json | 7 ++ ...osts-conflicts_with_loadBalanced-true.json | 7 ++ .../load-balanced/srvMaxHosts-zero-txt.json | 14 +++ .../load-balanced/srvMaxHosts-zero.json | 14 +++ ...axHosts-conflicts_with_replicaSet-txt.json | 7 ++ ...srvMaxHosts-conflicts_with_replicaSet.json | 7 ++ .../srvMaxHosts-equal_to_srv_records.json | 17 +++ .../srvMaxHosts-greater_than_srv_records.json | 16 +++ .../srvMaxHosts-invalid_integer.json | 7 ++ .../replica-set/srvMaxHosts-invalid_type.json | 7 ++ .../srvMaxHosts-less_than_srv_records.json | 13 ++ .../replica-set/srvMaxHosts-zero-txt.json | 17 +++ .../replica-set/srvMaxHosts-zero.json | 17 +++ .../srvMaxHosts-equal_to_srv_records.json | 16 +++ .../srvMaxHosts-greater_than_srv_records.json | 15 +++ .../sharded/srvMaxHosts-invalid_integer.json | 7 ++ .../sharded/srvMaxHosts-invalid_type.json | 7 ++ .../srvMaxHosts-less_than_srv_records.json | 9 ++ .../sharded/srvMaxHosts-zero.json | 15 +++ test/test_client.py | 16 ++- test/test_dns.py | 40 ++++-- test/test_srv_polling.py | 75 +++++++++-- test/uri_options/srv-options.json | 116 ++++++++++++++++++ test/uri_options/srv-service-name-option.json | 24 ---- 31 files changed, 501 insertions(+), 59 deletions(-) create mode 100644 test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true-txt.json create mode 100644 test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true.json create mode 100644 test/srv_seedlist/load-balanced/srvMaxHosts-zero-txt.json create mode 100644 test/srv_seedlist/load-balanced/srvMaxHosts-zero.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet-txt.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-equal_to_srv_records.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-greater_than_srv_records.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-invalid_integer.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-invalid_type.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-less_than_srv_records.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-zero-txt.json create mode 100644 test/srv_seedlist/replica-set/srvMaxHosts-zero.json create mode 100644 test/srv_seedlist/sharded/srvMaxHosts-equal_to_srv_records.json create mode 100644 test/srv_seedlist/sharded/srvMaxHosts-greater_than_srv_records.json create mode 100644 test/srv_seedlist/sharded/srvMaxHosts-invalid_integer.json create mode 100644 test/srv_seedlist/sharded/srvMaxHosts-invalid_type.json create mode 100644 test/srv_seedlist/sharded/srvMaxHosts-less_than_srv_records.json create mode 100644 test/srv_seedlist/sharded/srvMaxHosts-zero.json create mode 100644 test/uri_options/srv-options.json delete mode 100644 test/uri_options/srv-service-name-option.json diff --git a/doc/changelog.rst b/doc/changelog.rst index 1213634ce..3b3b664e9 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -179,6 +179,11 @@ Notable improvements - Enhanced connection pooling to create connections more efficiently and avoid connection storms. +- :class:`~pymongo.mongo_client.MongoClient` now accepts a URI and keyword + argument `srvMaxHosts` that limits the number of mongos-like hosts a client + will connect to. More specifically, when a mongodb+srv:// connection string + resolves to more than `srvMaxHosts` number of hosts, the client will randomly + choose a `srvMaxHosts` sized subset of hosts. Issues Resolved ............... diff --git a/pymongo/common.py b/pymongo/common.py index d77dd7edd..6d9ce0cd9 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -629,7 +629,8 @@ URI_OPTIONS_VALIDATOR_MAP = { 'w': validate_non_negative_int_or_basestring, 'wtimeoutms': validate_non_negative_integer, 'zlibcompressionlevel': validate_zlib_compression_level, - 'srvservicename': validate_string + 'srvservicename': validate_string, + 'srvmaxhosts': validate_non_negative_integer } # Dictionary where keys are the names of URI options specific to pymongo, diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index 3cca1ee36..7dca8bb9c 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -652,8 +652,8 @@ class MongoClient(common.BaseObject): dbase = None opts = common._CaseInsensitiveDictionary() fqdn = None - srv_service_name = keyword_opts.get("srvservicename", None) - + srv_service_name = keyword_opts.get("srvservicename") + srv_max_hosts = keyword_opts.get("srvmaxhosts") if len([h for h in host if "/" in h]) > 1: raise ConfigurationError("host must not contain multiple MongoDB " "URIs") @@ -669,7 +669,9 @@ class MongoClient(common.BaseObject): keyword_opts.cased_key("connecttimeoutms"), timeout) res = uri_parser.parse_uri( entity, port, validate=True, warn=True, normalize=False, - connect_timeout=timeout, srv_service_name=srv_service_name) + connect_timeout=timeout, + srv_service_name=srv_service_name, + srv_max_hosts=srv_max_hosts) seeds.update(res["nodelist"]) username = res["username"] or username password = res["password"] or password @@ -703,6 +705,7 @@ class MongoClient(common.BaseObject): if srv_service_name is None: srv_service_name = opts.get("srvServiceName", common.SRV_SERVICE_NAME) + srv_max_hosts = srv_max_hosts or opts.get("srvmaxhosts") # Handle security-option conflicts in combined options. opts = _handle_security_options(opts) # Normalize combined options. @@ -745,6 +748,7 @@ class MongoClient(common.BaseObject): srv_service_name=srv_service_name, direct_connection=options.direct_connection, load_balanced=options.load_balanced, + srv_max_hosts=srv_max_hosts ) self._topology = Topology(self._topology_settings) diff --git a/pymongo/settings.py b/pymongo/settings.py index 7e9d393ac..e9e28c13a 100644 --- a/pymongo/settings.py +++ b/pymongo/settings.py @@ -41,7 +41,8 @@ class TopologySettings(object): fqdn=None, srv_service_name=common.SRV_SERVICE_NAME, direct_connection=False, - load_balanced=None): + load_balanced=None, + srv_max_hosts=0): """Represent MongoClient's configuration. Take a list of (host, port) pairs and optional replica set name. @@ -63,7 +64,7 @@ class TopologySettings(object): self._fqdn = fqdn self._srv_service_name = srv_service_name self._heartbeat_frequency = heartbeat_frequency - + self._srv_max_hosts = srv_max_hosts or 0 self._direct = direct_connection self._load_balanced = load_balanced diff --git a/pymongo/srv_resolver.py b/pymongo/srv_resolver.py index e3f8f330f..69e075aec 100644 --- a/pymongo/srv_resolver.py +++ b/pymongo/srv_resolver.py @@ -15,6 +15,7 @@ """Support for resolving hosts and options from mongodb+srv:// URIs.""" import ipaddress +import random try: from dns import resolver @@ -25,7 +26,6 @@ except ImportError: from pymongo.common import CONNECT_TIMEOUT from pymongo.errors import ConfigurationError - # dnspython can return bytes or str from various parts # of its API depending on version. We always want str. def maybe_decode(text): @@ -48,11 +48,11 @@ _INVALID_HOST_MSG = ( class _SrvResolver(object): def __init__(self, fqdn, - connect_timeout, srv_service_name): + connect_timeout, srv_service_name, srv_max_hosts=0): self.__fqdn = fqdn self.__srv = srv_service_name self.__connect_timeout = connect_timeout or CONNECT_TIMEOUT - + self.__srv_max_hosts = srv_max_hosts or 0 # Validate the fully qualified domain name. try: ipaddress.ip_address(fqdn) @@ -111,7 +111,8 @@ class _SrvResolver(object): raise ConfigurationError("Invalid SRV host: %s" % (node[0],)) if self.__plist != nlist: raise ConfigurationError("Invalid SRV host: %s" % (node[0],)) - + if self.__srv_max_hosts: + nodes = random.sample(nodes, min(self.__srv_max_hosts, len(nodes))) return results, nodes def get_hosts(self): diff --git a/pymongo/topology_description.py b/pymongo/topology_description.py index c6b81b538..19a1681fa 100644 --- a/pymongo/topology_description.py +++ b/pymongo/topology_description.py @@ -15,6 +15,7 @@ """Represent a deployment of MongoDB servers.""" from collections import namedtuple +from random import sample from pymongo import common from pymongo.errors import ConfigurationError @@ -223,6 +224,10 @@ class TopologyDescription(object): def heartbeat_frequency(self): return self._topology_settings.heartbeat_frequency + @property + def srv_max_hosts(self): + return self._topology_settings._srv_max_hosts + def apply_selector(self, selector, address, custom_selector=None): def apply_local_threshold(selection): @@ -446,16 +451,23 @@ def _updated_topology_description_srv_polling(topology_description, seedlist): if set(sds.keys()) == set(seedlist): return topology_description - # Add SDs corresponding to servers recently added to the SRV record. - for address in seedlist: - if address not in sds: - sds[address] = ServerDescription(address) # Remove SDs corresponding to servers no longer part of the SRV record. for address in list(sds.keys()): if address not in seedlist: sds.pop(address) + if topology_description.srv_max_hosts != 0: + new_hosts = set(seedlist) - set(sds.keys()) + n_to_add = topology_description.srv_max_hosts - len(sds) + if n_to_add > 0: + seedlist = sample(new_hosts, min(n_to_add, len(new_hosts))) + else: + seedlist = [] + # Add SDs corresponding to servers recently added to the SRV record. + for address in seedlist: + if address not in sds: + sds[address] = ServerDescription(address) return TopologyDescription( topology_description.topology_type, sds, diff --git a/pymongo/uri_parser.py b/pymongo/uri_parser.py index 46c5eecd9..5d97eb483 100644 --- a/pymongo/uri_parser.py +++ b/pymongo/uri_parser.py @@ -393,7 +393,8 @@ def _check_options(nodes, options): def parse_uri(uri, default_port=DEFAULT_PORT, validate=True, warn=False, - normalize=True, connect_timeout=None, srv_service_name=None): + normalize=True, connect_timeout=None, srv_service_name=None, + srv_max_hosts=None): """Parse and validate a MongoDB URI. Returns a dict of the form:: @@ -494,10 +495,8 @@ def parse_uri(uri, default_port=DEFAULT_PORT, validate=True, warn=False, if opts: options.update(split_options(opts, validate, warn, normalize)) - if srv_service_name is None: srv_service_name = options.get("srvServiceName", SRV_SERVICE_NAME) - if '@' in host_part: userinfo, _, hosts = host_part.rpartition('@') user, passwd = parse_userinfo(userinfo) @@ -510,7 +509,7 @@ def parse_uri(uri, default_port=DEFAULT_PORT, validate=True, warn=False, hosts = unquote_plus(hosts) fqdn = None - + srv_max_hosts = srv_max_hosts or options.get("srvMaxHosts") if is_srv: if options.get('directConnection'): raise ConfigurationError( @@ -529,7 +528,8 @@ def parse_uri(uri, default_port=DEFAULT_PORT, validate=True, warn=False, # Use the connection timeout. connectTimeoutMS passed as a keyword # argument overrides the same option passed in the connection string. connect_timeout = connect_timeout or options.get("connectTimeoutMS") - dns_resolver = _SrvResolver(fqdn, connect_timeout, srv_service_name) + dns_resolver = _SrvResolver(fqdn, connect_timeout, srv_service_name, + srv_max_hosts) nodes = dns_resolver.get_hosts() dns_options = dns_resolver.get_options() if dns_options: @@ -542,11 +542,19 @@ def parse_uri(uri, default_port=DEFAULT_PORT, validate=True, warn=False, for opt, val in parsed_dns_options.items(): if opt not in options: options[opt] = val + if options.get("loadBalanced") and srv_max_hosts: + raise InvalidURI( + "You cannot specify loadBalanced with srvMaxHosts") + if options.get("replicaSet") and srv_max_hosts: + raise InvalidURI("You cannot specify replicaSet with srvMaxHosts") if "tls" not in options and "ssl" not in options: options["tls"] = True if validate else 'true' elif not is_srv and options.get("srvServiceName") is not None: raise ConfigurationError("The srvServiceName option is only allowed " "with 'mongodb+srv://' URIs") + elif not is_srv and srv_max_hosts: + raise ConfigurationError("The srvMaxHosts option is only allowed " + "with 'mongodb+srv://' URIs") else: nodes = split_hosts(hosts, default_port=default_port) diff --git a/test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true-txt.json b/test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true-txt.json new file mode 100644 index 000000000..a7600a8a7 --- /dev/null +++ b/test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true-txt.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test20.test.build.10gen.cc/?srvMaxHosts=1", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because positive integer for srvMaxHosts conflicts with loadBalanced=true (TXT)" +} diff --git a/test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true.json b/test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true.json new file mode 100644 index 000000000..d03a174b1 --- /dev/null +++ b/test/srv_seedlist/load-balanced/srvMaxHosts-conflicts_with_loadBalanced-true.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test3.test.build.10gen.cc/?loadBalanced=true&srvMaxHosts=1", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because positive integer for srvMaxHosts conflicts with loadBalanced=true" +} diff --git a/test/srv_seedlist/load-balanced/srvMaxHosts-zero-txt.json b/test/srv_seedlist/load-balanced/srvMaxHosts-zero-txt.json new file mode 100644 index 000000000..8d48b5bbb --- /dev/null +++ b/test/srv_seedlist/load-balanced/srvMaxHosts-zero-txt.json @@ -0,0 +1,14 @@ +{ + "uri": "mongodb+srv://test20.test.build.10gen.cc/?srvMaxHosts=0", + "seeds": [ + "localhost.test.build.10gen.cc:27017" + ], + "hosts": [ + "localhost.test.build.10gen.cc:27017" + ], + "options": { + "loadBalanced": true, + "srvMaxHosts": 0, + "ssl": true + } +} diff --git a/test/srv_seedlist/load-balanced/srvMaxHosts-zero.json b/test/srv_seedlist/load-balanced/srvMaxHosts-zero.json new file mode 100644 index 000000000..2382fccf8 --- /dev/null +++ b/test/srv_seedlist/load-balanced/srvMaxHosts-zero.json @@ -0,0 +1,14 @@ +{ + "uri": "mongodb+srv://test3.test.build.10gen.cc/?loadBalanced=true&srvMaxHosts=0", + "seeds": [ + "localhost.test.build.10gen.cc:27017" + ], + "hosts": [ + "localhost.test.build.10gen.cc:27017" + ], + "options": { + "loadBalanced": true, + "srvMaxHosts": 0, + "ssl": true + } +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet-txt.json b/test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet-txt.json new file mode 100644 index 000000000..6de1e37fa --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet-txt.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test5.test.build.10gen.cc/?srvMaxHosts=1", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because positive integer for srvMaxHosts conflicts with replicaSet option (TXT)" +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet.json b/test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet.json new file mode 100644 index 000000000..f96875750 --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-conflicts_with_replicaSet.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?replicaSet=repl0&srvMaxHosts=1", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because positive integer for srvMaxHosts conflicts with replicaSet option" +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-equal_to_srv_records.json b/test/srv_seedlist/replica-set/srvMaxHosts-equal_to_srv_records.json new file mode 100644 index 000000000..d9765ac66 --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-equal_to_srv_records.json @@ -0,0 +1,17 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=2", + "numSeeds": 2, + "seeds": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "hosts": [ + "localhost:27017", + "localhost:27018", + "localhost:27019" + ], + "options": { + "srvMaxHosts": 2, + "ssl": true + } +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-greater_than_srv_records.json b/test/srv_seedlist/replica-set/srvMaxHosts-greater_than_srv_records.json new file mode 100644 index 000000000..494bb8768 --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-greater_than_srv_records.json @@ -0,0 +1,16 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=3", + "seeds": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "hosts": [ + "localhost:27017", + "localhost:27018", + "localhost:27019" + ], + "options": { + "srvMaxHosts": 3, + "ssl": true + } +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-invalid_integer.json b/test/srv_seedlist/replica-set/srvMaxHosts-invalid_integer.json new file mode 100644 index 000000000..5ba1a3b54 --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-invalid_integer.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?replicaSet=repl0&srvMaxHosts=-1", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because srvMaxHosts is not greater than or equal to zero" +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-invalid_type.json b/test/srv_seedlist/replica-set/srvMaxHosts-invalid_type.json new file mode 100644 index 000000000..79e75b9b1 --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-invalid_type.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?replicaSet=repl0&srvMaxHosts=foo", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because srvMaxHosts is not an integer" +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-less_than_srv_records.json b/test/srv_seedlist/replica-set/srvMaxHosts-less_than_srv_records.json new file mode 100644 index 000000000..66a5e90da --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-less_than_srv_records.json @@ -0,0 +1,13 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=1", + "numSeeds": 1, + "hosts": [ + "localhost:27017", + "localhost:27018", + "localhost:27019" + ], + "options": { + "srvMaxHosts": 1, + "ssl": true + } +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-zero-txt.json b/test/srv_seedlist/replica-set/srvMaxHosts-zero-txt.json new file mode 100644 index 000000000..241a901c6 --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-zero-txt.json @@ -0,0 +1,17 @@ +{ + "uri": "mongodb+srv://test5.test.build.10gen.cc/?srvMaxHosts=0", + "seeds": [ + "localhost.test.build.10gen.cc:27017" + ], + "hosts": [ + "localhost:27017", + "localhost:27018", + "localhost:27019" + ], + "options": { + "authSource": "thisDB", + "replicaSet": "repl0", + "srvMaxHosts": 0, + "ssl": true + } +} diff --git a/test/srv_seedlist/replica-set/srvMaxHosts-zero.json b/test/srv_seedlist/replica-set/srvMaxHosts-zero.json new file mode 100644 index 000000000..c68610a20 --- /dev/null +++ b/test/srv_seedlist/replica-set/srvMaxHosts-zero.json @@ -0,0 +1,17 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?replicaSet=repl0&srvMaxHosts=0", + "seeds": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "hosts": [ + "localhost:27017", + "localhost:27018", + "localhost:27019" + ], + "options": { + "replicaSet": "repl0", + "srvMaxHosts": 0, + "ssl": true + } +} diff --git a/test/srv_seedlist/sharded/srvMaxHosts-equal_to_srv_records.json b/test/srv_seedlist/sharded/srvMaxHosts-equal_to_srv_records.json new file mode 100644 index 000000000..46390726f --- /dev/null +++ b/test/srv_seedlist/sharded/srvMaxHosts-equal_to_srv_records.json @@ -0,0 +1,16 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=2", + "numSeeds": 2, + "seeds": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "hosts": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "options": { + "srvMaxHosts": 2, + "ssl": true + } +} diff --git a/test/srv_seedlist/sharded/srvMaxHosts-greater_than_srv_records.json b/test/srv_seedlist/sharded/srvMaxHosts-greater_than_srv_records.json new file mode 100644 index 000000000..e02d72bf2 --- /dev/null +++ b/test/srv_seedlist/sharded/srvMaxHosts-greater_than_srv_records.json @@ -0,0 +1,15 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=3", + "seeds": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "hosts": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "options": { + "srvMaxHosts": 3, + "ssl": true + } +} diff --git a/test/srv_seedlist/sharded/srvMaxHosts-invalid_integer.json b/test/srv_seedlist/sharded/srvMaxHosts-invalid_integer.json new file mode 100644 index 000000000..0939624fc --- /dev/null +++ b/test/srv_seedlist/sharded/srvMaxHosts-invalid_integer.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=-1", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because srvMaxHosts is not greater than or equal to zero" +} diff --git a/test/srv_seedlist/sharded/srvMaxHosts-invalid_type.json b/test/srv_seedlist/sharded/srvMaxHosts-invalid_type.json new file mode 100644 index 000000000..c228d2661 --- /dev/null +++ b/test/srv_seedlist/sharded/srvMaxHosts-invalid_type.json @@ -0,0 +1,7 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=foo", + "seeds": [], + "hosts": [], + "error": true, + "comment": "Should fail because srvMaxHosts is not an integer" +} diff --git a/test/srv_seedlist/sharded/srvMaxHosts-less_than_srv_records.json b/test/srv_seedlist/sharded/srvMaxHosts-less_than_srv_records.json new file mode 100644 index 000000000..fdcc1692c --- /dev/null +++ b/test/srv_seedlist/sharded/srvMaxHosts-less_than_srv_records.json @@ -0,0 +1,9 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=1", + "numSeeds": 1, + "numHosts": 1, + "options": { + "srvMaxHosts": 1, + "ssl": true + } +} diff --git a/test/srv_seedlist/sharded/srvMaxHosts-zero.json b/test/srv_seedlist/sharded/srvMaxHosts-zero.json new file mode 100644 index 000000000..10ab9e656 --- /dev/null +++ b/test/srv_seedlist/sharded/srvMaxHosts-zero.json @@ -0,0 +1,15 @@ +{ + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=0", + "seeds": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "hosts": [ + "localhost.test.build.10gen.cc:27017", + "localhost.test.build.10gen.cc:27018" + ], + "options": { + "srvMaxHosts": 0, + "ssl": true + } +} diff --git a/test/test_client.py b/test/test_client.py index edd459eb3..745e8d6f2 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -65,7 +65,7 @@ from pymongo.server_type import SERVER_TYPE from pymongo.settings import TOPOLOGY_TYPE from pymongo.srv_resolver import _HAVE_DNSPYTHON from pymongo.topology import _ErrorContext -from pymongo.topology_description import TopologyDescription +from pymongo.topology_description import TopologyDescription, _updated_topology_description_srv_polling from pymongo.write_concern import WriteConcern from test import (client_context, client_knobs, @@ -1641,6 +1641,20 @@ class TestClient(IntegrationTest): self.assertEqual(client._topology_settings._srv_service_name, 'customname') + def test_srv_max_hosts_kwarg(self): + client = MongoClient( + 'mongodb+srv://test1.test.build.10gen.cc/') + self.assertGreater( + len(client.topology_description.server_descriptions()), 1) + client = MongoClient( + 'mongodb+srv://test1.test.build.10gen.cc/', srvmaxhosts=1) + self.assertEqual( + len(client.topology_description.server_descriptions()), 1) + client = MongoClient( + 'mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=1', + srvmaxhosts=2) + self.assertEqual( + len(client.topology_description.server_descriptions()), 2) class TestExhaustCursor(IntegrationTest): diff --git a/test/test_dns.py b/test/test_dns.py index 91f8750d8..8404c2aa6 100644 --- a/test/test_dns.py +++ b/test/test_dns.py @@ -50,14 +50,27 @@ class TestDNSLoadBalanced(unittest.TestCase): pass +class TestDNSSharded(unittest.TestCase): + TEST_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), + 'srv_seedlist', 'sharded') + load_balanced = False + + @client_context.require_mongos + def setUp(self): + pass + + def create_test(test_case): def run_test(self): if not _HAVE_DNSPYTHON: raise unittest.SkipTest("DNS tests require the dnspython module") uri = test_case['uri'] - seeds = test_case['seeds'] - hosts = test_case['hosts'] + seeds = test_case.get('seeds') + num_seeds = test_case.get('numSeeds', len(seeds or [])) + hosts = test_case.get('hosts') + num_hosts = test_case.get("numHosts", len(hosts or [])) + options = test_case.get('options', {}) if 'ssl' in options: options['tls'] = options.pop('ssl') @@ -75,9 +88,12 @@ def create_test(test_case): if hosts: hosts = frozenset(split_hosts(','.join(hosts))) - if seeds: + if seeds or num_seeds: result = parse_uri(uri, validate=True) - self.assertEqual(sorted(result['nodelist']), sorted(seeds)) + if seeds is not None: + self.assertEqual(sorted(result['nodelist']), sorted(seeds)) + if num_seeds is not None: + self.assertEqual(len(result['nodelist']), num_seeds) if options: opts = result['options'] if 'readpreferencetags' in opts: @@ -106,9 +122,18 @@ def create_test(test_case): copts['tlsAllowInvalidHostnames'] = True client = MongoClient(uri, **copts) - wait_until( - lambda: hosts == client.nodes, - 'match test hosts to client nodes') + if num_seeds is not None: + self.assertEqual(len(client._topology_settings.seeds), + num_seeds) + if hosts is not None: + wait_until( + lambda: hosts == client.nodes, + 'match test hosts to client nodes') + if num_hosts is not None: + wait_until(lambda: num_hosts == len(client.nodes), + "wait to connect to num_hosts") + # XXX: we should block until SRV poller runs at least once + # and re-run these assertions. else: try: parse_uri(uri) @@ -130,6 +155,7 @@ def create_tests(cls): create_tests(TestDNSRepl) create_tests(TestDNSLoadBalanced) +create_tests(TestDNSSharded) class TestParsingErrors(unittest.TestCase): diff --git a/test/test_srv_polling.py b/test/test_srv_polling.py index 190810347..030dcd27f 100644 --- a/test/test_srv_polling.py +++ b/test/test_srv_polling.py @@ -35,11 +35,11 @@ WAIT_TIME = 0.1 class SrvPollingKnobs(object): def __init__(self, ttl_time=None, min_srv_rescan_interval=None, - dns_resolver_nodelist_response=None, + nodelist_callback=None, count_resolver_calls=False): self.ttl_time = ttl_time self.min_srv_rescan_interval = min_srv_rescan_interval - self.dns_resolver_nodelist_response = dns_resolver_nodelist_response + self.nodelist_callback = nodelist_callback self.count_resolver_calls = count_resolver_calls self.old_min_srv_rescan_interval = None @@ -55,8 +55,8 @@ class SrvPollingKnobs(object): def mock_get_hosts_and_min_ttl(resolver, *args): nodes, ttl = self.old_dns_resolver_response(resolver) - if self.dns_resolver_nodelist_response is not None: - nodes = self.dns_resolver_nodelist_response() + if self.nodelist_callback is not None: + nodes = self.nodelist_callback() if self.ttl_time is not None: ttl = self.ttl_time return nodes, ttl @@ -113,7 +113,8 @@ class TestSrvPolling(unittest.TestCase): if set(expected_nodelist) == set(nodelist): return True return False - wait_until(predicate, "see expected nodelist", timeout=100*WAIT_TIME) + wait_until(predicate, "see expected nodelist", + timeout=100*WAIT_TIME) def assert_nodelist_nochange(self, expected_nodelist, client): """Check if the client._topology ever deviates from seeing all nodes @@ -154,7 +155,7 @@ class TestSrvPolling(unittest.TestCase): self.assert_nodelist_change(self.BASE_SRV_RESPONSE, client) # Patch list of hosts returned by DNS query. with SrvPollingKnobs( - dns_resolver_nodelist_response=dns_resolver_response, + nodelist_callback=dns_resolver_response, count_resolver_calls=count_resolver_calls): assertion_method(expected_response, client) @@ -207,7 +208,7 @@ class TestSrvPolling(unittest.TestCase): with SrvPollingKnobs( ttl_time=WAIT_TIME, min_srv_rescan_interval=WAIT_TIME, - dns_resolver_nodelist_response=initial_callback, + nodelist_callback=initial_callback, count_resolver_calls=True): # Client uses unpatched method to get initial nodelist client = MongoClient(self.CONNECTION_STRING) @@ -216,7 +217,7 @@ class TestSrvPolling(unittest.TestCase): with SrvPollingKnobs( ttl_time=WAIT_TIME, min_srv_rescan_interval=WAIT_TIME, - dns_resolver_nodelist_response=final_callback): + nodelist_callback=final_callback): # Nodelist should reflect new valid DNS resolver response. self.assert_nodelist_change(response_final, client) @@ -230,6 +231,64 @@ class TestSrvPolling(unittest.TestCase): raise ConfigurationError self._test_recover_from_initial(erroring_seedlist) + def test_10_all_dns_selected(self): + response = [("localhost.test.build.10gen.cc", 27017), + ("localhost.test.build.10gen.cc", 27019), + ("localhost.test.build.10gen.cc", 27020)] + + def nodelist_callback(): + return response + with SrvPollingKnobs(ttl_time=WAIT_TIME, + min_srv_rescan_interval=WAIT_TIME): + client = MongoClient(self.CONNECTION_STRING, srvMaxHosts=0) + self.addCleanup(client.close) + with SrvPollingKnobs(nodelist_callback=nodelist_callback): + self.assert_nodelist_change(response, client) + + def test_11_all_dns_selected(self): + response = [("localhost.test.build.10gen.cc", 27019), + ("localhost.test.build.10gen.cc", 27020)] + + def nodelist_callback(): + return response + + with SrvPollingKnobs( + ttl_time=WAIT_TIME, min_srv_rescan_interval=WAIT_TIME): + client = MongoClient(self.CONNECTION_STRING, srvMaxHosts=2) + self.addCleanup(client.close) + with SrvPollingKnobs(nodelist_callback=nodelist_callback): + self.assert_nodelist_change(response, client) + + def test_12_new_dns_randomly_selected(self): + response = [("localhost.test.build.10gen.cc", 27020), + ("localhost.test.build.10gen.cc", 27019), + ("localhost.test.build.10gen.cc", 27017)] + + def nodelist_callback(): + return response + + with SrvPollingKnobs( + ttl_time=WAIT_TIME, min_srv_rescan_interval=WAIT_TIME): + client = MongoClient(self.CONNECTION_STRING, srvMaxHosts=2) + self.addCleanup(client.close) + with SrvPollingKnobs(nodelist_callback=nodelist_callback): + sleep(2*common.MIN_SRV_RESCAN_INTERVAL) + final_topology = set( + client.topology_description.server_descriptions()) + self.assertIn(("localhost.test.build.10gen.cc", 27017), + final_topology) + self.assertEqual(len(final_topology), 2) + + def test_does_not_flipflop(self): + with SrvPollingKnobs( + ttl_time=WAIT_TIME, min_srv_rescan_interval=WAIT_TIME): + client = MongoClient(self.CONNECTION_STRING, srvMaxHosts=1) + self.addCleanup(client.close) + old = set(client.topology_description.server_descriptions()) + sleep(4*WAIT_TIME) + new = set(client.topology_description.server_descriptions()) + self.assertSetEqual(old, new) + if __name__ == '__main__': unittest.main() diff --git a/test/uri_options/srv-options.json b/test/uri_options/srv-options.json new file mode 100644 index 000000000..ffc356f12 --- /dev/null +++ b/test/uri_options/srv-options.json @@ -0,0 +1,116 @@ +{ + "tests": [ + { + "description": "SRV URI with custom srvServiceName", + "uri": "mongodb+srv://test22.test.build.10gen.cc/?srvServiceName=customname", + "valid": true, + "warning": false, + "hosts": null, + "auth": null, + "options": { + "srvServiceName": "customname" + } + }, + { + "description": "Non-SRV URI with custom srvServiceName", + "uri": "mongodb://example.com/?srvServiceName=customname", + "valid": false, + "warning": false, + "hosts": null, + "auth": null, + "options": {} + }, + { + "description": "SRV URI with srvMaxHosts", + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=2", + "valid": true, + "warning": false, + "hosts": null, + "auth": null, + "options": { + "srvMaxHosts": 2 + } + }, + { + "description": "SRV URI with negative integer for srvMaxHosts", + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=-1", + "valid": true, + "warning": true, + "hosts": null, + "auth": null, + "options": {} + }, + { + "description": "SRV URI with invalid type for srvMaxHosts", + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=foo", + "valid": true, + "warning": true, + "hosts": null, + "auth": null, + "options": {} + }, + { + "description": "Non-SRV URI with srvMaxHosts", + "uri": "mongodb://example.com/?srvMaxHosts=2", + "valid": false, + "warning": false, + "hosts": null, + "auth": null, + "options": {} + }, + { + "description": "SRV URI with positive srvMaxHosts and replicaSet", + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=2&replicaSet=foo", + "valid": false, + "warning": false, + "hosts": null, + "auth": null, + "options": {} + }, + { + "description": "SRV URI with positive srvMaxHosts and loadBalanced=true", + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=2&loadBalanced=true", + "valid": false, + "warning": false, + "hosts": null, + "auth": null, + "options": {} + }, + { + "description": "SRV URI with positive srvMaxHosts and loadBalanced=false", + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=2&loadBalanced=false", + "valid": true, + "warning": false, + "hosts": null, + "auth": null, + "options": { + "loadBalanced": false, + "srvMaxHosts": 2 + } + }, + { + "description": "SRV URI with srvMaxHosts=0 and replicaSet", + "uri": "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=0&replicaSet=foo", + "valid": true, + "warning": false, + "hosts": null, + "auth": null, + "options": { + "replicaSet": "foo", + "srvMaxHosts": 0 + } + }, + { + "description": "SRV URI with srvMaxHosts=0 and loadBalanced=true", + "uri": "mongodb+srv://test3.test.build.10gen.cc/?srvMaxHosts=0&loadBalanced=true", + "valid": true, + "warning": false, + "hosts": null, + "auth": null, + "options": { + "loadBalanced": true, + "srvMaxHosts": 0 + } + } + ] +} diff --git a/test/uri_options/srv-service-name-option.json b/test/uri_options/srv-service-name-option.json deleted file mode 100644 index 049a35bc7..000000000 --- a/test/uri_options/srv-service-name-option.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "tests": [ - { - "description": "SRV URI with custom srvServiceName", - "uri": "mongodb+srv://test22.test.build.10gen.cc/?srvServiceName=customname", - "valid": true, - "warning": false, - "hosts": null, - "auth": null, - "options": { - "srvServiceName": "customname" - } - }, - { - "description": "Non-SRV URI with custom srvServiceName", - "uri": "mongodb://example.com/?srvServiceName=customname", - "valid": false, - "warning": true, - "hosts": null, - "auth": null, - "options": {} - } - ] -}