openstack
/
swift
Code Issues Proposed changes

relinker: retry links from older part powers 

If a previous partition power increase failed to cleanup all files in
their old partition locations, then during the next partition power
increase the relinker may find the same file to relink in more than
one source partition. This currently leads to an error log due to the
second relink attempt getting an EEXIST error.

With this patch, when an EEXIST is raised, the relinker will attempt
to create/verify a link from older partition power locations to the
next part power location, and if such a link is found then suppress
the error log.

During the relink step, if an alternative link is verified and if a
file is found that is neither linked to the next partition power
location nor in the current part power location, then the file is
removed during the relink step. That prevents the same EEXIST occuring
again during the cleanup step when it may no longer be possible to
verify that an alternative link exists.

For example, consider identical filenames in the N+1th, Nth and N-1th
partition power locations, with the N+1th being linked to the Nth:

  - During relink, the Nth location is visited and its link is
    verified. Then the N-1th location is visited and an EEXIST error
    is encountered, but the new check verifies that a link exists to
    the Nth location, which is OK.

  - During cleanup the locations are visited in the same order, but
    files are removed so that the Nth location file no longer exists
    when the N-1th location is visited. If the N-1th location still
    has a conflicting file then existence of an alternative link to
    the Nth location can no longer be verified, so an error would be
    raised. Therefore, the N-1th location file must be removed during
    relink.

The error is only suppressed for tombstones. The number of partition
power location that the relinker will look back over may be configured
using the link_check_limit option in a conf file or --link-check-limit
on the command line, and defaults to 2.

Closes-Bug: 1921718
Change-Id: If9beb9efabdad64e81d92708f862146d5fafb16c
This commit is contained in:
Alistair Coles 2021-03-26 13:41:36 +00:00
parent 810e4c0cdd
commit 3bdd01cf4a
6 changed files with 620 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
etc/object-server.conf-sample
View File

@ -614,3 +614,22 @@ use = egg:swift#xprofile
# likelihood that the added I/O from a partition-power increase impacts
# client traffic. Use zero for unlimited.
# files_per_second = 0.0
#
# Maximum number of partition power locations to check for a valid link target
# if the relinker encounters an existing tombstone, but with different inode,
# in the next partition power location. If the relinker fails to make a link
# because a different tombstone already exists in the next partition power
# location then it will try to validate that the existing tombstone links to a
# valid target in the current partition power location, or previous partition
# power locations, in descending order. This option limits the number of
# partition power locations searched, including the current partition power,
# and should be a whole number. A value of 0 implies that no validation is
# attempted, and an error is logged, when an existing tombstone prevents link
# creation. A value of 1 implies that an existing link is accepted if it links
# to a tombstone in the current partition power location. The default value of
# 2 implies that an existing link is acceptable if it links to a tombstone in
# the current or previous partition power locations. Increased values may be
# useful if previous partition power increases have failed to cleanup
# tombstones from their old locations, causing duplicate tombstones with
# different inodes to be relinked to the next partition power location.
# link_check_limit = 2

84
swift/cli/relinker.py
View File

@ -257,6 +257,29 @@ class Relinker(object):
hashes.remove(hsh)
return hashes
def check_existing(self, new_file):
existing_link = None
link_created = False
start = self.next_part_power - 1
stop = max(start - self.conf['link_check_limit'], -1)
for check_part_power in range(start, stop, -1):
# Try to create the link from each of 3 previous part power
# locations. If an attempt succeeds then either a link was made or
# an existing link with the same inode as the next part power
# location was found: either is acceptable. The part power location
# that previously failed with EEXIST is included in the further
# attempts here for simplicity.
target_file = replace_partition_in_path(
self.conf['devices'], new_file, check_part_power)
try:
link_created = diskfile.relink_paths(target_file, new_file,
ignore_missing=False)
existing_link = target_file
break
except OSError:
pass
return existing_link, link_created
def process_location(self, hash_path, new_hash_path):
# Compare the contents of each hash dir with contents of same hash
# dir in its new partition to verify that the new location has the
@ -288,6 +311,7 @@ class Relinker(object):
missing_links = 0
created_links = 0
unwanted_files = []
for filename in required_links:
# Before removing old files, be sure that the corresponding
# required new files exist by calling relink_paths again. There
@ -321,22 +345,52 @@ class Relinker(object):
created_links += 1
self.stats['linked'] += 1
except OSError as exc:
self.logger.warning(
"Error relinking%s: failed to relink %s to "
"%s: %s", ' (cleanup)' if self.do_cleanup else '',
old_file, new_file, exc)
self.stats['errors'] += 1
missing_links += 1
existing_link = None
link_created = False
if exc.errno == errno.EEXIST and filename.endswith('.ts'):
# special case for duplicate tombstones in older partition
# power locations
# (https://bugs.launchpad.net/swift/+bug/1921718)
existing_link, link_created = self.check_existing(new_file)
if existing_link:
self.logger.debug(
"Relinking%s: link not needed: %s to %s due to "
"existing %s", ' (cleanup)' if self.do_cleanup else '',
old_file, new_file, existing_link)
if link_created:
# uncommon case: the retry succeeded in creating a link
created_links += 1
self.stats['linked'] += 1
wanted_file = replace_partition_in_path(
self.conf['devices'], old_file, self.part_power)
if old_file not in (existing_link, wanted_file):
# A link exists to a different file and this file
# is not the current target for client requests. If
# this file is visited again it is possible that
# the existing_link will have been cleaned up and
# the check will fail, so clean it up now.
self.logger.info(
"Relinking%s: cleaning up unwanted file: %s",
' (cleanup)' if self.do_cleanup else '', old_file)
unwanted_files.append(filename)
else:
self.logger.warning(
"Error relinking%s: failed to relink %s to %s: %s",
' (cleanup)' if self.do_cleanup else '',
old_file, new_file, exc)
self.stats['errors'] += 1
missing_links += 1
if created_links:
diskfile.invalidate_hash(os.path.dirname(new_hash_path))
if missing_links or not self.do_cleanup:
return
if self.do_cleanup and not missing_links:
# use the sorted list to help unit testing
unwanted_files = old_df_data['files']
# the new partition hash dir has the most up to date set of on
# disk files so it is safe to delete the old location...
rehash = False
# use the sorted list to help unit testing
for filename in old_df_data['files']:
for filename in unwanted_files:
old_file = os.path.join(hash_path, filename)
try:
os.remove(old_file)
@ -487,6 +541,13 @@ def main(args):
help='Set log file name. Ignored if using conf_file.')
parser.add_argument('--debug', default=False, action='store_true',
help='Enable debug mode')
parser.add_argument('--link-check-limit', type=non_negative_int,
default=None, dest='link_check_limit',
help='Maximum number of partition power locations to '
'check for a valid link target if relink '
'encounters an existing tombstone with different '
'inode in the next partition power location '
'(default: 2).')
args = parser.parse_args(args)
if args.conf_file:
@ -519,6 +580,9 @@ def main(args):
else non_negative_float(conf.get('files_per_second', '0'))),
'policies': set(args.policies) or POLICIES,
'partitions': set(args.partitions),
'link_check_limit': (
args.link_check_limit if args.link_check_limit is not None
else non_negative_int(conf.get('link_check_limit', 2))),
})
if args.action == 'relink':

16
swift/obj/diskfile.py
View File

@ -443,15 +443,19 @@ def invalidate_hash(suffix_dir):
inv_fh.write(suffix + b"\n")
def relink_paths(target_path, new_target_path):
def relink_paths(target_path, new_target_path, ignore_missing=True):
"""
Hard-links a file located in target_path using the second path
new_target_path. Creates intermediate directories if required.
Hard-links a file located in ``target_path`` using the second path
``new_target_path``. Creates intermediate directories if required.
:param target_path: current absolute filename
:param new_target_path: new absolute filename for the hardlink
:param ignore_missing: if True then no exception is raised if the link
could not be made because ``target_path`` did not exist, otherwise an
OSError will be raised.
:raises: OSError if the hard link could not be created, unless the intended
hard link already exists or the target_path does not exist.
hard link already exists or the ``target_path`` does not exist and
``must_exist`` if False.
:returns: True if the link was created by the call to this method, False
otherwise.
"""
@ -473,14 +477,14 @@ def relink_paths(target_path, new_target_path):
ok = False
if err.errno == errno.ENOENT:
# this is ok if the *target* path doesn't exist anymore
ok = not os.path.exists(target_path)
ok = not os.path.exists(target_path) and ignore_missing
if err.errno == errno.EEXIST:
# this is ok *if* the intended link has already been made
try:
orig_stat = os.stat(target_path)
except OSError as sub_err:
# this is ok: the *target* path doesn't exist anymore
ok = sub_err.errno == errno.ENOENT
ok = sub_err.errno == errno.ENOENT and ignore_missing
else:
try:
new_stat = os.stat(new_target_path)

520
test/unit/cli/test_relinker.py
View File

@ -78,8 +78,8 @@ class TestRelinker(unittest.TestCase):
container = 'c'
obj = 'o-' + str(uuid.uuid4())
_hash = utils.hash_path(account, container, obj)
part = utils.get_partition_for_hash(_hash, 8)
next_part = utils.get_partition_for_hash(_hash, 9)
part = utils.get_partition_for_hash(_hash, PART_POWER)
next_part = utils.get_partition_for_hash(_hash, PART_POWER + 1)
obj_path = os.path.join(os.path.sep, account, container, obj)
# There's 1/512 chance that both old and new parts will be 0;
# that's not a terribly interesting case, as there's nothing to do
@ -121,6 +121,21 @@ class TestRelinker(unittest.TestCase):
self.expected_dir = os.path.join(self.next_suffix_dir, self._hash)
self.expected_file = os.path.join(self.expected_dir, self.object_fname)
def _make_link(self, filename, part_power):
# make a file in the older part_power location and link it to a file in
# the next part power location
new_filepath = os.path.join(self.expected_dir, filename)
older_filepath = utils.replace_partition_in_path(
self.devices, new_filepath, part_power)
os.makedirs(os.path.dirname(older_filepath))
with open(older_filepath, 'w') as fd:
fd.write(older_filepath)
os.makedirs(self.expected_dir)
os.link(older_filepath, new_filepath)
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read()) # sanity check
return older_filepath, new_filepath
def _save_ring(self, policies=POLICIES):
self.rb._ring = None
rd = self.rb.get_ring()
@ -288,6 +303,7 @@ class TestRelinker(unittest.TestCase):
'log_level': 'DEBUG',
'policies': POLICIES,
'partitions': set(),
'link_check_limit': 2,
}
mock_relink.assert_called_once_with(exp_conf, mock.ANY, device='sdx')
logger = mock_relink.call_args[0][1]
@ -313,6 +329,7 @@ class TestRelinker(unittest.TestCase):
log_level = WARNING
log_name = test-relinker
files_per_second = 11.1
link_check_limit = 1
"""
with open(conf_file, 'w') as f:
f.write(dedent(config))
@ -328,6 +345,7 @@ class TestRelinker(unittest.TestCase):
'log_level': 'WARNING',
'policies': POLICIES,
'partitions': set(),
'link_check_limit': 1,
}, mock.ANY, device='sdx')
logger = mock_relink.call_args[0][1]
self.assertEqual(logging.WARNING, logger.getEffectiveLevel())
@ -340,7 +358,9 @@ class TestRelinker(unittest.TestCase):
'--swift-dir', 'cli-dir', '--devices', 'cli-devs',
'--skip-mount-check', '--files-per-second', '2.2',
'--policy', '1', '--partition', '123',
'--partition', '123', '--partition', '456'])
'--partition', '123', '--partition', '456',
'--link-check-limit', '3',
])
mock_relink.assert_called_once_with({
'__file__': mock.ANY,
'swift_dir': 'cli-dir',
@ -351,6 +371,7 @@ class TestRelinker(unittest.TestCase):
'log_name': 'test-relinker',
'policies': {POLICIES[1]},
'partitions': {123, 456},
'link_check_limit': 3,
}, mock.ANY, device='sdx')
with mock.patch('swift.cli.relinker.relink') as mock_relink, \
@ -366,6 +387,7 @@ class TestRelinker(unittest.TestCase):
'log_level': 'INFO',
'policies': POLICIES,
'partitions': set(),
'link_check_limit': 2,
}, mock.ANY, device='sdx')
mock_logging_config.assert_called_once_with(
format='%(message)s', level=logging.INFO, filename=None)
@ -390,6 +412,7 @@ class TestRelinker(unittest.TestCase):
'log_level': 'DEBUG',
'policies': set(POLICIES),
'partitions': set(),
'link_check_limit': 2
}, mock.ANY, device='sdx')
# --debug is now effective
mock_logging_config.assert_called_once_with(
@ -456,7 +479,8 @@ class TestRelinker(unittest.TestCase):
def _do_link_test(self, command, old_file_specs, new_file_specs,
conflict_file_specs, exp_old_specs, exp_new_specs,
exp_ret_code=0, relink_errors=None):
exp_ret_code=0, relink_errors=None,
mock_relink_paths=None, extra_options=None):
# Each 'spec' is a tuple (file extension, timestamp offset); files are
# created for each old_file_specs and links are created for each in
# new_file_specs, then cleanup is run and checks made that
@ -466,12 +490,13 @@ class TestRelinker(unittest.TestCase):
# - relink_errors is a dict ext->exception; the exception will be
# raised each time relink_paths is called with a target_path ending
# with 'ext'
self.assertFalse(relink_errors and mock_relink_paths) # sanity check
new_file_specs = [] if new_file_specs is None else new_file_specs
conflict_file_specs = ([] if conflict_file_specs is None
else conflict_file_specs)
exp_old_specs = [] if exp_old_specs is None else exp_old_specs
relink_errors = {} if relink_errors is None else relink_errors
extra_options = extra_options if extra_options else []
# remove the file created by setUp - we'll create it again if wanted
os.unlink(self.objname)
@ -491,7 +516,7 @@ class TestRelinker(unittest.TestCase):
for filename in old_filenames:
filepath = os.path.join(self.objdir, filename)
with open(filepath, 'w') as fd:
fd.write(filename)
fd.write(filepath)
for filename in new_filenames:
new_filepath = os.path.join(self.expected_dir, filename)
if filename in old_filenames:
@ -499,22 +524,24 @@ class TestRelinker(unittest.TestCase):
os.link(filepath, new_filepath)
else:
with open(new_filepath, 'w') as fd:
fd.write(filename)
fd.write(new_filepath)
for filename in conflict_filenames:
new_filepath = os.path.join(self.expected_dir, filename)
with open(new_filepath, 'w') as fd:
fd.write(filename)
fd.write(new_filepath)
orig_relink_paths = relink_paths
def mock_relink_paths(target_path, new_target_path):
def default_mock_relink_paths(target_path, new_target_path, **kwargs):
for ext, error in relink_errors.items():
if target_path.endswith(ext):
raise error
return orig_relink_paths(target_path, new_target_path)
return orig_relink_paths(target_path, new_target_path,
**kwargs)
with mock.patch('swift.cli.relinker.diskfile.relink_paths',
mock_relink_paths):
mock_relink_paths if mock_relink_paths
else default_mock_relink_paths):
with mock.patch.object(relinker.logging, 'getLogger',
return_value=self.logger):
self.assertEqual(exp_ret_code, relinker.main([
@ -522,7 +549,7 @@ class TestRelinker(unittest.TestCase):
'--swift-dir', self.testdir,
'--devices', self.devices,
'--skip-mount',
]), [self.logger.all_log_lines()])
] + extra_options), [self.logger.all_log_lines()])
if exp_new_specs:
self.assertTrue(os.path.isdir(self.expected_dir))
@ -701,6 +728,18 @@ class TestRelinker(unittest.TestCase):
self.assertIn('1 hash dirs processed (cleanup=False) '
'(0 files, 0 linked, 0 removed, 0 errors)', info_lines)
def test_relink_conflicting_ts_file(self):
self._cleanup_test((('ts', 0),),
None,
(('ts', 0),), # different inode
(('ts', 0),),
(('ts', 0),),
exp_ret_code=1)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn('1 hash dirs processed (cleanup=True) '
'(1 files, 0 linked, 0 removed, 1 errors)',
info_lines)
def test_relink_link_already_exists_but_different_inode(self):
self.rb.prepare_increase_partition_power()
self._save_ring()
@ -734,11 +773,12 @@ class TestRelinker(unittest.TestCase):
self._save_ring()
orig_relink_paths = relink_paths
def mock_relink_paths(target_path, new_target_path):
def mock_relink_paths(target_path, new_target_path, **kwargs):
# pretend another process has created the link before this one
os.makedirs(self.expected_dir)
os.link(target_path, new_target_path)
return orig_relink_paths(target_path, new_target_path)
return orig_relink_paths(target_path, new_target_path,
**kwargs)
with mock.patch.object(relinker.logging, 'getLogger',
return_value=self.logger):
@ -769,10 +809,11 @@ class TestRelinker(unittest.TestCase):
self._save_ring()
orig_relink_paths = relink_paths
def mock_relink_paths(target_path, new_target_path):
def mock_relink_paths(target_path, new_target_path, **kwargs):
# pretend another process has cleaned up the target path
os.unlink(target_path)
return orig_relink_paths(target_path, new_target_path)
return orig_relink_paths(target_path, new_target_path,
**kwargs)
with mock.patch.object(relinker.logging, 'getLogger',
return_value=self.logger):
@ -1159,6 +1200,359 @@ class TestRelinker(unittest.TestCase):
self.assertEqual([], mocked.call_args_list)
self.assertEqual(2, res)
def test_relink_conflicting_ts_is_linked_to_part_power(self):
# link from next partition to current partition;
# different file in current-1 partition
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
filename = '.'.join([self.obj_ts.internal, 'ts'])
new_filepath = os.path.join(self.expected_dir, filename)
old_filepath = os.path.join(self.objdir, filename)
# setup a file in the current-1 part power (PART_POWER - 1) location
# that is *not* linked to the file in the next part power location;
# this file should be removed during relink.
older_filepath = utils.replace_partition_in_path(
self.devices, new_filepath, PART_POWER - 1)
os.makedirs(os.path.dirname(older_filepath))
with open(older_filepath, 'w') as fd:
fd.write(older_filepath)
self._do_link_test('relink',
(('ts', 0),),
(('ts', 0),),
None,
(('ts', 0),),
(('ts', 0),),
exp_ret_code=0)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn(
'Relinking: cleaning up unwanted file: %s' % older_filepath,
info_lines)
# both the PART_POWER and PART_POWER - N partitions are visited, no new
# links are created, and both the older files are removed
self.assertIn('2 hash dirs processed (cleanup=False) '
'(2 files, 0 linked, 1 removed, 0 errors)',
info_lines)
print(info_lines)
with open(new_filepath, 'r') as fd:
self.assertEqual(old_filepath, fd.read())
self.assertFalse(os.path.exists(older_filepath))
def test_relink_conflicting_ts_is_linked_to_part_power_minus_1(self):
# link from next partition to current-1 partition;
# different file in current partition
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older (PART_POWER - 1) location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 1)
self._do_link_test('relink',
(('ts', 0),),
None,
None, # we already made file linked to older part
(('ts', 0),), # retained
(('ts', 0),),
exp_ret_code=0)
info_lines = self.logger.get_lines_for_level('info')
# both the PART_POWER and PART_POWER - N partitions are visited, no new
# links are created, and both the older files are retained
self.assertIn('2 hash dirs processed (cleanup=False) '
'(2 files, 0 linked, 0 removed, 0 errors)',
info_lines)
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read())
# prev part power file is retained because it is link target
self.assertTrue(os.path.exists(older_filepath))
def test_relink_conflicting_ts_link_to_part_power_minus_1_disappears(self):
# uncommon case: existing link from next partition to current-1
# partition disappears between an EEXIST while attempting to link
# different file in current partition and checking for an ok link
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older (PART_POWER - 1) location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 1)
old_filepath = os.path.join(self.objdir, filename)
orig_relink_paths = relink_paths
calls = []
def mock_relink_paths(target_path, new_target_path, **kwargs):
# while retrying link from current part power to next part power
# location, unlink the conflicting file in next part power location
calls.append((target_path, kwargs))
if len(calls) == 2:
os.unlink(os.path.join(self.expected_dir, filename))
return orig_relink_paths(target_path, new_target_path,
**kwargs)
with mock.patch('swift.cli.relinker.diskfile.relink_paths',
mock_relink_paths):
self._do_link_test('relink',
(('ts', 0),),
None,
None,
(('ts', 0),), # retained
(('ts', 0),),
exp_ret_code=0,
mock_relink_paths=mock_relink_paths)
self.assertEqual(
[(old_filepath, {}), # link attempted - EEXIST raised
(old_filepath, {'ignore_missing': False}), # check makes link!
(older_filepath, {}), # link attempted - EEXIST raised
(old_filepath, {'ignore_missing': False})], # check finds ok link
calls)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn(
'Relinking: cleaning up unwanted file: %s' % older_filepath,
info_lines)
self.assertIn('2 hash dirs processed (cleanup=False) '
'(2 files, 1 linked, 1 removed, 0 errors)',
info_lines)
self.assertTrue(os.path.exists(old_filepath))
with open(new_filepath, 'r') as fd:
self.assertEqual(old_filepath, fd.read())
# older file is not needed
self.assertFalse(os.path.exists(older_filepath))
def test_relink_conflicting_ts_link_to_part_power_disappears(self):
# uncommon case: existing link from next partition to current
# partition disappears between an EEXIST while attempting to link
# different file in current-1 partition and checking for an ok link
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
filename = '.'.join([self.obj_ts.internal, 'ts'])
new_filepath = os.path.join(self.expected_dir, filename)
old_filepath = utils.replace_partition_in_path(
self.devices, new_filepath, PART_POWER)
older_filepath = utils.replace_partition_in_path(
self.devices, new_filepath, PART_POWER - 1)
os.makedirs(os.path.dirname(older_filepath))
with open(older_filepath, 'w') as fd:
fd.write(older_filepath)
orig_relink_paths = relink_paths
calls = []
def mock_relink_paths(target_path, new_target_path, **kwargs):
# while retrying link from current-1 part power to next part power
# location, unlink the conflicting file in next part power location
calls.append((target_path, kwargs))
if len(calls) == 3:
os.unlink(os.path.join(self.expected_dir, filename))
return orig_relink_paths(target_path, new_target_path,
**kwargs)
with mock.patch('swift.cli.relinker.diskfile.relink_paths',
mock_relink_paths):
self._do_link_test('relink',
(('ts', 0),),
(('ts', 0),),
None,
(('ts', 0),), # retained
(('ts', 0),),
exp_ret_code=0,
mock_relink_paths=mock_relink_paths)
self.assertEqual(
[(old_filepath, {}), # link attempted but exists
(older_filepath, {}), # link attempted - EEXIST raised
(old_filepath, {'ignore_missing': False})], # check makes link!
calls)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn('2 hash dirs processed (cleanup=False) '
'(2 files, 1 linked, 1 removed, 0 errors)',
info_lines)
self.assertTrue(os.path.exists(old_filepath))
with open(new_filepath, 'r') as fd:
self.assertEqual(old_filepath, fd.read())
self.assertFalse(os.path.exists(older_filepath))
def test_relink_conflicting_ts_is_linked_to_part_power_minus_2_err(self):
# link from next partition to current-2 partition;
# different file in current partition
# by default the relinker will NOT validate the current-2 location
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older (PART_POWER - 2) location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 2)
self._do_link_test('relink',
(('ts', 0),),
None,
None, # we already made file linked to older part
(('ts', 0),), # retained
(('ts', 0),),
exp_ret_code=1)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn('2 hash dirs processed (cleanup=False) '
'(2 files, 0 linked, 0 removed, 1 errors)',
info_lines)
warning_lines = self.logger.get_lines_for_level('warning')
self.assertIn('Error relinking: failed to relink', warning_lines[0])
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read())
# prev-1 part power file is always retained because it is link target
self.assertTrue(os.path.exists(older_filepath))
def test_relink_conflicting_ts_is_linked_to_part_power_minus_2_ok(self):
# link from next partition to current-2 partition;
# different file in current partition
# increase link_check_limit so that the relinker WILL validate the
# current-2 location
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older (PART_POWER - 2) location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 2)
self._do_link_test('relink',
(('ts', 0),),
None,
None, # we already made file linked to older part
(('ts', 0),), # retained
(('ts', 0),),
exp_ret_code=0,
extra_options=['--link-check-limit', '3'])
info_lines = self.logger.get_lines_for_level('info')
self.assertIn('2 hash dirs processed (cleanup=False) '
'(2 files, 0 linked, 0 removed, 0 errors)',
info_lines)
warning_lines = self.logger.get_lines_for_level('warning')
self.assertEqual([], warning_lines)
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read())
# prev-1 part power file is retained because it is link target
self.assertTrue(os.path.exists(older_filepath))
def test_relink_conflicting_ts_both_in_older_part_powers(self):
# link from next partition to current-1 partition;
# different file in current partition
# different file in current-2 location
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 2))
self.rb.prepare_increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older (PART_POWER - 1) location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 1)
# setup a file in an even older part power (PART_POWER - 2) location
# that is *not* linked to the file in the next part power location;
# this file should be removed during relink.
oldest_filepath = utils.replace_partition_in_path(
self.devices, new_filepath, PART_POWER - 2)
os.makedirs(os.path.dirname(oldest_filepath))
with open(oldest_filepath, 'w') as fd:
fd.write(oldest_filepath)
self._do_link_test('relink',
(('ts', 0),),
None,
None, # we already made file linked to older part
(('ts', 0),), # retained
(('ts', 0),),
exp_ret_code=0)
info_lines = self.logger.get_lines_for_level('info')
# both the PART_POWER and PART_POWER - N partitions are visited, no new
# links are created, and both the older files are retained
self.assertIn('3 hash dirs processed (cleanup=False) '
'(3 files, 0 linked, 1 removed, 0 errors)',
info_lines)
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read())
self.assertTrue(os.path.exists(older_filepath)) # linked so retained
self.assertFalse(os.path.exists(oldest_filepath))
def test_relink_conflicting_ts_is_not_linked_link_check_limit_zero(self):
# different file in next part power partition, so all attempts to link
# fail, check no rechecks made with zero link-check-limit
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
filename = '.'.join([self.obj_ts.internal, 'ts'])
current_filepath = os.path.join(self.objdir, filename)
orig_relink_paths = relink_paths
calls = []
def mock_relink_paths(target_path, new_target_path, **kwargs):
calls.append((target_path, kwargs))
return orig_relink_paths(target_path, new_target_path,
**kwargs)
self._do_link_test(
'relink',
(('ts', 0),),
None,
(('ts', 0),),
(('ts', 0),),
(('ts', 0),),
exp_ret_code=1,
extra_options=['--link-check-limit', '0'],
mock_relink_paths=mock_relink_paths,
)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn('1 hash dirs processed (cleanup=False) '
'(1 files, 0 linked, 0 removed, 1 errors)',
info_lines)
# one attempt to link from current plus a check/retry from each
# possible partition power, stopping at part power 1
expected = [(current_filepath, {})]
self.assertEqual(expected, calls)
def test_relink_conflicting_ts_is_not_linked_link_check_limit_absurd(self):
# different file in next part power partition, so all attempts to link
# fail, check that absurd link-check-limit gets capped
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self._save_ring()
filename = '.'.join([self.obj_ts.internal, 'ts'])
current_filepath = os.path.join(self.objdir, filename)
orig_relink_paths = relink_paths
calls = []
def mock_relink_paths(target_path, new_target_path, **kwargs):
calls.append((target_path, kwargs))
return orig_relink_paths(target_path, new_target_path,
**kwargs)
self._do_link_test(
'relink',
(('ts', 0),),
None,
(('ts', 0),),
(('ts', 0),),
(('ts', 0),),
exp_ret_code=1,
extra_options=['--link-check-limit', str(PART_POWER + 99)],
mock_relink_paths=mock_relink_paths,
)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn('1 hash dirs processed (cleanup=False) '
'(1 files, 0 linked, 0 removed, 1 errors)',
info_lines)
# one attempt to link from current plus a check/retry from each
# possible partition power, stopping at part power 1
expected = [(current_filepath, {})] + [
(utils.replace_partition_in_path(
self.devices, current_filepath, part_power),
{'ignore_missing': False})
for part_power in range(PART_POWER, -1, -1)]
self.assertEqual(expected, calls)
@patch_policies(
[StoragePolicy(0, name='gold', is_default=True),
ECStoragePolicy(1, name='platinum', ec_type=DEFAULT_TEST_EC_TYPE,
@ -1518,6 +1912,100 @@ class TestRelinker(unittest.TestCase):
'(1 files, 0 linked, 0 removed, 1 errors)',
info_lines)
def test_cleanup_conflicting_ts_is_linked_to_part_power_minus_1(self):
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self.rb.increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older PART_POWER - 1 location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 1)
self._do_link_test('cleanup',
(('ts', 0),),
None,
None, # we already made file linked to older part
None,
(('ts', 0),),
exp_ret_code=0)
info_lines = self.logger.get_lines_for_level('info')
# both the PART_POWER and PART_POWER - N partitions are visited, no new
# links are created, and both the older files are removed
self.assertIn('2 hash dirs processed (cleanup=True) '
'(2 files, 0 linked, 2 removed, 0 errors)',
info_lines)
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read())
self.assertFalse(os.path.exists(older_filepath))
def test_cleanup_conflicting_ts_is_linked_to_part_power_minus_2_err(self):
# link from next partition to current-2 partition;
# different file in current partition
# by default the relinker will NOT validate the current-2 location
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self.rb.increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older (PART_POWER - 2) location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 2)
self._do_link_test('cleanup',
(('ts', 0),),
None,
None, # we already made file linked to older part
(('ts', 0),), # retained
(('ts', 0),),
exp_ret_code=1)
info_lines = self.logger.get_lines_for_level('info')
self.assertIn('2 hash dirs processed (cleanup=True) '
'(2 files, 0 linked, 1 removed, 1 errors)',
info_lines)
warning_lines = self.logger.get_lines_for_level('warning')
self.assertIn('Error relinking (cleanup): failed to relink',
warning_lines[0])
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read())
# current-2 is linked so can be removed in cleanup
self.assertFalse(os.path.exists(older_filepath))
def test_cleanup_conflicting_ts_is_linked_to_part_power_minus_2_ok(self):
# link from next partition to current-2 partition;
# different file in current partition
# increase link_check_limit so that the relinker WILL validate the
# current-2 location
self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1))
self.rb.prepare_increase_partition_power()
self.rb.increase_partition_power()
self._save_ring()
# setup a file in the next part power (PART_POWER + 1) location that is
# linked to a file in an older (PART_POWER - 2) location
filename = '.'.join([self.obj_ts.internal, 'ts'])
older_filepath, new_filepath = self._make_link(filename,
PART_POWER - 2)
self._do_link_test('cleanup',
(('ts', 0),),
None,
None, # we already made file linked to older part
None,
(('ts', 0),),
exp_ret_code=0,
extra_options=['--link-check-limit', '3'])
info_lines = self.logger.get_lines_for_level('info')
# both the PART_POWER and PART_POWER - N partitions are visited, no new
# links are created, and both the older files are removed
self.assertIn('2 hash dirs processed (cleanup=True) '
'(2 files, 0 linked, 2 removed, 0 errors)',
info_lines)
warning_lines = self.logger.get_lines_for_level('warning')
self.assertEqual([], warning_lines)
with open(new_filepath, 'r') as fd:
self.assertEqual(older_filepath, fd.read())
self.assertFalse(os.path.exists(older_filepath))
def test_cleanup_conflicting_older_data_file(self):
# older conflicting file isn't relevant so cleanup succeeds
self._cleanup_test((('data', 0),),

2
test/unit/common/test_utils.py
View File

@ -4439,6 +4439,8 @@ cluster_dfw1 = http://dfw1.host/v1/
self.assertEqual(350, utils.get_partition_for_hash(hex_hash, 9))
self.assertEqual(700, utils.get_partition_for_hash(hex_hash, 10))
self.assertEqual(1400, utils.get_partition_for_hash(hex_hash, 11))
self.assertEqual(0, utils.get_partition_for_hash(hex_hash, 0))
self.assertEqual(0, utils.get_partition_for_hash(hex_hash, -1))
def test_replace_partition_in_path(self):
# Check for new part = part * 2

12
test/unit/obj/test_diskfile.py
View File

@ -223,7 +223,11 @@ class TestDiskFileModuleMethods(unittest.TestCase):
side_effect=Exception('oops')):
with self.assertRaises(Exception) as cm:
diskfile.relink_paths(target_path, new_target_path)
self.assertEqual('oops', str(cm.exception))
self.assertEqual('oops', str(cm.exception))
with self.assertRaises(Exception) as cm:
diskfile.relink_paths(target_path, new_target_path,
ignore_missing=False)
self.assertEqual('oops', str(cm.exception))
def test_relink_paths_makedirs_race(self):
# test two concurrent relinks of the same object hash dir with race
@ -313,6 +317,12 @@ class TestDiskFileModuleMethods(unittest.TestCase):
self.assertFalse(os.path.exists(target_path))
self.assertFalse(os.path.exists(new_target_path))
self.assertFalse(created)
with self.assertRaises(OSError) as cm:
diskfile.relink_paths(target_path, new_target_path,
ignore_missing=False)
self.assertEqual(errno.ENOENT, cm.exception.errno)
self.assertFalse(os.path.exists(target_path))
self.assertFalse(os.path.exists(new_target_path))
def test_relink_paths_os_link_race(self):
# test two concurrent relinks of the same object hash dir with race