From 87708a2f96a8bcb524c06617f51145eced6e5dc7 Mon Sep 17 00:00:00 2001 From: "A. Jesse Jiryu Davis" Date: Mon, 7 Nov 2016 16:49:33 -0500 Subject: [PATCH] PYTHON-1169 - Default maxStalenessSeconds is -1. --- pymongo/client_options.py | 2 +- pymongo/common.py | 9 +++-- pymongo/max_staleness_selectors.py | 5 ++- pymongo/message.py | 2 +- pymongo/mongo_client.py | 2 +- pymongo/read_preferences.py | 56 ++++++++++++++++-------------- test/test_max_staleness.py | 37 +++++++++++++++++++- test/test_topology.py | 4 +-- 8 files changed, 76 insertions(+), 41 deletions(-) diff --git a/pymongo/client_options.py b/pymongo/client_options.py index 39dc74a08..e5baf3e10 100644 --- a/pymongo/client_options.py +++ b/pymongo/client_options.py @@ -44,7 +44,7 @@ def _parse_read_preference(options): mode = options.get('readpreference', 0) tags = options.get('readpreferencetags') - max_staleness = options.get('maxstalenessseconds') + max_staleness = options.get('maxstalenessseconds', -1) return make_read_preference(mode, tags, max_staleness) diff --git a/pymongo/common.py b/pymongo/common.py index 562c3abe0..3189754a0 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -292,11 +292,10 @@ def validate_timeout_or_zero(option, value): def validate_max_staleness(option, value): - """Validates a timeout specified in seconds returning - a value in floating point seconds. - """ - if value is None: - return value + """Validates maxStalenessSeconds according to the Max Staleness Spec.""" + if value == -1 or value == "-1": + # Default: No maximum staleness. + return -1 return validate_positive_float(option, value) diff --git a/pymongo/max_staleness_selectors.py b/pymongo/max_staleness_selectors.py index 0ba5037f8..99b637f3d 100644 --- a/pymongo/max_staleness_selectors.py +++ b/pymongo/max_staleness_selectors.py @@ -34,6 +34,7 @@ from pymongo.server_type import SERVER_TYPE def _validate_max_staleness(max_staleness, heartbeat_frequency, idle_write_period): + # We checked for max staleness -1 before this, it must be positive here. if max_staleness < heartbeat_frequency + idle_write_period: raise ConfigurationError( "maxStalenessSeconds must be at least heartbeatFrequencyMS +" @@ -46,7 +47,6 @@ def _validate_max_staleness(max_staleness, def _with_primary(max_staleness, selection): """Apply max_staleness, in seconds, to a Selection with a known primary.""" primary = selection.primary - assert primary # Server Selection Spec: If the TopologyType is ReplicaSetWithPrimary, a # client MUST raise an error if maxStaleness < heartbeatFrequency + @@ -83,7 +83,6 @@ def _no_primary(max_staleness, selection): # Secondary we've most recently checked. srecent = selection.secondary_with_max_last_update_time() - assert srecent sds = [] @@ -111,7 +110,7 @@ def _no_primary(max_staleness, selection): def select(max_staleness, selection): """Apply max_staleness, in seconds, to a Selection.""" - if not max_staleness: + if max_staleness == -1: return selection if selection.primary: diff --git a/pymongo/message.py b/pymongo/message.py index 76adcf88a..03041adb8 100644 --- a/pymongo/message.py +++ b/pymongo/message.py @@ -82,7 +82,7 @@ def _maybe_add_read_preference(spec, read_preference): if mode and ( mode != ReadPreference.SECONDARY_PREFERRED.mode or tag_sets != [{}] - or max_staleness): + or max_staleness != -1): if "$query" not in spec: spec = SON([("$query", spec)]) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index 9f5a9a24a..a9b2b73d4 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -255,7 +255,7 @@ class MongoClient(common.BaseObject): - `maxStalenessSeconds`: (integer or float) The maximum estimated length of time a replica set secondary can fall behind the primary in replication before it will no longer be selected for operations. - Defaults to ``None`` (no limit). + Defaults to ``-1``, meaning no maximum. | **SSL configuration:** diff --git a/pymongo/read_preferences.py b/pymongo/read_preferences.py index e70675419..ca843ac49 100644 --- a/pymongo/read_preferences.py +++ b/pymongo/read_preferences.py @@ -62,22 +62,22 @@ def _validate_tag_sets(tag_sets): return tag_sets +# Distinct from the validator in common.py for URI option "maxStalenessSeconds". def _validate_max_staleness(max_staleness): """Validate max_staleness.""" - if max_staleness is None: - return 0.0 + if max_staleness != -1: + errmsg = "max_staleness must be an integer or float" + try: + max_staleness = float(max_staleness) + except ValueError: + raise ValueError(errmsg) + except TypeError: + raise TypeError(errmsg) - errmsg = "max_staleness must be an integer or float" - try: - max_staleness = float(max_staleness) - except ValueError: - raise ValueError(errmsg) - except TypeError: - raise TypeError(errmsg) - - if not 0 < max_staleness < 1e9: - raise ValueError( - "max_staleness must be greater than 0 and less than one billion") + if not 0 < max_staleness < 1e9: + raise ValueError( + "max_staleness must be greater than 0" + " and less than one billion") return max_staleness @@ -88,7 +88,7 @@ class _ServerMode(object): __slots__ = ("__mongos_mode", "__mode", "__tag_sets", "__max_staleness") - def __init__(self, mode, tag_sets=None, max_staleness=None): + def __init__(self, mode, tag_sets=None, max_staleness=-1): self.__mongos_mode = _MONGOS_MODES[mode] self.__mode = mode self.__tag_sets = _validate_tag_sets(tag_sets) @@ -107,7 +107,7 @@ class _ServerMode(object): doc = {'mode': self.__mongos_mode} if self.__tag_sets not in (None, [{}]): doc['tags'] = self.__tag_sets - if self.__max_staleness: + if self.__max_staleness != -1: doc['maxStalenessSeconds'] = self.__max_staleness return doc @@ -136,9 +136,7 @@ class _ServerMode(object): def max_staleness(self): """The maximum estimated length of time (in seconds) a replica set secondary can fall behind the primary in replication before it will - no longer be selected for operations.""" - if not self.__max_staleness: - return None + no longer be selected for operations, or -1 for no maximum.""" return self.__max_staleness @property @@ -152,11 +150,11 @@ class _ServerMode(object): `min_wire_version`, or the driver raises :exc:`~pymongo.errors.ConfigurationError`. """ - return 5 if self.__max_staleness else 0 + return 0 if self.__max_staleness == -1 else 5 def __repr__(self): return "%s(tag_sets=%r, max_staleness=%r)" % ( - self.name, self.__tag_sets, self.max_staleness) + self.name, self.__tag_sets, self.__max_staleness) def __eq__(self, other): if isinstance(other, _ServerMode): @@ -182,7 +180,7 @@ class _ServerMode(object): self.__mode = value['mode'] self.__mongos_mode = _MONGOS_MODES[self.__mode] self.__tag_sets = _validate_tag_sets(value['tag_sets']) - self.__max_staleness = value['max_staleness'] + self.__max_staleness = _validate_max_staleness(value['max_staleness']) class Primary(_ServerMode): @@ -227,9 +225,10 @@ class PrimaryPreferred(_ServerMode): - `max_staleness`: (integer or float, in seconds) The maximum estimated length of time a replica set secondary can fall behind the primary in replication before it will no longer be selected for operations. + Default -1, meaning no maximum. """ - def __init__(self, tag_sets=None, max_staleness=None): + def __init__(self, tag_sets=None, max_staleness=-1): super(PrimaryPreferred, self).__init__(_PRIMARY_PREFERRED, tag_sets, max_staleness) @@ -260,9 +259,10 @@ class Secondary(_ServerMode): - `max_staleness`: (integer or float, in seconds) The maximum estimated length of time a replica set secondary can fall behind the primary in replication before it will no longer be selected for operations. + Default -1, meaning no maximum. """ - def __init__(self, tag_sets=None, max_staleness=None): + def __init__(self, tag_sets=None, max_staleness=-1): super(Secondary, self).__init__(_SECONDARY, tag_sets, max_staleness) def __call__(self, selection): @@ -288,9 +288,10 @@ class SecondaryPreferred(_ServerMode): - `max_staleness`: (integer or float, in seconds) The maximum estimated length of time a replica set secondary can fall behind the primary in replication before it will no longer be selected for operations. + Default -1, meaning no maximum. """ - def __init__(self, tag_sets=None, max_staleness=None): + def __init__(self, tag_sets=None, max_staleness=-1): super(SecondaryPreferred, self).__init__(_SECONDARY_PREFERRED, tag_sets, max_staleness) @@ -323,9 +324,10 @@ class Nearest(_ServerMode): - `max_staleness`: (integer or float, in seconds) The maximum estimated length of time a replica set secondary can fall behind the primary in replication before it will no longer be selected for operations. + Default -1, meaning no maximum. """ - def __init__(self, tag_sets=None, max_staleness=None): + def __init__(self, tag_sets=None, max_staleness=-1): super(Nearest, self).__init__(_NEAREST, tag_sets, max_staleness) def __call__(self, selection): @@ -340,12 +342,12 @@ _ALL_READ_PREFERENCES = (Primary, PrimaryPreferred, Secondary, SecondaryPreferred, Nearest) -def make_read_preference(mode, tag_sets, max_staleness=None): +def make_read_preference(mode, tag_sets, max_staleness=-1): if mode == _PRIMARY: if tag_sets not in (None, [{}]): raise ConfigurationError("Read preference primary " "cannot be combined with tags") - if max_staleness: + if max_staleness != -1: raise ConfigurationError("Read preference primary cannot be " "combined with maxStalenessSeconds") return Primary() diff --git a/test/test_max_staleness.py b/test/test_max_staleness.py index 72055e0c6..93f58d06e 100644 --- a/test/test_max_staleness.py +++ b/test/test_max_staleness.py @@ -18,6 +18,7 @@ import datetime import os import time import sys +import warnings sys.path[0:0] = [""] @@ -162,7 +163,7 @@ def create_test(scenario_def): mode_string = pref_def.get('mode', 'primary') mode_string = mode_string[:1].lower() + mode_string[1:] mode = read_preferences.read_pref_mode_from_name(mode_string) - max_staleness = pref_def.get('maxStalenessSeconds') + max_staleness = pref_def.get('maxStalenessSeconds', -1) tag_sets = pref_def.get('tag_sets') if scenario_def.get('error'): @@ -224,11 +225,19 @@ create_tests() class TestMaxStaleness(unittest.TestCase): def test_max_staleness(self): + client = MongoClient() + self.assertEqual(-1, client.read_preference.max_staleness) + + client = MongoClient("mongodb://a/?readPreference=secondary") + self.assertEqual(-1, client.read_preference.max_staleness) + # These tests are specified in max-staleness-tests.rst. with self.assertRaises(ConfigurationError): + # Default read pref "primary" can't be used with max staleness. MongoClient("mongodb://a/?maxStalenessSeconds=120") with self.assertRaises(ConfigurationError): + # Read pref "primary" can't be used with max staleness. MongoClient("mongodb://a/?readPreference=primary&" "maxStalenessSeconds=120") @@ -240,6 +249,32 @@ class TestMaxStaleness(unittest.TestCase): "maxStalenessSeconds=1") self.assertEqual(1, client.read_preference.max_staleness) + client = MongoClient("mongodb://a/?readPreference=secondary&" + "maxStalenessSeconds=-1") + self.assertEqual(-1, client.read_preference.max_staleness) + + client = MongoClient(maxStalenessSeconds=-1, readPreference="nearest") + self.assertEqual(-1, client.read_preference.max_staleness) + + with self.assertRaises(TypeError): + # Prohibit None. + MongoClient(maxStalenessSeconds=None, readPreference="nearest") + + def test_max_staleness_zero(self): + # Zero is too small. + with self.assertRaises(ValueError) as ctx: + rs_or_single_client(maxStalenessSeconds=0, + readPreference="nearest") + + self.assertIn("must be greater than 0", str(ctx.exception)) + + with warnings.catch_warnings(record=True) as ctx: + warnings.simplefilter("always") + MongoClient("mongodb://host/?maxStalenessSeconds=0" + "&readPreference=nearest") + + self.assertIn("must be greater than 0", str(ctx[0])) + @client_context.require_version_min(3, 3, 6) # SERVER-8858 def test_last_write_date(self): # From max-staleness-tests.rst, "Parse lastWriteDate". diff --git a/test/test_topology.py b/test/test_topology.py index f1e0eb22b..70bb0bf26 100644 --- a/test/test_topology.py +++ b/test/test_topology.py @@ -710,12 +710,12 @@ class TestServerSelectionErrors(TopologyTest): self.assertMessage( 'No replica set members match selector' - ' "Secondary(tag_sets=None, max_staleness=None)"', + ' "Secondary(tag_sets=None, max_staleness=-1)"', t, ReadPreference.SECONDARY) self.assertMessage( "No replica set members match selector" - " \"Secondary(tag_sets=[{'dc': 'ny'}], max_staleness=None)\"", + " \"Secondary(tag_sets=[{'dc': 'ny'}], max_staleness=-1)\"", t, Secondary(tag_sets=[{'dc': 'ny'}])) def test_bad_replica_set_name(self):