openstack/swift
Code Issues Proposed changes

Always fix devices with multiple part-replica assignments

Browse Source 
I've found that given a sufficiently bad replica2part2dev table we can
accidently not entirely fix palcement when more than two replicas of a
part are assigned to the duplicate devices.

It shows up most on > 3 replica rings when you have two *different*
devices both holding two replicas.  But you can see it on a three
replica ring when all three replicas are assigned to the same device.

Change-Id: Ieb213c1a259815a2ed657291242919cda568c7b5
This commit is contained in:
Clay Gerrard 2016-02-01 15:30:59 -08:00
parent dfb1b58f52
commit b19dc1ddec
2 changed files with 46 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
swift/common/ring/builder.py
View File

@ -978,12 +978,6 @@ class RingBuilder(object):
if dev_id == NONE_DEV: if dev_id == NONE_DEV:
continue continue
dev = self.devs[dev_id] dev = self.devs[dev_id]
# the min part hour check is ignored iff a device has more
# than one replica of a part assigned to it - which would have
# only been possible on rings built with older version of code
if (self._last_part_moves[part] < self.min_part_hours and
not replicas_at_tier[dev['tiers'][-1]] > 1):
break
if all(replicas_at_tier[tier] <= if all(replicas_at_tier[tier] <=
replica_plan[tier]['max'] replica_plan[tier]['max']
for tier in dev['tiers']): for tier in dev['tiers']):
@ -996,8 +990,12 @@ class RingBuilder(object):
undispersed_dev_replicas.sort( undispersed_dev_replicas.sort(
key=lambda dr: dr[0]['parts_wanted']) key=lambda dr: dr[0]['parts_wanted'])
for dev, replica in undispersed_dev_replicas: for dev, replica in undispersed_dev_replicas:
if self._last_part_moves[part] < self.min_part_hours: # the min part hour check is ignored iff a device has more
break # than one replica of a part assigned to it - which would have
# only been possible on rings built with older version of code
if (self._last_part_moves[part] < self.min_part_hours and
not replicas_at_tier[dev['tiers'][-1]] > 1):
continue
dev['parts_wanted'] += 1 dev['parts_wanted'] += 1
dev['parts'] -= 1 dev['parts'] -= 1
assign_parts[part].append(replica) assign_parts[part].append(replica)

40
test/unit/common/ring/test_builder.py
View File

@ -1113,6 +1113,46 @@ class TestRingBuilder(unittest.TestCase):
rb.rebalance() rb.rebalance()
self.assertEqual(rb.get_balance(), 0) self.assertEqual(rb.get_balance(), 0)
def test_multiple_duplicate_device_assignment(self):
rb = ring.RingBuilder(4, 4, 1)
devs = [
'r1z1-127.0.0.1:33440/d1',
'r1z1-127.0.0.1:33441/d2',
'r1z1-127.0.0.1:33442/d3',
'r1z1-127.0.0.1:33443/d4',
'r1z1-127.0.0.2:33440/d5',
'r1z1-127.0.0.2:33441/d6',
'r1z1-127.0.0.2:33442/d7',
'r1z1-127.0.0.2:33442/d8',
]
for add_value in devs:
dev = utils.parse_add_value(add_value)
dev['weight'] = 1.0
rb.add_dev(dev)
rb.rebalance()
rb._replica2part2dev = [
# these are the relevant one's here
# | | | | |
# v v v v v
array('H', [0, 1, 2, 3, 3, 0, 0, 0, 4, 6, 4, 4, 4, 4, 4, 4]),
array('H', [0, 1, 3, 1, 1, 1, 1, 1, 5, 7, 5, 5, 5, 5, 5, 5]),
array('H', [0, 1, 2, 2, 2, 2, 2, 2, 4, 6, 6, 6, 6, 6, 6, 6]),
array('H', [0, 3, 2, 3, 3, 3, 3, 3, 5, 7, 7, 7, 7, 7, 7, 7])
# ^
# |
# this sort of thing worked already
]
# fix up bookkeeping
new_dev_parts = defaultdict(int)
for part2dev_id in rb._replica2part2dev:
for dev_id in part2dev_id:
new_dev_parts[dev_id] += 1
for dev in rb._iter_devs():
dev['parts'] = new_dev_parts[dev['id']]
rb.pretend_min_part_hours_passed()
rb.rebalance()
rb.validate()
def test_region_fullness_with_balanceable_ring(self): def test_region_fullness_with_balanceable_ring(self):
rb = ring.RingBuilder(8, 3, 1) rb = ring.RingBuilder(8, 3, 1)
rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1, rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,