diff --git a/swift/proxy/server.py b/swift/proxy/server.py index b62c434fb9..dc7aca68e0 100644 --- a/swift/proxy/server.py +++ b/swift/proxy/server.py @@ -490,6 +490,7 @@ class Application(object): handoff_nodes = node_iter nodes_left = self.request_node_count(len(primary_nodes)) + log_handoffs_threshold = nodes_left - len(primary_nodes) for node in primary_nodes: if not self.error_limited(node): yield node @@ -501,11 +502,11 @@ class Application(object): for node in handoff_nodes: if not self.error_limited(node): handoffs += 1 - if self.log_handoffs: + if self.log_handoffs and handoffs > log_handoffs_threshold: self.logger.increment('handoff_count') self.logger.warning( 'Handoff requested (%d)' % handoffs) - if handoffs == len(primary_nodes): + if handoffs - log_handoffs_threshold == len(primary_nodes): self.logger.increment('handoff_all_count') yield node if not self.error_limited(node): diff --git a/test/unit/__init__.py b/test/unit/__init__.py index 019b8ffef2..fc4cc60e4b 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -152,10 +152,7 @@ class FakeRing(Ring): def clear_errors(self): for dev in self.devs: for key in ('errors', 'last_error'): - try: - del dev[key] - except KeyError: - pass + dev.pop(key, None) def set_replicas(self, replicas): self.replicas = replicas diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 552fae9cea..eca1128f1e 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -2381,34 +2381,85 @@ class TestObjectController(unittest.TestCase): collected_nodes.append(node) self.assertEquals(len(collected_nodes), 9) + # zero error-limited primary nodes -> no handoff warnings self.app.log_handoffs = True self.app.logger = FakeLogger() - object_ring.max_more_nodes = 2 + self.app.request_node_count = lambda r: 7 + object_ring.max_more_nodes = 20 partition, nodes = object_ring.get_nodes('account', 'container', 'object') collected_nodes = [] - for node in self.app.iter_nodes(object_ring, - partition): + for node in self.app.iter_nodes(object_ring, partition): collected_nodes.append(node) - self.assertEquals(len(collected_nodes), 5) - self.assertEquals( - self.app.logger.log_dict['warning'], - [(('Handoff requested (1)',), {}), - (('Handoff requested (2)',), {})]) - - self.app.log_handoffs = False - self.app.logger = FakeLogger() - object_ring.max_more_nodes = 2 - partition, nodes = object_ring.get_nodes('account', - 'container', - 'object') - collected_nodes = [] - for node in self.app.iter_nodes(object_ring, - partition): - collected_nodes.append(node) - self.assertEquals(len(collected_nodes), 5) + self.assertEquals(len(collected_nodes), 7) self.assertEquals(self.app.logger.log_dict['warning'], []) + self.assertEquals(self.app.logger.get_increments(), []) + + # one error-limited primary node -> one handoff warning + self.app.log_handoffs = True + self.app.logger = FakeLogger() + self.app.request_node_count = lambda r: 7 + object_ring.clear_errors() + object_ring._devs[0]['errors'] = 999 + object_ring._devs[0]['last_error'] = 2 ** 63 - 1 + + collected_nodes = [] + for node in self.app.iter_nodes(object_ring, partition): + collected_nodes.append(node) + self.assertEquals(len(collected_nodes), 7) + self.assertEquals(self.app.logger.log_dict['warning'], [ + (('Handoff requested (5)',), {})]) + self.assertEquals(self.app.logger.get_increments(), + ['handoff_count']) + + # two error-limited primary nodes -> two handoff warnings + self.app.log_handoffs = True + self.app.logger = FakeLogger() + self.app.request_node_count = lambda r: 7 + object_ring.clear_errors() + for i in range(2): + object_ring._devs[i]['errors'] = 999 + object_ring._devs[i]['last_error'] = 2 ** 63 - 1 + + collected_nodes = [] + for node in self.app.iter_nodes(object_ring, partition): + collected_nodes.append(node) + self.assertEquals(len(collected_nodes), 7) + self.assertEquals(self.app.logger.log_dict['warning'], [ + (('Handoff requested (5)',), {}), + (('Handoff requested (6)',), {})]) + self.assertEquals(self.app.logger.get_increments(), + ['handoff_count', + 'handoff_count']) + + # all error-limited primary nodes -> four handoff warnings, + # plus a handoff-all metric + self.app.log_handoffs = True + self.app.logger = FakeLogger() + self.app.request_node_count = lambda r: 10 + object_ring.set_replicas(4) # otherwise we run out of handoffs + object_ring.clear_errors() + for i in range(4): + object_ring._devs[i]['errors'] = 999 + object_ring._devs[i]['last_error'] = 2 ** 63 - 1 + + collected_nodes = [] + for node in self.app.iter_nodes(object_ring, partition): + collected_nodes.append(node) + self.assertEquals(len(collected_nodes), 10) + self.assertEquals(self.app.logger.log_dict['warning'], [ + (('Handoff requested (7)',), {}), + (('Handoff requested (8)',), {}), + (('Handoff requested (9)',), {}), + (('Handoff requested (10)',), {})]) + self.assertEquals(self.app.logger.get_increments(), + ['handoff_count', + 'handoff_count', + 'handoff_count', + 'handoff_count', + 'handoff_all_count']) + finally: object_ring.max_more_nodes = 0