From 0a230e5aed3150bcdbf0f9659ee1ab2f74a305eb Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Fri, 22 Jan 2021 13:10:16 +0000 Subject: [PATCH] Fix logging in proxy container GET path The proxy tried to map the x-backend-sharding-state header value to a ShardRange state, but the value is the container *db* state. The attempted mapping would commonly fail because db state names do not always correspond to ShardRange state names, in which the case the fallback was to log the header value i.e. the correct outcome. Sometimes the attempted mapping would succeed because for example 'sharded' is both a db state name and a ShardRange state name. In that case the log message would look something like: "Found 1024 objects in shard (state=(70, 'sharded')), total = 1024" i.e. the tuple of ShardRange state name and number was logged, which was inappropriate. Change-Id: Ic08e6e7df7162a4c1283a3ef6e67c3b21a4ce494 --- swift/proxy/controllers/container.py | 12 ++++-------- test/unit/proxy/controllers/test_container.py | 9 +++++++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/swift/proxy/controllers/container.py b/swift/proxy/controllers/container.py index 19f631c454..49e4e6f013 100644 --- a/swift/proxy/controllers/container.py +++ b/swift/proxy/controllers/container.py @@ -391,23 +391,19 @@ class ContainerController(Controller): req, shard_range.account, shard_range.container, headers=headers, params=params) - shard_state = 'unknown' - try: - shard_state = shard_resp.headers['x-backend-sharding-state'] - shard_state = ShardRange.resolve_state(shard_state) - except (AttributeError, ValueError, KeyError): - pass + sharding_state = shard_resp.headers.get('x-backend-sharding-state', + 'unknown') if objs is None: # tolerate errors self.app.logger.debug( 'Failed to get objects from shard (state=%s), total = %d', - shard_state, len(objects)) + sharding_state, len(objects)) continue self.app.logger.debug( 'Found %d objects in shard (state=%s), total = %d', - len(objs), shard_state, len(objs) + len(objects)) + len(objs), sharding_state, len(objs) + len(objects)) if not objs: # tolerate empty shard containers diff --git a/test/unit/proxy/controllers/test_container.py b/test/unit/proxy/controllers/test_container.py index 979ca5d72a..34686a1f19 100644 --- a/test/unit/proxy/controllers/test_container.py +++ b/test/unit/proxy/controllers/test_container.py @@ -982,7 +982,7 @@ class TestContainerController(TestRingBase): # when shrinking the final shard will return the root shard range into # which it is shrinking shard_resp_hdrs = { - 'X-Backend-Sharding-State': 'sharded', + 'X-Backend-Sharding-State': 'sharding', 'X-Container-Object-Count': 0, 'X-Container-Bytes-Used': 0, 'X-Backend-Storage-Policy-Index': 0, @@ -1028,6 +1028,11 @@ class TestContainerController(TestRingBase): self.assertEqual( [('a', 'c'), ('.shards_a', 'c_xyz')], resp.request.environ.get('swift.shard_listing_history')) + lines = [line for line in self.app.logger.get_lines_for_level('debug') + if line.startswith('Found 1024 objects in shard')] + self.assertEqual(2, len(lines), lines) + self.assertIn("(state=sharded), total = 1024", lines[0]) # shard->root + self.assertIn("(state=sharding), total = 1024", lines[1]) # shard def test_GET_sharded_container_shard_redirects_between_shards(self): # check that if one shard redirects listing to another shard that @@ -1767,7 +1772,7 @@ class TestContainerController(TestRingBase): 'X-Backend-Storage-Policy-Index': '0'} def _do_test_caching(self, record_type, exp_recheck_listing): - # this test gest shard ranges into cache and then reads from cache + # this test gets shard ranges into cache and then reads from cache sharding_state = 'sharded' self.memcache.delete_all() self.memcache.clear_calls()