openstack/swift
Code Issues Proposed changes

Clean up account-reaper a bit

Browse Source 
- 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
This commit is contained in:
Tim Burke 2018-10-18 10:35:31 -07:00
parent 3c224af80c
commit baf18edc00
2 changed files with 52 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
swift/account/reaper.py
View File

@ -16,7 +16,6 @@
import os import os
import random import random
import socket import socket
from swift import gettext_ as _
from logging import DEBUG from logging import DEBUG
from math import sqrt from math import sqrt
from time import time from time import time
@ -142,10 +141,10 @@ class AccountReaper(Daemon):
continue continue
self.reap_device(device) self.reap_device(device)
except (Exception, Timeout): except (Exception, Timeout):
self.logger.exception(_("Exception in top-level account reaper " self.logger.exception("Exception in top-level account reaper "
"loop")) "loop")
elapsed = time() - begin elapsed = time() - begin
self.logger.info(_('Devices pass completed: %.02fs'), elapsed) self.logger.info('Devices pass completed: %.02fs', elapsed)
def reap_device(self, device): def reap_device(self, device):
""" """
@ -255,19 +254,15 @@ class AccountReaper(Daemon):
self.delay_reaping: self.delay_reaping:
return False return False
account = info['account'] account = info['account']
self.logger.info(_('Beginning pass on account %s'), account) self.logger.info('Beginning pass on account %s', account)
self.reset_stats() self.reset_stats()
container_limit = 1000 container_limit = 1000
if container_shard is not None: if container_shard is not None:
container_limit *= len(nodes) container_limit *= len(nodes)
try: try:
marker = '' containers = list(broker.list_containers_iter(
while True: container_limit, '', None, None, None))
containers = \ while containers:
list(broker.list_containers_iter(container_limit, marker,
None, None, None))
if not containers:
break
try: try:
for (container, _junk, _junk, _junk, _junk) in containers: for (container, _junk, _junk, _junk, _junk) in containers:
if six.PY3: if six.PY3:
@ -284,43 +279,44 @@ class AccountReaper(Daemon):
self.container_pool.waitall() self.container_pool.waitall()
except (Exception, Timeout): except (Exception, Timeout):
self.logger.exception( self.logger.exception(
_('Exception with containers for account %s'), account) 'Exception with containers for account %s', account)
marker = containers[-1][0] containers = list(broker.list_containers_iter(
if marker == '': container_limit, containers[-1][0], None, None, None))
break log_buf = ['Completed pass on account %s' % account]
log = 'Completed pass on account %s' % account
except (Exception, Timeout): except (Exception, Timeout):
self.logger.exception( self.logger.exception('Exception with account %s', account)
_('Exception with account %s'), account) log_buf = ['Incomplete pass on account %s' % account]
log = _('Incomplete pass on account %s') % account
if self.stats_containers_deleted: 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: 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: if self.stats_containers_remaining:
log += _(', %s containers remaining') % \ log_buf.append(', %s containers remaining' %
self.stats_containers_remaining self.stats_containers_remaining)
if self.stats_objects_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: if self.stats_containers_possibly_remaining:
log += _(', %s containers possibly remaining') % \ log_buf.append(', %s containers possibly remaining' %
self.stats_containers_possibly_remaining self.stats_containers_possibly_remaining)
if self.stats_objects_possibly_remaining: if self.stats_objects_possibly_remaining:
log += _(', %s objects possibly remaining') % \ log_buf.append(', %s objects possibly remaining' %
self.stats_objects_possibly_remaining self.stats_objects_possibly_remaining)
if self.stats_return_codes: if self.stats_return_codes:
log += _(', return codes: ') log_buf.append(', return codes: ')
for code in sorted(self.stats_return_codes): for code in sorted(self.stats_return_codes):
log += '%s %sxxs, ' % (self.stats_return_codes[code], code) log_buf.append('%s %sxxs, ' % (self.stats_return_codes[code],
log = log[:-2] code))
log += _(', elapsed: %.02fs') % (time() - begin) log_buf[-1] = log_buf[-1][:-2]
self.logger.info(log) log_buf.append(', elapsed: %.02fs' % (time() - begin))
self.logger.info(''.join(log_buf))
self.logger.timing_since('timing', self.start_time) self.logger.timing_since('timing', self.start_time)
delete_timestamp = Timestamp(info['delete_timestamp']) delete_timestamp = Timestamp(info['delete_timestamp'])
if self.stats_containers_remaining and \ if self.stats_containers_remaining and \
begin - float(delete_timestamp) >= self.reap_not_done_after: begin - float(delete_timestamp) >= self.reap_not_done_after:
self.logger.warning( 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}) {'account': account, 'time': delete_timestamp.isoformat})
return True return True
@ -379,14 +375,14 @@ class AccountReaper(Daemon):
except ClientException as err: except ClientException as err:
if self.logger.getEffectiveLevel() <= DEBUG: if self.logger.getEffectiveLevel() <= DEBUG:
self.logger.exception( 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[err.http_status // 100] = \
self.stats_return_codes.get(err.http_status // 100, 0) + 1 self.stats_return_codes.get(err.http_status // 100, 0) + 1
self.logger.increment( self.logger.increment(
'return_codes.%d' % (err.http_status // 100,)) 'return_codes.%d' % (err.http_status // 100,))
except (Timeout, socket.error) as err: except (Timeout, socket.error) as err:
self.logger.error( self.logger.error(
_('Timeout Exception with %(ip)s:%(port)s/%(device)s'), 'Timeout Exception with %(ip)s:%(port)s/%(device)s',
node) node)
if not objects: if not objects:
break break
@ -397,21 +393,15 @@ class AccountReaper(Daemon):
self.logger.error('ERROR: invalid storage policy index: %r' self.logger.error('ERROR: invalid storage policy index: %r'
% policy_index) % policy_index)
for obj in objects: 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, pool.spawn(self.reap_object, account, container, part,
nodes, obj_name, policy_index) nodes, obj['name'], policy_index)
pool.waitall() pool.waitall()
except (Exception, Timeout): except (Exception, Timeout):
self.logger.exception(_('Exception with objects for container ' self.logger.exception('Exception with objects for container '
'%(container)s for account %(account)s' '%(container)s for account %(account)s',
),
{'container': container, {'container': container,
'account': account}) 'account': account})
marker = objects[-1]['name'] marker = objects[-1]['name']
if marker == '':
break
successes = 0 successes = 0
failures = 0 failures = 0
timestamp = Timestamp.now() timestamp = Timestamp.now()
@ -434,7 +424,7 @@ class AccountReaper(Daemon):
except ClientException as err: except ClientException as err:
if self.logger.getEffectiveLevel() <= DEBUG: if self.logger.getEffectiveLevel() <= DEBUG:
self.logger.exception( self.logger.exception(
_('Exception with %(ip)s:%(port)s/%(device)s'), node) 'Exception with %(ip)s:%(port)s/%(device)s', node)
failures += 1 failures += 1
self.logger.increment('containers_failures') self.logger.increment('containers_failures')
self.stats_return_codes[err.http_status // 100] = \ self.stats_return_codes[err.http_status // 100] = \
@ -443,7 +433,7 @@ class AccountReaper(Daemon):
'return_codes.%d' % (err.http_status // 100,)) 'return_codes.%d' % (err.http_status // 100,))
except (Timeout, socket.error) as err: except (Timeout, socket.error) as err:
self.logger.error( self.logger.error(
_('Timeout Exception with %(ip)s:%(port)s/%(device)s'), 'Timeout Exception with %(ip)s:%(port)s/%(device)s',
node) node)
failures += 1 failures += 1
self.logger.increment('containers_failures') self.logger.increment('containers_failures')
@ -510,7 +500,7 @@ class AccountReaper(Daemon):
except ClientException as err: except ClientException as err:
if self.logger.getEffectiveLevel() <= DEBUG: if self.logger.getEffectiveLevel() <= DEBUG:
self.logger.exception( self.logger.exception(
_('Exception with %(ip)s:%(port)s/%(device)s'), node) 'Exception with %(ip)s:%(port)s/%(device)s', node)
failures += 1 failures += 1
self.logger.increment('objects_failures') self.logger.increment('objects_failures')
self.stats_return_codes[err.http_status // 100] = \ self.stats_return_codes[err.http_status // 100] = \
@ -521,7 +511,7 @@ class AccountReaper(Daemon):
failures += 1 failures += 1
self.logger.increment('objects_failures') self.logger.increment('objects_failures')
self.logger.error( self.logger.error(
_('Timeout Exception with %(ip)s:%(port)s/%(device)s'), 'Timeout Exception with %(ip)s:%(port)s/%(device)s',
node) node)
if successes > failures: if successes > failures:
self.stats_objects_deleted += 1 self.stats_objects_deleted += 1

21
test/unit/account/test_reaper.py
View File

@ -22,7 +22,6 @@ import unittest
from logging import DEBUG from logging import DEBUG
from mock import patch, call, DEFAULT from mock import patch, call, DEFAULT
import six
import eventlet import eventlet
from swift.account import reaper from swift.account import reaper
@ -85,9 +84,13 @@ class FakeAccountBroker(object):
'delete_timestamp': time.time() - 10} 'delete_timestamp': time.time() - 10}
return info return info
def list_containers_iter(self, *args): def list_containers_iter(self, limit, marker, *args):
for cont in self.containers: 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): def is_status_deleted(self):
return True return True
@ -196,11 +199,11 @@ class TestReaper(unittest.TestCase):
raise self.myexp raise self.myexp
if self.timeout: if self.timeout:
raise eventlet.Timeout() raise eventlet.Timeout()
objects = [{'name': 'o1'}, objects = [{'name': u'o1'},
{'name': 'o2'}, {'name': u'o2'},
{'name': six.text_type('o3')}, {'name': u'o3'},
{'name': ''}] {'name': u'o4'}]
return None, objects return None, [o for o in objects if o['name'] > kwargs['marker']]
def fake_container_ring(self): def fake_container_ring(self):
return FakeRing() return FakeRing()
@ -589,7 +592,7 @@ class TestReaper(unittest.TestCase):
self.r.stats_return_codes.get(2, 0) + 1 self.r.stats_return_codes.get(2, 0) + 1
def test_reap_account(self): def test_reap_account(self):
containers = ('c1', 'c2', 'c3', '') containers = ('c1', 'c2', 'c3', 'c4')
broker = FakeAccountBroker(containers) broker = FakeAccountBroker(containers)
self.called_amount = 0 self.called_amount = 0
self.r = r = self.init_reaper({}, fakelogger=True) self.r = r = self.init_reaper({}, fakelogger=True)