From 65e1de29d68f7d7a19833e3a261c9a34d65c136a Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Wed, 13 Jun 2018 12:20:17 +0100 Subject: [PATCH] Fix shard_max_row in ContainerBroker.get_replication_info() ContainerBroker adds the shard_max_row item to the get_replication_info result by querying the db for the shard ranges table max rowid. However, the wrong table name was being used in the db query such that the value was always -1. This bug was benign because the value of shard_max_row is not currently used. Noticed while reviewing [1] which does make use of the shard_max_row *key* in replication info. [1] Related-Change: I7231e8af310e268484f2075f0194b7783cf1c3ea Change-Id: I9e733e301894f1ffff4a1092926cc0df8419c5b5 --- swift/container/backend.py | 2 +- test/unit/container/test_backend.py | 35 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/swift/container/backend.py b/swift/container/backend.py index 8861883340..73d8e20c3d 100644 --- a/swift/container/backend.py +++ b/swift/container/backend.py @@ -838,7 +838,7 @@ class ContainerBroker(DatabaseBroker): def get_replication_info(self): info = super(ContainerBroker, self).get_replication_info() - info['shard_max_row'] = self.get_max_row('shard_ranges') + info['shard_max_row'] = self.get_max_row(SHARD_RANGE_TABLE) return info def _do_get_info_query(self, conn): diff --git a/test/unit/container/test_backend.py b/test/unit/container/test_backend.py index 19a81f1dcf..e063c51536 100644 --- a/test/unit/container/test_backend.py +++ b/test/unit/container/test_backend.py @@ -2013,6 +2013,41 @@ class TestContainerBroker(unittest.TestCase): self.assertEqual(info['reported_object_count'], 2) self.assertEqual(info['reported_bytes_used'], 1123) + @with_tempdir + def test_get_replication_info(self, tempdir): + ts_iter = make_timestamp_iter() + db_path = os.path.join(tempdir, 'part', 'suffix', 'hash', 'hash.db') + broker = ContainerBroker( + db_path, account='myaccount', container='mycontainer') + broker.initialize(next(ts_iter).internal, 0) + metadata = {'blah': ['val', next(ts_iter).internal]} + broker.update_metadata(metadata) + expected = broker.get_info() + expected['metadata'] = json.dumps(metadata) + expected.pop('object_count') + expected['count'] = 0 + expected['max_row'] = -1 + expected['shard_max_row'] = -1 + actual = broker.get_replication_info() + self.assertEqual(expected, actual) + + broker.put_object('o1', next(ts_iter).internal, 123, 'text/plain', + 'fake etag') + expected = broker.get_info() + expected['metadata'] = json.dumps(metadata) + expected.pop('object_count') + expected['count'] = 1 + expected['max_row'] = 1 + expected['shard_max_row'] = -1 + actual = broker.get_replication_info() + self.assertEqual(expected, actual) + + sr = ShardRange('.shards_a/c', next(ts_iter)) + broker.merge_shard_ranges(sr) + expected['shard_max_row'] = 1 + actual = broker.get_replication_info() + self.assertEqual(expected, actual) + @with_tempdir def test_remove_objects(self, tempdir): objects = (('undeleted', Timestamp.now().internal, 0, 'text/plain',