From cb67f9472d9de0e1ef66caf5004bf24c3e517666 Mon Sep 17 00:00:00 2001 From: Clay Gerrard Date: Wed, 22 Jul 2020 15:17:08 -0500 Subject: [PATCH] container-sharding: Stable overlap order When you have overlapping active shard ranges updates will get sent to "the first" database; but when the proxy queries shards for listings they get stitched together end-to-end with markers. This means mostly the second shard range is ignored. But since the order of shard ranges is not stable (it seems to depend on the database indexes; which can change when rows are added or removed) you could send updates to "the wrong" shard. Using a stable order leads to more correct and robust behavior under failure; and is also better for cognitive overhead. Change-Id: Ia9d29822bf07757fc1cf58ded90b49f12b7b2c24 --- swift/container/backend.py | 3 ++- test/unit/container/test_backend.py | 40 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/swift/container/backend.py b/swift/container/backend.py index 6886ccd69d..01e854c3f8 100644 --- a/swift/container/backend.py +++ b/swift/container/backend.py @@ -1777,7 +1777,8 @@ class ContainerBroker(DatabaseBroker): exclude_others=exclude_others)] # note if this ever changes to *not* sort by upper first then it breaks # a key assumption for bisect, which is used by utils.find_shard_ranges - shard_ranges.sort(key=lambda sr: (sr.upper, sr.state, sr.lower)) + shard_ranges.sort(key=lambda sr: ( + sr.upper, sr.state, sr.lower, sr.name)) if includes: shard_range = find_shard_range(includes, shard_ranges) return [shard_range] if shard_range else [] diff --git a/test/unit/container/test_backend.py b/test/unit/container/test_backend.py index eb976c01fe..2555107181 100644 --- a/test/unit/container/test_backend.py +++ b/test/unit/container/test_backend.py @@ -26,6 +26,7 @@ import random from collections import defaultdict from contextlib import contextmanager import sqlite3 +import string import pickle import json import itertools @@ -3927,6 +3928,45 @@ class TestContainerBroker(unittest.TestCase): include_own=False, exclude_others=True) self.assertFalse(actual) + @with_tempdir + def test_overloap_shard_range_order(self, tempdir): + db_path = os.path.join(tempdir, 'container.db') + broker = ContainerBroker(db_path, account='a', container='c') + broker.initialize(next(self.ts).internal, 0) + + epoch0 = next(self.ts) + epoch1 = next(self.ts) + shard_ranges = [ + ShardRange('.shard_a/shard_%d-%d' % (e, s), epoch, l, u, + state=ShardRange.ACTIVE) + for s, (l, u) in enumerate(zip(string.ascii_letters[:7], + string.ascii_letters[1:])) + for e, epoch in enumerate((epoch0, epoch1)) + ] + + random.shuffle(shard_ranges) + for sr in shard_ranges: + broker.merge_shard_ranges([sr]) + + expected = [ + '.shard_a/shard_0-0', + '.shard_a/shard_1-0', + '.shard_a/shard_0-1', + '.shard_a/shard_1-1', + '.shard_a/shard_0-2', + '.shard_a/shard_1-2', + '.shard_a/shard_0-3', + '.shard_a/shard_1-3', + '.shard_a/shard_0-4', + '.shard_a/shard_1-4', + '.shard_a/shard_0-5', + '.shard_a/shard_1-5', + '.shard_a/shard_0-6', + '.shard_a/shard_1-6', + ] + self.assertEqual(expected, [ + sr.name for sr in broker.get_shard_ranges()]) + @with_tempdir def test_get_shard_ranges_with_sharding_overlaps(self, tempdir): db_path = os.path.join(tempdir, 'container.db')