From 1ad364484f98633e9b35a1f75c298121a1bda4cc Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Wed, 23 Sep 2015 14:23:55 -0400 Subject: [PATCH] Adds support for write concern Pulp now accepts a write concern for the MongoDB connection. All writes to the database are now acknowledged. For replica sets, 'majority' or 'all' members can be asked to acknowledge the write. closes: #974 closes: #1139 --- common/pulp/common/error_codes.py | 2 + docs/user-guide/release-notes/2.7.x.rst | 4 ++ pulp.spec | 1 + server/etc/pulp/server.conf | 6 +++ server/pulp/server/config.py | 1 + server/pulp/server/db/connection.py | 37 +++++++++------ server/test/unit/server/db/test_connection.py | 46 ++++++++++++++++--- 7 files changed, 77 insertions(+), 20 deletions(-) diff --git a/common/pulp/common/error_codes.py b/common/pulp/common/error_codes.py index 3d267a99aa..0855c1828d 100644 --- a/common/pulp/common/error_codes.py +++ b/common/pulp/common/error_codes.py @@ -100,6 +100,8 @@ PLP0041 = Error("PLP0041", _("Database 'replica_set' config must be specified when more than one " "seed is provided. Refer to /etc/pulp/server.conf for proper use."), []) +PLP0043 = Error("PLP0043", _("Database 'write_concern' config can only be 'majority' or 'all'. " + "Refer to /etc/pulp/server.conf for proper use."), []) # Create a section for general validation errors (PLP1000 - PLP2999) # Validation problems should be reported with a general PLP1000 error with a more specific diff --git a/docs/user-guide/release-notes/2.7.x.rst b/docs/user-guide/release-notes/2.7.x.rst index 965f3c05ec..dd6c04d0a0 100644 --- a/docs/user-guide/release-notes/2.7.x.rst +++ b/docs/user-guide/release-notes/2.7.x.rst @@ -52,6 +52,10 @@ New Features * Pulp properly connects to MongoDB replica sets. The `replica_set` setting in `database` section of `/etc/pulp/server.conf` must be provided for Pulp to connect to a replica set. +* Pulp allows users to specify the write concern for MongoDB connection. The two options are + 'majority' and 'all'. When 'all' is specified, the number of seeds listed will be used as the value + for 'w'. For installations without replica sets, all writes to the database will be acknowledged. + Deprecation ----------- diff --git a/pulp.spec b/pulp.spec index 9d9de27ab4..210531c87b 100644 --- a/pulp.spec +++ b/pulp.spec @@ -312,6 +312,7 @@ Requires: python-httplib2 Requires: python-isodate >= 0.5.0-1.pulp Requires: python-qpid Requires: python-nectar >= 1.1.6 +Requires: python-semantic-version >= 2.2.0 Requires: httpd Requires: mod_ssl Requires: openssl diff --git a/server/etc/pulp/server.conf b/server/etc/pulp/server.conf index 0542587af4..811a454cb6 100644 --- a/server/etc/pulp/server.conf +++ b/server/etc/pulp/server.conf @@ -39,6 +39,11 @@ # of the connection. # unsafe_autoretry: If true, retry commands to the database if there is a connection error. # Warning: if set to true, this setting can result in duplicate records. +# write_concern: Write concern of 'majority' or 'all'. When 'all' is specified, 'w' is set to +# number of seeds specified. For version of MongoDB < 2.6, replica_set must also +# be specified. Please note that 'all' will cause Pulp to halt if any of the +# replica set members is not available. 'majority' is used by default. + [database] # name: pulp_database @@ -52,6 +57,7 @@ # verify_ssl: true # ca_path: /etc/pki/tls/certs/ca-bundle.crt # unsafe_autoretry: false +# write_concern: majority # = Server = diff --git a/server/pulp/server/config.py b/server/pulp/server/config.py index 2dbea52641..bbe89d8443 100644 --- a/server/pulp/server/config.py +++ b/server/pulp/server/config.py @@ -34,6 +34,7 @@ 'verify_ssl': 'true', 'ca_path': '/etc/pki/tls/certs/ca-bundle.crt', 'unsafe_autoretry': 'false', + 'write_concern': 'majority', }, 'email': { 'host': 'localhost', diff --git a/server/pulp/server/db/connection.py b/server/pulp/server/db/connection.py index 507445d583..4151395e1c 100644 --- a/server/pulp/server/db/connection.py +++ b/server/pulp/server/db/connection.py @@ -18,14 +18,16 @@ from pulp.server.compat import wraps from pulp.server.exceptions import PulpCodedException, PulpException +import semantic_version + _CONNECTION = None _DATABASE = None _DEFAULT_MAX_POOL_SIZE = 10 # please keep this in X.Y.Z format, with only integers. # see version.cpp in mongo source code for version format info. -MONGO_MINIMUM_VERSION = "2.4.0" - +MONGO_MINIMUM_VERSION = semantic_version.Version("2.4.0") +MONGO_WRITE_CONCERN_VERSION = semantic_version.Version("2.6.0") _logger = logging.getLogger(__name__) @@ -48,6 +50,7 @@ def initialize(name=None, seeds=None, max_pool_size=None, replica_set=None, max_ if seeds is None: seeds = config.config.get('database', 'seeds') + seeds_list = seeds.split(',') if max_pool_size is None: # we may want to make this configurable, but then again, we may not @@ -61,6 +64,12 @@ def initialize(name=None, seeds=None, max_pool_size=None, replica_set=None, max_ if replica_set is not None: connection_kwargs['replicaSet'] = replica_set + write_concern = config.config.get('database', 'write_concern') + if write_concern not in ['majority', 'all']: + raise PulpCodedException(error_code=error_codes.PLP0043) + elif write_concern == 'all': + write_concern = len(seeds_list) + # Process SSL settings if config.config.getboolean('database', 'ssl'): connection_kwargs['ssl'] = True @@ -91,12 +100,24 @@ def initialize(name=None, seeds=None, max_pool_size=None, replica_set=None, max_ itertools.repeat(32)) if seeds != '': - seeds_list = seeds.split(',') if len(seeds_list) > 1 and not replica_set: raise PulpCodedException(error_code=error_codes.PLP0041) while True: _CONNECTION = _connect_to_one_of_seeds(connection_kwargs, seeds_list, name) if _CONNECTION: + db_version = semantic_version.Version(_CONNECTION.server_info()['version']) + if db_version < MONGO_MINIMUM_VERSION: + raise RuntimeError(_("Pulp requires Mongo version %s, but DB is reporting" + "version %s") % (MONGO_MINIMUM_VERSION, + db_version)) + elif db_version >= MONGO_WRITE_CONCERN_VERSION or replica_set: + # Write concern of 'majority' only works with a replica set or when using + # MongoDB >= 2.6.0 + _CONNECTION.write_concern['w'] = write_concern + else: + _CONNECTION.write_concern['w'] = 1 + _logger.info(_("Write concern for Mongo connection: %s") % + _CONNECTION.write_concern) break else: next_delay = min(mongo_retry_timeout_seconds_generator.next(), max_timeout) @@ -120,16 +141,6 @@ def initialize(name=None, seeds=None, max_pool_size=None, replica_set=None, max_ # Query the collection names to ensure that we are authenticated properly _logger.debug(_('Querying the database to validate the connection.')) _DATABASE.collection_names() - - db_version = _CONNECTION.server_info()['version'] - _logger.info(_("Mongo database for connection is version %s") % db_version) - - db_version_tuple = tuple(db_version.split(".")) - db_min_version_tuple = tuple(MONGO_MINIMUM_VERSION.split(".")) - if db_version_tuple < db_min_version_tuple: - raise RuntimeError(_("Pulp requires Mongo version %s, but DB is reporting version %s") % - (MONGO_MINIMUM_VERSION, db_version)) - except Exception, e: _logger.critical(_('Database initialization failed: %s') % str(e)) _CONNECTION = None diff --git a/server/test/unit/server/db/test_connection.py b/server/test/unit/server/db/test_connection.py index 5b470cb45c..2d9b1914b5 100644 --- a/server/test/unit/server/db/test_connection.py +++ b/server/test/unit/server/db/test_connection.py @@ -2,17 +2,23 @@ from mock import call, patch, MagicMock, Mock from pymongo.errors import AutoReconnect +import unittest2 + +from pulp.common import error_codes from pulp.server import config from pulp.server.db import connection from pulp.server.exceptions import PulpCodedException +MONGO_MIN_TEST_VERSION = "2.4.0" + + class MongoEngineConnectionError(Exception): pass -class TestDatabaseSeeds(unittest.TestCase): +class TestDatabaseSeeds(unittest2.TestCase): def tearDown(self): # Reload the configuration so that things are cleaned up properly @@ -72,6 +78,32 @@ def test_seeds_from_config(self, mock_connect_seeds, mock_mongoengine): 'secondhost:5678'], database) + @patch('pulp.server.db.connection.mongoengine') + @patch('pulp.server.db.connection._connect_to_one_of_seeds') + def test_multiple_seeds_no_replica_set(self, mock_connect_seeds, mock_mongoengine): + mock_mongoengine.connect.return_value.server_info.return_value = {'version': '2.6.0'} + mock_connect_seeds.return_value.server_info.return_value = {'version': '2.6.0'} + seeds = "firsthost:1234,secondhost:5678" + config.config.set('database', 'write_concern', 'majority') + config.config.set('database', 'seeds', seeds) + with self.assertRaises(PulpCodedException) as connection_error: + connection.initialize() + self.assertEqual(connection_error.exception.error_code.message, error_codes.PLP0041.message) + + @patch('pulp.server.db.connection.mongoengine') + @patch('pulp.server.db.connection._connect_to_one_of_seeds') + def test_invalid_write_concern(self, mock_connect_seeds, mock_mongoengine): + mock_mongoengine.connect.return_value.server_info.return_value = {'version': '2.6.0'} + mock_connect_seeds.return_value.server_info.return_value = {'version': '2.6.0'} + seeds = "firsthost:1234,secondhost:5678" + replica_set = 'fakeReplica' + config.config.set('database', 'write_concern', 'blah') + config.config.set('database', 'seeds', seeds) + config.config.set('database', 'replica_set', replica_set) + with self.assertRaises(PulpCodedException) as connection_error: + connection.initialize() + self.assertEqual(connection_error.exception.error_code.message, error_codes.PLP0043.message) + class TestDatabaseName(unittest.TestCase): @@ -411,7 +443,7 @@ def tearDown(self): def test_initialize_username_and_password(self, mock_mongoengine): mock_mongoengine_instance = mock_mongoengine.connect.return_value mock_mongoengine_instance.server_info.return_value = {"version": - connection.MONGO_MINIMUM_VERSION} + MONGO_MIN_TEST_VERSION} config.config.set('database', 'name', 'nbachamps') config.config.set('database', 'username', 'larrybird') config.config.set('database', 'password', 'celtics1981') @@ -429,7 +461,7 @@ def test_initialize_username_and_password(self, mock_mongoengine): def test_initialize_username_and_shadows_password(self, mock_mongoengine, mock_log): mock_mongoengine_instance = mock_mongoengine.connect.return_value mock_mongoengine_instance.server_info.return_value = {"version": - connection.MONGO_MINIMUM_VERSION} + MONGO_MIN_TEST_VERSION} config.config.set('database', 'name', 'nbachamps') config.config.set('database', 'username', 'larrybird') config.config.set('database', 'password', 'celtics1981') @@ -453,7 +485,7 @@ def test_initialize_username_and_shadows_password(self, mock_mongoengine, mock_l def test_initialize_no_username_or_password(self, mock_mongoengine): mock_mongoengine_instance = mock_mongoengine.connect.return_value mock_mongoengine_instance.server_info.return_value = {"version": - connection.MONGO_MINIMUM_VERSION} + MONGO_MIN_TEST_VERSION} config.config.set('database', 'username', '') config.config.set('database', 'password', '') connection.initialize() @@ -463,7 +495,7 @@ def test_initialize_no_username_or_password(self, mock_mongoengine): def test_initialize_username_no_password(self, mock_mongoengine): mock_mongoengine_instance = mock_mongoengine.connect.return_value mock_mongoengine_instance.server_info.return_value = {"version": - connection.MONGO_MINIMUM_VERSION} + MONGO_MIN_TEST_VERSION} config.config.set('database', 'username', 'admin') config.config.set('database', 'password', '') # ensure no exception is raised (redmine #708) @@ -473,7 +505,7 @@ def test_initialize_username_no_password(self, mock_mongoengine): def test_initialize_password_no_username(self, mock_mongoengine): mock_mongoengine_instance = mock_mongoengine.connect.return_value mock_mongoengine_instance.server_info.return_value = {"version": - connection.MONGO_MINIMUM_VERSION} + MONGO_MIN_TEST_VERSION} config.config.set('database', 'username', '') config.config.set('database', 'password', 'foo') self.assertRaises(Exception, connection.initialize) @@ -483,7 +515,7 @@ def test_initialize_password_no_username(self, mock_mongoengine): def test_authentication_fails_with_RuntimeError(self, mock_mongoengine): mock_mongoengine_instance = mock_mongoengine.connect.return_value mock_mongoengine_instance.server_info.return_value = {"version": - connection.MONGO_MINIMUM_VERSION} + MONGO_MIN_TEST_VERSION} exc = MongoEngineConnectionError() exc.code = 18 mock_mongoengine.connection.get_db.side_effect = exc