openstack/swift
Code Issues Proposed changes

Merge "expirer: Only try to delete empty containers"

Browse Source
This commit is contained in:
Zuul 2022-02-26 08:28:38 +00:00 committed by Gerrit Code Review
commit 7fc522743b
2 changed files with 104 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
swift/obj/expirer.py
View File

@ -265,7 +265,9 @@ class ObjectExpirer(Daemon):
task_container, task_object, timestamp_to_delete, and target_path task_container, task_object, timestamp_to_delete, and target_path
""" """
for task_account, task_container in task_account_container_list: for task_account, task_container in task_account_container_list:
container_empty = True
for o in self.swift.iter_objects(task_account, task_container): for o in self.swift.iter_objects(task_account, task_container):
container_empty = False
if six.PY2: if six.PY2:
task_object = o['name'].encode('utf8') task_object = o['name'].encode('utf8')
else: else:
@ -295,6 +297,17 @@ class ObjectExpirer(Daemon):
target_account, target_container, target_object]), target_account, target_container, target_object]),
'delete_timestamp': delete_timestamp, 'delete_timestamp': delete_timestamp,
'is_async_delete': is_async} 'is_async_delete': is_async}
if container_empty:
try:
self.swift.delete_container(
task_account, task_container,
acceptable_statuses=(2, HTTP_NOT_FOUND, HTTP_CONFLICT))
except (Exception, Timeout) as err:
self.logger.exception(
'Exception while deleting container %(account)s '
'%(container)s %(err)s', {
'account': task_account,
'container': task_container, 'err': str(err)})
def run_once(self, *args, **kwargs): def run_once(self, *args, **kwargs):
""" """
@ -323,7 +336,6 @@ class ObjectExpirer(Daemon):
self.report_objects = 0 self.report_objects = 0
try: try:
self.logger.debug('Run begin') self.logger.debug('Run begin')
task_account_container_list_to_delete = list()
for task_account, my_index, divisor in \ for task_account, my_index, divisor in \
self.iter_task_accounts_to_expire(): self.iter_task_accounts_to_expire():
container_count, obj_count = \ container_count, obj_count = \
@ -345,9 +357,6 @@ class ObjectExpirer(Daemon):
[(task_account, task_container) for task_container in [(task_account, task_container) for task_container in
self.iter_task_containers_to_expire(task_account)] self.iter_task_containers_to_expire(task_account)]
task_account_container_list_to_delete.extend(
task_account_container_list)
# delete_task_iter is a generator to yield a dict of # delete_task_iter is a generator to yield a dict of
# task_account, task_container, task_object, delete_timestamp, # task_account, task_container, task_object, delete_timestamp,
# target_path to handle delete actual object and pop the task # target_path to handle delete actual object and pop the task
@ -362,18 +371,6 @@ class ObjectExpirer(Daemon):
pool.spawn_n(self.delete_object, **delete_task) pool.spawn_n(self.delete_object, **delete_task)
pool.waitall() pool.waitall()
for task_account, task_container in \
task_account_container_list_to_delete:
try:
self.swift.delete_container(
task_account, task_container,
acceptable_statuses=(2, HTTP_NOT_FOUND, HTTP_CONFLICT))
except (Exception, Timeout) as err:
self.logger.exception(
'Exception while deleting container %(account)s '
'%(container)s %(err)s', {
'account': task_account,
'container': task_container, 'err': str(err)})
self.logger.debug('Run end') self.logger.debug('Run end')
self.report(final=True) self.report(final=True)
except (Exception, Timeout): except (Exception, Timeout):

109
test/unit/obj/test_expirer.py
View File

@ -110,26 +110,31 @@ class TestObjectExpirer(TestCase):
self.logger = debug_logger('test-expirer') self.logger = debug_logger('test-expirer')
self.ts = make_timestamp_iter() self.ts = make_timestamp_iter()
self.empty_time = str(int(time() - 864000))
self.past_time = str(int(time() - 86400)) self.past_time = str(int(time() - 86400))
self.just_past_time = str(int(time() - 1))
self.future_time = str(int(time() + 86400)) self.future_time = str(int(time() + 86400))
# Dummy task queue for test # Dummy task queue for test
self.fake_swift = FakeInternalClient({ self.fake_swift = FakeInternalClient({
'.expiring_objects': { '.expiring_objects': {
# this task container will be checked # this task container will be checked
self.empty_time: [],
self.past_time: [ self.past_time: [
# tasks ready for execution # tasks ready for execution
self.past_time + '-a0/c0/o0', self.past_time + '-a0/c0/o0',
self.past_time + '-a1/c1/o1', self.past_time + '-a1/c1/o1',
self.past_time + '-a2/c2/o2', self.past_time + '-a2/c2/o2',
self.past_time + '-a3/c3/o3', self.past_time + '-a3/c3/o3',
self.past_time + '-a4/c4/o4', self.past_time + '-a4/c4/o4'],
self.past_time + '-a5/c5/o5', self.just_past_time: [
self.past_time + '-a6/c6/o6', self.just_past_time + '-a5/c5/o5',
self.past_time + '-a7/c7/o7', self.just_past_time + '-a6/c6/o6',
self.just_past_time + '-a7/c7/o7',
# task objects for unicode test # task objects for unicode test
self.past_time + u'-a8/c8/o8\u2661', self.just_past_time + u'-a8/c8/o8\u2661',
self.past_time + u'-a9/c9/o9\xf8', self.just_past_time + u'-a9/c9/o9\xf8',
# this task will be skipped # this task will be skipped and prevent us from even
# *trying* to delete the container
self.future_time + '-a10/c10/o10'], self.future_time + '-a10/c10/o10'],
# this task container will be skipped # this task container will be skipped
self.future_time: [ self.future_time: [
@ -138,14 +143,20 @@ class TestObjectExpirer(TestCase):
self.expirer = expirer.ObjectExpirer(self.conf, logger=self.logger, self.expirer = expirer.ObjectExpirer(self.conf, logger=self.logger,
swift=self.fake_swift) swift=self.fake_swift)
# target object paths which should be expirerd now # map of times to target object paths which should be expirerd now
self.expired_target_path_list = [ self.expired_target_paths = {
self.past_time: [
swob.wsgi_to_str(tgt) for tgt in ( swob.wsgi_to_str(tgt) for tgt in (
'a0/c0/o0', 'a1/c1/o1', 'a2/c2/o2', 'a3/c3/o3', 'a4/c4/o4', 'a0/c0/o0', 'a1/c1/o1', 'a2/c2/o2', 'a3/c3/o3', 'a4/c4/o4',
)
],
self.just_past_time: [
swob.wsgi_to_str(tgt) for tgt in (
'a5/c5/o5', 'a6/c6/o6', 'a7/c7/o7', 'a5/c5/o5', 'a6/c6/o6', 'a7/c7/o7',
'a8/c8/o8\xe2\x99\xa1', 'a9/c9/o9\xc3\xb8', 'a8/c8/o8\xe2\x99\xa1', 'a9/c9/o9\xc3\xb8',
) )
] ],
}
def make_fake_ic(self, app): def make_fake_ic(self, app):
app._pipeline_final_app = mock.MagicMock() app._pipeline_final_app = mock.MagicMock()
@ -329,7 +340,11 @@ class TestObjectExpirer(TestCase):
expected = { expected = {
self.past_time: [ self.past_time: [
self.past_time + '-' + target_path self.past_time + '-' + target_path
for target_path in self.expired_target_path_list]} for target_path in self.expired_target_paths[self.past_time]],
self.just_past_time: [
self.just_past_time + '-' + target_path
for target_path
in self.expired_target_paths[self.just_past_time]]}
self.assertEqual(deleted_objects, expected) self.assertEqual(deleted_objects, expected)
def test_delete_object(self): def test_delete_object(self):
@ -605,7 +620,7 @@ class TestObjectExpirer(TestCase):
self.assertEqual( self.assertEqual(
self.expirer.logger.get_lines_for_level('info'), [ self.expirer.logger.get_lines_for_level('info'), [
'Pass beginning for task account .expiring_objects; ' 'Pass beginning for task account .expiring_objects; '
'2 possible containers; 12 possible objects', '4 possible containers; 12 possible objects',
'Pass completed in 0s; 10 objects expired', 'Pass completed in 0s; 10 objects expired',
]) ])
@ -628,7 +643,10 @@ class TestObjectExpirer(TestCase):
x.run_once() x.run_once()
self.assertEqual(calls, [([ self.assertEqual(calls, [([
self.make_task(self.past_time, target_path) self.make_task(self.past_time, target_path)
for target_path in self.expired_target_path_list for target_path in self.expired_target_paths[self.past_time]
] + [
self.make_task(self.just_past_time, target_path)
for target_path in self.expired_target_paths[self.just_past_time]
], 2)]) ], 2)])
def test_skip_task_account_without_task_container(self): def test_skip_task_account_without_task_container(self):
@ -649,16 +667,32 @@ class TestObjectExpirer(TestCase):
my_index = 0 my_index = 0
divisor = 1 divisor = 1
# empty container gets deleted inline
task_account_container_list = [('.expiring_objects', self.empty_time)]
with mock.patch.object(self.expirer.swift, 'delete_container') \
as mock_delete_container:
self.assertEqual(
list(self.expirer.iter_task_to_expire(
task_account_container_list, my_index, divisor)),
[])
self.assertEqual(mock_delete_container.mock_calls, [
mock.call('.expiring_objects', self.empty_time,
acceptable_statuses=(2, 404, 409))])
task_account_container_list = [('.expiring_objects', self.past_time)] task_account_container_list = [('.expiring_objects', self.past_time)]
expected = [ expected = [
self.make_task(self.past_time, target_path) self.make_task(self.past_time, target_path)
for target_path in self.expired_target_path_list] for target_path in self.expired_target_paths[self.past_time]]
with mock.patch.object(self.expirer.swift, 'delete_container') \
as mock_delete_container:
self.assertEqual( self.assertEqual(
list(self.expirer.iter_task_to_expire( list(self.expirer.iter_task_to_expire(
task_account_container_list, my_index, divisor)), task_account_container_list, my_index, divisor)),
expected) expected)
# not empty; not deleted
self.assertEqual(mock_delete_container.mock_calls, [])
# the task queue has invalid task object # the task queue has invalid task object
invalid_aco_dict = deepcopy(self.fake_swift.aco_dict) invalid_aco_dict = deepcopy(self.fake_swift.aco_dict)
@ -713,7 +747,9 @@ class TestObjectExpirer(TestCase):
expected = [ expected = [
self.make_task(self.past_time, target_path, self.make_task(self.past_time, target_path,
is_async_delete=True) is_async_delete=True)
for target_path in self.expired_target_path_list] for target_path in (
self.expired_target_paths[self.past_time] +
self.expired_target_paths[self.just_past_time])]
self.assertEqual( self.assertEqual(
list(x.iter_task_to_expire( list(x.iter_task_to_expire(
@ -737,8 +773,10 @@ class TestObjectExpirer(TestCase):
self.expirer.run_once() self.expirer.run_once()
# iter_objects is called only for past_time, not future_time # iter_objects is called only for past_time, not future_time
self.assertEqual(mock_method.call_args_list, self.assertEqual(mock_method.call_args_list, [
[mock.call('.expiring_objects', self.past_time)]) mock.call('.expiring_objects', self.empty_time),
mock.call('.expiring_objects', self.past_time),
mock.call('.expiring_objects', self.just_past_time)])
def test_object_timestamp_break(self): def test_object_timestamp_break(self):
with mock.patch.object(self.expirer, 'delete_actual_object') \ with mock.patch.object(self.expirer, 'delete_actual_object') \
@ -750,7 +788,10 @@ class TestObjectExpirer(TestCase):
self.assertEqual( self.assertEqual(
mock_method.call_args_list, mock_method.call_args_list,
[mock.call(target_path, self.past_time, False) [mock.call(target_path, self.past_time, False)
for target_path in self.expired_target_path_list]) for target_path in self.expired_target_paths[self.past_time]] +
[mock.call(target_path, self.just_past_time, False)
for target_path
in self.expired_target_paths[self.just_past_time]])
def test_failed_delete_keeps_entry(self): def test_failed_delete_keeps_entry(self):
def deliberately_blow_up(actual_obj, timestamp): def deliberately_blow_up(actual_obj, timestamp):
@ -776,7 +817,11 @@ class TestObjectExpirer(TestCase):
mock_method.call_args_list, mock_method.call_args_list,
[mock.call('.expiring_objects', self.past_time, [mock.call('.expiring_objects', self.past_time,
self.past_time + '-' + target_path) self.past_time + '-' + target_path)
for target_path in self.expired_target_path_list]) for target_path in self.expired_target_paths[self.past_time]] +
[mock.call('.expiring_objects', self.just_past_time,
self.just_past_time + '-' + target_path)
for target_path
in self.expired_target_paths[self.just_past_time]])
def test_success_gets_counted(self): def test_success_gets_counted(self):
self.assertEqual(self.expirer.report_objects, 0) self.assertEqual(self.expirer.report_objects, 0)
@ -812,29 +857,37 @@ class TestObjectExpirer(TestCase):
raise Exception('failed to delete container') raise Exception('failed to delete container')
def fail_delete_actual_object(actual_obj, timestamp, is_async_delete): def fail_delete_actual_object(actual_obj, timestamp, is_async_delete):
if timestamp == self.just_past_time:
raise Exception('failed to delete actual object') raise Exception('failed to delete actual object')
with mock.patch.object(self.fake_swift, 'delete_container', with mock.patch.object(self.fake_swift, 'delete_container',
fail_delete_container), \ fail_delete_container), \
mock.patch.object(self.expirer, 'delete_actual_object', mock.patch.object(self.expirer, 'delete_actual_object',
fail_delete_actual_object): fail_delete_actual_object), \
mock.patch.object(self.expirer, 'pop_queue') as mock_pop:
self.expirer.run_once() self.expirer.run_once()
error_lines = self.expirer.logger.get_lines_for_level('error') error_lines = self.expirer.logger.get_lines_for_level('error')
self.assertEqual(error_lines, [ self.assertEqual(error_lines, [
'Exception while deleting container .expiring_objects %s failed '
'to delete container: ' % self.empty_time
] + [
'Exception while deleting object %s %s %s ' 'Exception while deleting object %s %s %s '
'failed to delete actual object: ' % ( 'failed to delete actual object: ' % (
'.expiring_objects', self.past_time, '.expiring_objects', self.just_past_time,
self.past_time + '-' + target_path) self.just_past_time + '-' + target_path)
for target_path in self.expired_target_path_list] + [ for target_path in self.expired_target_paths[self.just_past_time]
'Exception while deleting container %s %s ' ])
'failed to delete container: ' % (
'.expiring_objects', self.past_time)])
self.assertEqual(self.expirer.logger.get_lines_for_level('info'), [ self.assertEqual(self.expirer.logger.get_lines_for_level('info'), [
'Pass beginning for task account .expiring_objects; ' 'Pass beginning for task account .expiring_objects; '
'2 possible containers; 12 possible objects', '4 possible containers; 12 possible objects',
'Pass completed in 0s; 0 objects expired', 'Pass completed in 0s; 5 objects expired',
])
self.assertEqual(mock_pop.mock_calls, [
mock.call('.expiring_objects', self.past_time,
self.past_time + '-' + target_path)
for target_path in self.expired_target_paths[self.past_time]
]) ])
def test_run_forever_initial_sleep_random(self): def test_run_forever_initial_sleep_random(self):