openstack/swift
Code Issues Proposed changes

sharder: don't cleave DB if own_shard_range missing

Browse Source 
The sharder aborts cleaving shards if the root DB does not have an
own_shard_range (see Related-Change). Previously, the cleaving was
aborted *after* a shard broker had been created, which potentially
leaves an unused shard DB in a handoff location. This shard DB would
eventually be replicated and cleaned up by the replicator, but its
creation can be avoided by checking the root DB own_shard_range
earlier.

Change-Id: I05ec542faa452db2053302910d34b93f0abd10a7
Related-Change: I68c3e7558d04e8dcd5874f80b4763ec0b820331d
This commit is contained in:
Alistair Coles 2022-02-17 12:31:10 +00:00
parent 3358345682
commit 6609fcd9a1
2 changed files with 36 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
swift/container/sharder.py
View File

@ -1603,7 +1603,8 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
len(created_ranges))
return len(created_ranges)
def _cleave_shard_range(self, broker, cleaving_context, shard_range):
def _cleave_shard_range(self, broker, cleaving_context, shard_range,
own_shard_range):
self.logger.info("Cleaving '%s' from row %s into %s for %r",
quote(broker.path),
cleaving_context.last_cleave_to_row,
@ -1620,15 +1621,6 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
self._increment_stat('cleaved', 'failure', statsd=True)
return CLEAVE_FAILED
own_shard_range = broker.get_own_shard_range(no_default=True)
if own_shard_range is None:
# A default should never be SHRINKING or SHRUNK but because we
# may write own_shard_range back to broker, let's make sure
# it can't be defaulted.
self.logger.warning('Failed to get own_shard_range for %s',
quote(broker.path))
return CLEAVE_FAILED
# only cleave from the retiring db - misplaced objects handler will
# deal with any objects in the fresh db
source_broker = broker.get_brokers()[0]
@ -1796,6 +1788,15 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
self.logger.debug('Starting to cleave (%s todo): %s',
cleaving_context.ranges_todo, quote(broker.path))
own_shard_range = broker.get_own_shard_range(no_default=True)
if own_shard_range is None:
# A default should never be SHRINKING or SHRUNK but because we
# may write own_shard_range back to broker, let's make sure
# it can't be defaulted.
self.logger.warning('Failed to get own_shard_range for %s',
quote(broker.path))
ranges_todo = [] # skip cleaving
ranges_done = []
for shard_range in ranges_todo:
if cleaving_context.cleaving_done:
@ -1820,7 +1821,7 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
break
cleave_result = self._cleave_shard_range(
broker, cleaving_context, shard_range)
broker, cleaving_context, shard_range, own_shard_range)
if cleave_result == CLEAVE_SUCCESS:
ranges_done.append(shard_range)

38
test/unit/container/test_sharder.py
View File

@ -42,8 +42,7 @@ from swift.container.sharder import ContainerSharder, sharding_enabled, \
CleavingContext, DEFAULT_SHARDER_CONF, finalize_shrinking, \
find_shrinking_candidates, process_compactible_shard_sequences, \
find_compactible_shard_sequences, is_shrinking_candidate, \
is_sharding_candidate, find_paths, rank_paths, ContainerSharderConf, \
CLEAVE_FAILED
is_sharding_candidate, find_paths, rank_paths, ContainerSharderConf
from swift.common.utils import ShardRange, Timestamp, hash_path, \
encode_timestamps, parse_db_filename, quorum_size, Everything, md5
from test import annotate_failure
@ -1696,25 +1695,36 @@ class TestSharder(BaseTestSharder):
self.assertFalse(cleaving_context.cleaving_done)
def test_cleave_shard_range_no_own_shard_range(self):
broker = self._make_sharding_broker()
# create an unsharded broker that has shard ranges but no
# own_shard_range, verify that it does not cleave...
broker = self._make_broker()
broker.set_sharding_sysmeta('Quoted-Root', 'a/c')
shard_ranges = self._make_shard_ranges(
(('', 'middle'), ('middle', '')),
state=ShardRange.CLEAVED)
broker.merge_shard_ranges(shard_ranges)
obj = {'name': 'obj', 'created_at': next(self.ts_iter).internal,
'size': 14, 'content_type': 'text/plain', 'etag': 'an etag',
'deleted': 0}
broker.get_brokers()[0].merge_items([obj])
self.assertEqual(2, len(broker.db_files)) # sanity check
context = CleavingContext.load(broker)
shard_range = broker.get_shard_ranges()[0]
with self._mock_sharder() as sharder, mock.patch(
'swift.container.backend.ContainerBroker.get_own_shard_range',
return_value=None):
self.assertEqual(
sharder._cleave_shard_range(broker, context, shard_range),
CLEAVE_FAILED)
self.assertEqual(SHARDING, broker.get_db_state())
with self._mock_sharder() as sharder:
self.assertFalse(sharder._cleave(broker))
self.assertEqual(UNSHARDED, broker.get_db_state())
warning_lines = sharder.logger.get_lines_for_level('warning')
self.assertEqual(warning_lines[0],
'Failed to get own_shard_range for a/c')
sharder._replicate_object.assert_not_called()
context = CleavingContext.load(broker)
self.assertTrue(context.misplaced_done)
self.assertFalse(context.cleaving_done)
# only the root broker on disk
suffix_dir = os.path.dirname(broker.db_dir)
self.assertEqual([os.path.basename(broker.db_dir)],
os.listdir(suffix_dir))
partition_dir = os.path.dirname(suffix_dir)
self.assertEqual([broker.db_dir[-3:]], os.listdir(partition_dir))
containers_dir = os.path.dirname(partition_dir)
self.assertEqual(['0'], os.listdir(containers_dir))
def test_cleave_shard(self):
broker = self._make_broker(account='.shards_a', container='shard_c')