From baf18edc00851f6749a40794587ca14a52135bf3 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 18 Oct 2018 10:35:31 -0700 Subject: [PATCH] Clean up account-reaper a bit - Drop the (partial) logging translation - Save our log concatenations until the end - Stop encoding object names; direct_client is happy to take Unicode - Remove a couple loop breaks that were only used by tests Change-Id: I4a4f301a7a6cb0f217ca0bf8712ee0291bbc14a3 Partial-Bug: #1674543 --- swift/account/reaper.py | 90 ++++++++++++++------------------ test/unit/account/test_reaper.py | 21 ++++---- 2 files changed, 52 insertions(+), 59 deletions(-) diff --git a/swift/account/reaper.py b/swift/account/reaper.py index 86d6c83f51..24b876073f 100644 --- a/swift/account/reaper.py +++ b/swift/account/reaper.py @@ -16,7 +16,6 @@ import os import random import socket -from swift import gettext_ as _ from logging import DEBUG from math import sqrt from time import time @@ -142,10 +141,10 @@ class AccountReaper(Daemon): continue self.reap_device(device) except (Exception, Timeout): - self.logger.exception(_("Exception in top-level account reaper " - "loop")) + self.logger.exception("Exception in top-level account reaper " + "loop") elapsed = time() - begin - self.logger.info(_('Devices pass completed: %.02fs'), elapsed) + self.logger.info('Devices pass completed: %.02fs', elapsed) def reap_device(self, device): """ @@ -255,19 +254,15 @@ class AccountReaper(Daemon): self.delay_reaping: return False account = info['account'] - self.logger.info(_('Beginning pass on account %s'), account) + self.logger.info('Beginning pass on account %s', account) self.reset_stats() container_limit = 1000 if container_shard is not None: container_limit *= len(nodes) try: - marker = '' - while True: - containers = \ - list(broker.list_containers_iter(container_limit, marker, - None, None, None)) - if not containers: - break + containers = list(broker.list_containers_iter( + container_limit, '', None, None, None)) + while containers: try: for (container, _junk, _junk, _junk, _junk) in containers: if six.PY3: @@ -284,43 +279,44 @@ class AccountReaper(Daemon): self.container_pool.waitall() except (Exception, Timeout): self.logger.exception( - _('Exception with containers for account %s'), account) - marker = containers[-1][0] - if marker == '': - break - log = 'Completed pass on account %s' % account + 'Exception with containers for account %s', account) + containers = list(broker.list_containers_iter( + container_limit, containers[-1][0], None, None, None)) + log_buf = ['Completed pass on account %s' % account] except (Exception, Timeout): - self.logger.exception( - _('Exception with account %s'), account) - log = _('Incomplete pass on account %s') % account + self.logger.exception('Exception with account %s', account) + log_buf = ['Incomplete pass on account %s' % account] if self.stats_containers_deleted: - log += _(', %s containers deleted') % self.stats_containers_deleted + log_buf.append(', %s containers deleted' % + self.stats_containers_deleted) if self.stats_objects_deleted: - log += _(', %s objects deleted') % self.stats_objects_deleted + log_buf.append(', %s objects deleted' % self.stats_objects_deleted) if self.stats_containers_remaining: - log += _(', %s containers remaining') % \ - self.stats_containers_remaining + log_buf.append(', %s containers remaining' % + self.stats_containers_remaining) if self.stats_objects_remaining: - log += _(', %s objects remaining') % self.stats_objects_remaining + log_buf.append(', %s objects remaining' % + self.stats_objects_remaining) if self.stats_containers_possibly_remaining: - log += _(', %s containers possibly remaining') % \ - self.stats_containers_possibly_remaining + log_buf.append(', %s containers possibly remaining' % + self.stats_containers_possibly_remaining) if self.stats_objects_possibly_remaining: - log += _(', %s objects possibly remaining') % \ - self.stats_objects_possibly_remaining + log_buf.append(', %s objects possibly remaining' % + self.stats_objects_possibly_remaining) if self.stats_return_codes: - log += _(', return codes: ') + log_buf.append(', return codes: ') for code in sorted(self.stats_return_codes): - log += '%s %sxxs, ' % (self.stats_return_codes[code], code) - log = log[:-2] - log += _(', elapsed: %.02fs') % (time() - begin) - self.logger.info(log) + log_buf.append('%s %sxxs, ' % (self.stats_return_codes[code], + code)) + log_buf[-1] = log_buf[-1][:-2] + log_buf.append(', elapsed: %.02fs' % (time() - begin)) + self.logger.info(''.join(log_buf)) self.logger.timing_since('timing', self.start_time) delete_timestamp = Timestamp(info['delete_timestamp']) if self.stats_containers_remaining and \ begin - float(delete_timestamp) >= self.reap_not_done_after: self.logger.warning( - _('Account %(account)s has not been reaped since %(time)s') % + 'Account %(account)s has not been reaped since %(time)s' % {'account': account, 'time': delete_timestamp.isoformat}) return True @@ -379,14 +375,14 @@ class AccountReaper(Daemon): except ClientException as err: if self.logger.getEffectiveLevel() <= DEBUG: self.logger.exception( - _('Exception with %(ip)s:%(port)s/%(device)s'), node) + 'Exception with %(ip)s:%(port)s/%(device)s', node) self.stats_return_codes[err.http_status // 100] = \ self.stats_return_codes.get(err.http_status // 100, 0) + 1 self.logger.increment( 'return_codes.%d' % (err.http_status // 100,)) except (Timeout, socket.error) as err: self.logger.error( - _('Timeout Exception with %(ip)s:%(port)s/%(device)s'), + 'Timeout Exception with %(ip)s:%(port)s/%(device)s', node) if not objects: break @@ -397,21 +393,15 @@ class AccountReaper(Daemon): self.logger.error('ERROR: invalid storage policy index: %r' % policy_index) for obj in objects: - obj_name = obj['name'] - if isinstance(obj_name, six.text_type): - obj_name = obj_name.encode('utf8') pool.spawn(self.reap_object, account, container, part, - nodes, obj_name, policy_index) + nodes, obj['name'], policy_index) pool.waitall() except (Exception, Timeout): - self.logger.exception(_('Exception with objects for container ' - '%(container)s for account %(account)s' - ), + self.logger.exception('Exception with objects for container ' + '%(container)s for account %(account)s', {'container': container, 'account': account}) marker = objects[-1]['name'] - if marker == '': - break successes = 0 failures = 0 timestamp = Timestamp.now() @@ -434,7 +424,7 @@ class AccountReaper(Daemon): except ClientException as err: if self.logger.getEffectiveLevel() <= DEBUG: self.logger.exception( - _('Exception with %(ip)s:%(port)s/%(device)s'), node) + 'Exception with %(ip)s:%(port)s/%(device)s', node) failures += 1 self.logger.increment('containers_failures') self.stats_return_codes[err.http_status // 100] = \ @@ -443,7 +433,7 @@ class AccountReaper(Daemon): 'return_codes.%d' % (err.http_status // 100,)) except (Timeout, socket.error) as err: self.logger.error( - _('Timeout Exception with %(ip)s:%(port)s/%(device)s'), + 'Timeout Exception with %(ip)s:%(port)s/%(device)s', node) failures += 1 self.logger.increment('containers_failures') @@ -510,7 +500,7 @@ class AccountReaper(Daemon): except ClientException as err: if self.logger.getEffectiveLevel() <= DEBUG: self.logger.exception( - _('Exception with %(ip)s:%(port)s/%(device)s'), node) + 'Exception with %(ip)s:%(port)s/%(device)s', node) failures += 1 self.logger.increment('objects_failures') self.stats_return_codes[err.http_status // 100] = \ @@ -521,7 +511,7 @@ class AccountReaper(Daemon): failures += 1 self.logger.increment('objects_failures') self.logger.error( - _('Timeout Exception with %(ip)s:%(port)s/%(device)s'), + 'Timeout Exception with %(ip)s:%(port)s/%(device)s', node) if successes > failures: self.stats_objects_deleted += 1 diff --git a/test/unit/account/test_reaper.py b/test/unit/account/test_reaper.py index 8d490e5447..c801ef0949 100644 --- a/test/unit/account/test_reaper.py +++ b/test/unit/account/test_reaper.py @@ -22,7 +22,6 @@ import unittest from logging import DEBUG from mock import patch, call, DEFAULT -import six import eventlet from swift.account import reaper @@ -85,9 +84,13 @@ class FakeAccountBroker(object): 'delete_timestamp': time.time() - 10} return info - def list_containers_iter(self, *args): + def list_containers_iter(self, limit, marker, *args): for cont in self.containers: - yield cont, None, None, None, None + if cont > marker: + yield cont, None, None, None, None + limit -= 1 + if limit <= 0: + break def is_status_deleted(self): return True @@ -196,11 +199,11 @@ class TestReaper(unittest.TestCase): raise self.myexp if self.timeout: raise eventlet.Timeout() - objects = [{'name': 'o1'}, - {'name': 'o2'}, - {'name': six.text_type('o3')}, - {'name': ''}] - return None, objects + objects = [{'name': u'o1'}, + {'name': u'o2'}, + {'name': u'o3'}, + {'name': u'o4'}] + return None, [o for o in objects if o['name'] > kwargs['marker']] def fake_container_ring(self): return FakeRing() @@ -589,7 +592,7 @@ class TestReaper(unittest.TestCase): self.r.stats_return_codes.get(2, 0) + 1 def test_reap_account(self): - containers = ('c1', 'c2', 'c3', '') + containers = ('c1', 'c2', 'c3', 'c4') broker = FakeAccountBroker(containers) self.called_amount = 0 self.r = r = self.init_reaper({}, fakelogger=True)