From 45e16bdfdf2a48b9a58671083ac309e1c3c37fd9 Mon Sep 17 00:00:00 2001 From: Eric Jackson Date: Tue, 19 Jun 2018 15:07:30 -0400 Subject: [PATCH] Remove creation of unused partitions, implement recommendation The bluestore partition process has been working from original assumptions. The refactor of bluestore args created a disconnect between the two steps. Partitions are getting created, but never used nor removed. This also implements the recommendation to use a single DB partition since the WAL will naturally use an existing DB. This reduces the number of managed partitions from two to one for each OSD. Note that osd.report is intentionally not changed to give the admin an indication that the configuration can be improved. Signed-off-by: Eric Jackson --- srv/salt/_modules/osd.py | 37 +++++++++++++++++--------------- tests/unit/_modules/test_osd.py | 38 ++++++++++++++------------------- 2 files changed, 36 insertions(+), 39 deletions(-) diff --git a/srv/salt/_modules/osd.py b/srv/salt/_modules/osd.py index d9fa293e2..81c1f12ca 100644 --- a/srv/salt/_modules/osd.py +++ b/srv/salt/_modules/osd.py @@ -898,14 +898,16 @@ def _bluestore_partitions(self): log.warning(("WAL size is unsupported for same device of " "{}".format(self.osd.device))) else: - # Create wal of wal_size on wal device - self.create(self.osd.wal, [('wal', self.osd.wal_size)]) + log.info(("WAL will reside on same device {} as db - " + "recommend removing the WAL entry from the " + "configuration for device " + "{}").format(self.osd.db, self.osd.device)) else: # pylint: disable=line-too-long log.warning("No size specified for wal {}. Using default sizes.".format(self.osd.wal)) if self.osd.db_size: - if self.osd.wal == self.osd.device: + if self.osd.db == self.osd.device: # pylint: disable=line-too-long log.warning("DB size is unsupported for same device of {}".format(self.osd.device)) else: @@ -914,25 +916,22 @@ def _bluestore_partitions(self): else: log.warning("No size specified for db {}. Using default sizes".format(self.osd.db)) else: - # This situation seems unintentional - use faster media for - # the wal or db but not the other. Help newbies out by - # putting wal and db on same device + # Only WAL if self.osd.wal: if self.osd.wal_size: if self.osd.wal == self.osd.device: log.warning(("WAL size is unsupported for same device of " "{}".format(self.osd.device))) else: - log.warning("Setting db to same device {} as wal".format(self.osd.wal)) + log.info("DB will reside on device {}".format(self.osd.device)) # Create wal of wal_size on wal device - # Create db on wal device - self.create(self.osd.wal, [('wal', self.osd.wal_size), - ('db', self._halve(self.osd.wal_size))]) + self.create(self.osd.wal, [('wal', self.osd.wal_size)]) else: if self.osd.wal_size: log.warning(("WAL size is unsupported for same device of " "{}".format(self.osd.device))) + # Only DB if self.osd.db: if self.osd.db_size: if self.osd.db == self.osd.device: @@ -941,9 +940,7 @@ def _bluestore_partitions(self): else: log.warning("Setting wal to same device {} as db".format(self.osd.db)) # Create db of db_size on db device - # Create wal on db device - self.create(self.osd.db, [('wal', self._double(self.osd.db_size)), - ('db', self.osd.db_size)]) + self.create(self.osd.db, [('db', self.osd.db_size)]) else: if self.osd.db_size: log.warning(("DB size is unsupported for same device of " @@ -1173,11 +1170,17 @@ def _bluestore_args(self): # WAL cornercase with sizes if self.osd.wal and self.osd.wal_size and self.osd.wal != self.osd.device: - _partition = self.highest_partition(self.osd.wal, 'wal') - if _partition: - args += "--block.wal {}{} ".format(self.osd.wal, _partition) + if self.osd.db: + log.warning(("Ignoring WAL setting - " + "No need for two partitions, " + "WAL will use same device as DB " + "{}").format(self.osd.db)) else: - args += "--block.wal {} ".format(self.osd.wal) + _partition = self.highest_partition(self.osd.wal, 'wal') + if _partition: + args += "--block.wal {}{} ".format(self.osd.wal, _partition) + else: + args += "--block.wal {} ".format(self.osd.wal) # DB cornercase with sizes if self.osd.db and self.osd.db_size and self.osd.db != self.osd.device: diff --git a/tests/unit/_modules/test_osd.py b/tests/unit/_modules/test_osd.py index 69c82ac48..ec78b8b04 100644 --- a/tests/unit/_modules/test_osd.py +++ b/tests/unit/_modules/test_osd.py @@ -814,8 +814,7 @@ def test_bluestore_partitions_wal_and_db_all_size_no_eq(self, create_mock): And I have a wal_size And I have a db_size And wal is not equivalent to the device - Expect to call create('/dev/sddb', [('db', db_size)]) - Expect to call create('/dev/sdwal', [('wal', wal_size)]) + Expect to call create('/dev/sddb', [('db', db_size)]) only """ kwargs = {'format': 'bluestore', 'wal': '/dev/sdwal', @@ -826,8 +825,8 @@ def test_bluestore_partitions_wal_and_db_all_size_no_eq(self, create_mock): test_module = helper_specs(module=DEFAULT_MODULE)() obj = test_module.OSDPartitions(osd_config) obj._bluestore_partitions() - create_mock.assert_any_call('/dev/sdwal', [('wal', 'walsize')]) create_mock.assert_any_call('/dev/sddb', [('db', 'dbsize')]) + assert create_mock.call_count == 1 @mock.patch('srv.salt._modules.osd.OSDPartitions.create') @mock.patch('srv.salt._modules.osd.log') @@ -860,7 +859,8 @@ def test_bluestore_partitions_wal_and_db_wal_size_no_eq(self, mock_log, create_m And I have a wal_size And I do not have a db_size And wal is not equivalent to the device - Expect to call create('/dev/sdx', [('wal', wal_size)]) + + Expect WAL to reside on DB device """ kwargs = {'format': 'bluestore', 'wal': '/dev/sdwal', @@ -871,7 +871,7 @@ def test_bluestore_partitions_wal_and_db_wal_size_no_eq(self, mock_log, create_m test_module = helper_specs(module=DEFAULT_MODULE)() obj = test_module.OSDPartitions(osd_config) obj._bluestore_partitions() - create_mock.assert_any_call('/dev/sdwal', [('wal', 'walsize')]) + create_mock.call_count == 0 @mock.patch('srv.salt._modules.osd.log') def test_bluestore_partitions_wal_and_db_no_sizes_no_eq(self, mock_log): @@ -913,16 +913,12 @@ def test_bluestore_partitions_no_waldb_only_wal_and_size(self, mock_log): obj._bluestore_partitions() mock_log.warning.assert_any_call('WAL size is unsupported for same device of /dev/sdx') - @mock.patch('srv.salt._modules.osd.OSDPartitions._halve') @mock.patch('srv.salt._modules.osd.OSDPartitions.create') - @mock.patch('srv.salt._modules.osd.log') - def test_bluestore_partitions_no_waldb_only_wal_and_size_no_eq(self, mock_log, create_mock, halve_mock): + def test_bluestore_partitions_no_waldb_only_wal_and_size_no_eq(self, create_mock): """ Given I defined only wal And I have a wal_size And wal is not equivalent to the device - Expect to call log() - Expect to call _halve() Expect to call create() """ kwargs = {'format': 'bluestore', @@ -934,9 +930,7 @@ def test_bluestore_partitions_no_waldb_only_wal_and_size_no_eq(self, mock_log, c test_module = helper_specs(module=DEFAULT_MODULE)() obj = test_module.OSDPartitions(osd_config) obj._bluestore_partitions() - mock_log.warning.assert_called_with('Setting db to same device /dev/sdwal as wal') - create_mock.assert_called_with('/dev/sdwal', [('wal', 100000), ('db', halve_mock('100000'))]) - halve_mock.assert_called_with('100000') + create_mock.assert_called_with('/dev/sdwal', [('wal', 100000)]) @mock.patch('srv.salt._modules.osd.log') def test_bluestore_partitions_no_waldb_only_wal_and_no_size(self, mock_log): @@ -956,8 +950,9 @@ def test_bluestore_partitions_no_waldb_only_wal_and_no_size(self, mock_log): obj._bluestore_partitions() mock_log.warning.assert_called_with('WAL size is unsupported for same device of /dev/sdx') + @mock.patch('srv.salt._modules.osd.OSDPartitions.create') @mock.patch('srv.salt._modules.osd.log') - def test_bluestore_partitions_no_waldb_only_db_and_size_eq_log(self, mock_log): + def test_bluestore_partitions_no_waldb_only_db_and_size_eq_log(self, mock_log, create_mock): """ Given I haven't defined wal but a db And I have a db_size @@ -968,17 +963,16 @@ def test_bluestore_partitions_no_waldb_only_db_and_size_eq_log(self, mock_log): 'wal': '/dev/sdx', # temp fix until #340 is merged 'db': '/dev/sddb', 'wal_size': None, - 'db_size': 'dbsize'} + 'db_size': 100000} osd_config = OSDConfig(**kwargs) test_module = helper_specs(module=DEFAULT_MODULE)() obj = test_module.OSDPartitions(osd_config) obj._bluestore_partitions() - mock_log.warning.assert_called_with('DB size is unsupported for same device of /dev/sdx') + create_mock.assert_called_with('/dev/sddb', [('db', 100000)]) - @mock.patch('srv.salt._modules.osd.OSDPartitions._double') @mock.patch('srv.salt._modules.osd.OSDPartitions.create') @mock.patch('srv.salt._modules.osd.log') - def test_bluestore_partitions_no_waldb_only_db_and_size_no_eq_create(self, mock_log, create_mock, double_mock): + def test_bluestore_partitions_no_waldb_only_db_and_size_no_eq_create(self, mock_log, create_mock): """ Given I haven't defined wal but a db And I have a db_size @@ -996,8 +990,7 @@ def test_bluestore_partitions_no_waldb_only_db_and_size_no_eq_create(self, mock_ obj = test_module.OSDPartitions(osd_config) obj._bluestore_partitions() mock_log.warning.assert_called_with('Setting wal to same device /dev/sddb as db') - create_mock.assert_called_with('/dev/sddb', [('wal', double_mock(100000)), ('db', 100000)]) - double_mock.assert_called_with(100000) + create_mock.assert_called_with('/dev/sddb', [('db', 100000)]) @mock.patch('srv.salt._modules.osd.log') def test_bluestore_partitions_no_waldb_no_db_log(self, mock_log): @@ -1591,8 +1584,9 @@ def test_bluestore_args_1(self, ip_mock, hp_mock, osdc_o): And and the device is a NVME Expect args to be: - --block.wal /dev/sdwal1 --block.db /dev/sddb1 /dev/nvme0n1p1 + --block.db /dev/sddb1 /dev/nvme0n1p1 + since the WAL will use the DB """ kwargs = {'wal': '/dev/sdwal', 'wal_size': '2G', @@ -1604,7 +1598,7 @@ def test_bluestore_args_1(self, ip_mock, hp_mock, osdc_o): ip_mock.side_effect = [True] hp_mock.side_effect = [1, 1] ret = obj._bluestore_args() - assert ret == "--block.wal /dev/sdwal1 --block.db /dev/sddb1 /dev/nvme0n1p1" + assert ret == "--block.db /dev/sddb1 /dev/nvme0n1p1" @mock.patch('srv.salt._modules.osd.OSDCommands.highest_partition') @mock.patch('srv.salt._modules.osd.OSDCommands.is_partitioned')