openstack/swift
Code Issues Proposed changes

container-sharding: Stable overlap order

Browse Source 
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
This commit is contained in:
Clay Gerrard 2020-07-22 15:17:08 -05:00 committed by Tim Burke
parent b1f3872896
commit cb67f9472d
2 changed files with 42 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
swift/container/backend.py
View File

@ -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 []

40
test/unit/container/test_backend.py
View File

@ -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')