diff options
author | Matthew Oliver <matt@oliver.net.au> | 2023-02-07 17:00:59 +1100 |
---|---|---|
committer | Tim Burke <tim.burke@gmail.com> | 2023-02-08 14:23:45 -0800 |
commit | 6d1a0b334729f65f16e9875630996023f22511c4 (patch) | |
tree | 241d986c357af57d56b38d1f1e837d39245515d9 | |
parent | 488f8c839f034172d20b3f28ca851d83b7a3bfff (diff) | |
download | swift-6d1a0b334729f65f16e9875630996023f22511c4.tar.gz |
Don't clear x-container-sysmeta-sharding on delete_db
In the container backend we have a delete_meta_whitelist which
whitelists specific metadata from being cleared when we delete a broker.
Currently the sharding root and quoted root are in there.
They're there because we now process deleted shards and we need to know
what a shard's root is. But since that time we've found and
edge case bug where we've found old sharding brokers that are stuck
because they're deleted (a deleted shard will clear the sysmeta-sharding
truth value), but also has lost their shard-ranges. This makes them fail
the sharding_enabled check and therefore never get a chance to pull new
shard ranges from their root and are therefore stuck.
A deleted shard wont be recreated, so it doesn't hurt to keep this
sharding sysmeta value, and will be an extra line of defence stopping
similar stuck container issues in the future.
Change-Id: I0bef534eca71b9ce2b29927021b1977463ffbe74
-rw-r--r-- | swift/container/backend.py | 3 | ||||
-rw-r--r-- | test/unit/container/test_backend.py | 5 | ||||
-rw-r--r-- | test/unit/container/test_sharder.py | 25 |
3 files changed, 30 insertions, 3 deletions
diff --git a/swift/container/backend.py b/swift/container/backend.py index 132f9ed55..1130d828c 100644 --- a/swift/container/backend.py +++ b/swift/container/backend.py @@ -379,7 +379,8 @@ class ContainerBroker(DatabaseBroker): db_contains_type = 'object' db_reclaim_timestamp = 'created_at' delete_meta_whitelist = ['x-container-sysmeta-shard-quoted-root', - 'x-container-sysmeta-shard-root'] + 'x-container-sysmeta-shard-root', + 'x-container-sysmeta-sharding'] def __init__(self, db_file, timeout=BROKER_TIMEOUT, logger=None, account=None, container=None, pending_timeout=None, diff --git a/test/unit/container/test_backend.py b/test/unit/container/test_backend.py index 0c614f484..7e0a77872 100644 --- a/test/unit/container/test_backend.py +++ b/test/unit/container/test_backend.py @@ -1686,7 +1686,7 @@ class TestContainerBroker(unittest.TestCase): self.assertEqual(fresh_broker.get_db_state(), 'unsharded') @with_tempdir - def test_delete_db_does_not_clear_root_path(self, tempdir): + def test_delete_db_does_not_clear_particular_sharding_meta(self, tempdir): acct = '.sharded_a' cont = 'c' hsh = hash_path(acct, cont) @@ -1702,6 +1702,7 @@ class TestContainerBroker(unittest.TestCase): 'foo': ('bar', ts), 'icecream': ('sandwich', ts), 'X-Container-Sysmeta-Some': ('meta', ts), + 'X-Container-Sysmeta-Sharding': ('yes', ts), 'X-Container-Sysmeta-Shard-Quoted-Root': ('a/c', ts), 'X-Container-Sysmeta-Shard-Root': ('a/c', ts)}) @@ -1722,6 +1723,8 @@ class TestContainerBroker(unittest.TestCase): self.assertEqual(meta['X-Container-Sysmeta-Shard-Root'], ['a/c', ts]) self.assertEqual('a/c', broker.root_path) + self.assertEqual(meta['X-Container-Sysmeta-Sharding'], + ['yes', ts]) self.assertFalse(broker.is_root_container()) check_metadata(broker) diff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py index e76565b12..5e77c9071 100644 --- a/test/unit/container/test_sharder.py +++ b/test/unit/container/test_sharder.py @@ -4808,13 +4808,36 @@ class TestSharder(BaseTestSharder): def test_sharding_enabled(self): broker = self._make_broker() self.assertFalse(sharding_enabled(broker)) + # Setting sharding to a true value and sharding will be enabled broker.update_metadata( {'X-Container-Sysmeta-Sharding': ('yes', Timestamp.now().internal)}) self.assertTrue(sharding_enabled(broker)) - # deleting broker clears sharding sysmeta + + # deleting broker doesn't clear the Sysmeta-Sharding sysmeta broker.delete_db(Timestamp.now().internal) + self.assertTrue(sharding_enabled(broker)) + + # re-init the DB for the deleted tests + broker.set_storage_policy_index(0, Timestamp.now().internal) + broker.update_metadata( + {'X-Container-Sysmeta-Sharding': + ('yes', Timestamp.now().internal)}) + self.assertTrue(sharding_enabled(broker)) + + # if the Sysmeta-Sharding is falsy value then sharding isn't enabled + for value in ('', 'no', 'false', 'some_fish'): + broker.update_metadata( + {'X-Container-Sysmeta-Sharding': + (value, Timestamp.now().internal)}) + self.assertFalse(sharding_enabled(broker)) + # deleting broker doesn't clear the Sysmeta-Sharding sysmeta + broker.delete_db(Timestamp.now().internal) + self.assertEqual(broker.metadata['X-Container-Sysmeta-Sharding'][0], + 'some_fish') + # so it still isn't enabled (some_fish isn't a true value). self.assertFalse(sharding_enabled(broker)) + # but if broker has a shard range then sharding is enabled broker.merge_shard_ranges( ShardRange('acc/a_shard', Timestamp.now(), 'l', 'u')) |