5070869ac0
We should never assign multiple replicas of the same partition to the
same device - our on-disk layout can only support a single replica of a
given part on a single device. We should not do this, so we validate
against it and raise a loud warning if this terrible state is ever
observed after a rebalance.
Unfortunately currently there's a couple not necessarily uncommon
scenarios which will trigger this observed state today:
1. If we have less devices than replicas
2. If a server or zones aggregate device weight make it the most
appropriate candidate for multiple replicas and you're a bit unlucky
Fixing #1 would be easy, we should just not allow that state anymore.
Really we never did - if you have a 3 replica ring with one device - you
have one replica. Everything that iter_nodes'd would de-dupe. We
should just be insisting that you explicitly acknowledge your replica
count with set_replicas.
I have been lost in the abyss for days searching for a general solutions
to #2. I'm sure it exists, but I will not have wrestled it to
submission by RC1. In the meantime we can eliminate a great deal of the
luck required simply by refusing to place more than one replica of a
part on a device in assign_parts.
The meat of the change is a small update to the .validate method in
RingBuilder. It basically unrolls a pre-existing (part, replica) loop
so that all the replicas of the part come out in order so that we can
build up the set of dev_id's for which all the replicas of a given part
are assigned part-by-part.
If we observe any duplicates - we raise a warning.
To clean the cobwebs out of the rest of the corner cases we're going to
delay get_required_overload from kicking in until we achive dispersion,
and a small check was added when selecting a device subtier to validate
if it's already being used - picking any other device in the tier works
out much better. If no other devices are available in the tier - we
raise a warning. A more elegant or optimized solution may exist.
Many unittests did not meet the criteria #1, but the fix was straight
forward after being identified by the pigeonhole check.
However, many more tests were affected by #2 - but again the fix came to
be simply adding more devices. The fantasy that all failure domains
contain at least replica count devices is prevalent in both our ring
placement algorithm and it's tests. These tests were trying to
demonstrate some complex characteristics of our ring placement algorithm
and I believe we just got a bit too carried away trying to find the
simplest possible example to demonstrate the desirable trait. I think
a better example looks more like a real ring - with many devices in each
server and many servers in each zone - I think more devices makes the
tests better. As much as possible I've tried to maintain the original
intent of the tests - when adding devices I've either spread the weight
out amongst them or added proportional weights to the other tiers.
I added an example straw man test to validate that three devices with
different weights in three different zones won't blow up. Once we can
do that without raising warnings and assigning duplicate device part
replicas - we can add more. And more importantly change the warnings to
errors - because we would much prefer to not do that #$%^ anymore.
Co-Authored-By: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>
Related-Bug: #1452431
Change-Id: I592d5b611188670ae842fe3d030aa3b340ac36f9
256 lines
9.8 KiB
Python
256 lines
9.8 KiB
Python
#! /usr/bin/env python
|
|
# Copyright (c) 2015 Samuel Merritt <sam@swiftstack.com>
|
|
#
|
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
|
# you may not use this file except in compliance with the License.
|
|
# You may obtain a copy of the License at
|
|
#
|
|
# http://www.apache.org/licenses/LICENSE-2.0
|
|
#
|
|
# Unless required by applicable law or agreed to in writing, software
|
|
# distributed under the License is distributed on an "AS IS" BASIS,
|
|
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
|
|
# implied.
|
|
# See the License for the specific language governing permissions and
|
|
# limitations under the License.
|
|
|
|
import os
|
|
import json
|
|
import mock
|
|
import unittest
|
|
from StringIO import StringIO
|
|
from test.unit import with_tempdir
|
|
|
|
from swift.cli.ring_builder_analyzer import parse_scenario, run_scenario
|
|
|
|
|
|
class TestRunScenario(unittest.TestCase):
|
|
@with_tempdir
|
|
def test_it_runs(self, tempdir):
|
|
builder_path = os.path.join(tempdir, 'test.builder')
|
|
scenario = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0,
|
|
'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100],
|
|
['add', 'z2-3.4.5.6:7/sda9', 200],
|
|
['add', 'z2-3.4.5.6:7/sda10', 200],
|
|
['add', 'z2-3.4.5.6:7/sda11', 200]],
|
|
[['set_weight', 0, 150]],
|
|
[['remove', 1]],
|
|
[['save', builder_path]]]}
|
|
parsed = parse_scenario(json.dumps(scenario))
|
|
|
|
fake_stdout = StringIO()
|
|
with mock.patch('sys.stdout', fake_stdout):
|
|
run_scenario(parsed)
|
|
|
|
# Just test that it produced some output as it ran; the fact that
|
|
# this doesn't crash and produces output that resembles something
|
|
# useful is good enough.
|
|
self.assertTrue('Rebalance' in fake_stdout.getvalue())
|
|
self.assertTrue(os.path.exists(builder_path))
|
|
|
|
|
|
class TestParseScenario(unittest.TestCase):
|
|
def test_good(self):
|
|
scenario = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0,
|
|
'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100],
|
|
['add', 'z2-3.4.5.6:7/sda9', 200]],
|
|
[['set_weight', 0, 150]],
|
|
[['remove', 1]]]}
|
|
parsed = parse_scenario(json.dumps(scenario))
|
|
|
|
self.assertEqual(parsed['replicas'], 3)
|
|
self.assertEqual(parsed['part_power'], 8)
|
|
self.assertEqual(parsed['random_seed'], 123)
|
|
self.assertEqual(parsed['overload'], 0)
|
|
self.assertEqual(parsed['rounds'], [
|
|
[['add', {'device': 'sda8',
|
|
'ip': '3.4.5.6',
|
|
'meta': '',
|
|
'port': 7,
|
|
'region': 1,
|
|
'replication_ip': '3.4.5.6',
|
|
'replication_port': 7,
|
|
'weight': 100.0,
|
|
'zone': 2}],
|
|
['add', {'device': u'sda9',
|
|
'ip': u'3.4.5.6',
|
|
'meta': '',
|
|
'port': 7,
|
|
'region': 1,
|
|
'replication_ip': '3.4.5.6',
|
|
'replication_port': 7,
|
|
'weight': 200.0,
|
|
'zone': 2}]],
|
|
[['set_weight', 0, 150.0]],
|
|
[['remove', 1]]])
|
|
|
|
# The rest of this test class is just a catalog of the myriad ways that
|
|
# the input can be malformed.
|
|
def test_invalid_json(self):
|
|
self.assertRaises(ValueError, parse_scenario, "{")
|
|
|
|
def test_json_not_object(self):
|
|
self.assertRaises(ValueError, parse_scenario, "[]")
|
|
self.assertRaises(ValueError, parse_scenario, "\"stuff\"")
|
|
|
|
def test_bad_replicas(self):
|
|
working_scenario = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0,
|
|
'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]}
|
|
|
|
busted = dict(working_scenario)
|
|
del busted['replicas']
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, replicas='blahblah')
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, replicas=-1)
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_part_power(self):
|
|
working_scenario = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0,
|
|
'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]}
|
|
|
|
busted = dict(working_scenario)
|
|
del busted['part_power']
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, part_power='blahblah')
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, part_power=0)
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, part_power=33)
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_random_seed(self):
|
|
working_scenario = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0,
|
|
'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]}
|
|
|
|
busted = dict(working_scenario)
|
|
del busted['random_seed']
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, random_seed='blahblah')
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_overload(self):
|
|
working_scenario = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0,
|
|
'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]}
|
|
|
|
busted = dict(working_scenario)
|
|
del busted['overload']
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, overload='blahblah')
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(working_scenario, overload=-0.01)
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_rounds(self):
|
|
base = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0}
|
|
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(base))
|
|
|
|
busted = dict(base, rounds={})
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(base, rounds=[{}])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
busted = dict(base, rounds=[[['bork']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_add(self):
|
|
base = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0}
|
|
|
|
# no dev
|
|
busted = dict(base, rounds=[[['add']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# no weight
|
|
busted = dict(base, rounds=[[['add', 'r1z2-1.2.3.4:6000/d7']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# too many fields
|
|
busted = dict(base, rounds=[[['add', 'r1z2-1.2.3.4:6000/d7', 1, 2]]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# can't parse
|
|
busted = dict(base, rounds=[[['add', 'not a good value', 100]]])
|
|
# N.B. the ValueError's coming out of ring.utils.parse_add_value
|
|
# are already pretty good
|
|
expected = "Invalid device specifier (round 0, command 0): " \
|
|
"Invalid add value: not a good value"
|
|
try:
|
|
parse_scenario(json.dumps(busted))
|
|
except ValueError as err:
|
|
self.assertEqual(str(err), expected)
|
|
|
|
# negative weight
|
|
busted = dict(base, rounds=[[['add', 'r1z2-1.2.3.4:6000/d7', -1]]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_remove(self):
|
|
base = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0}
|
|
|
|
# no dev
|
|
busted = dict(base, rounds=[[['remove']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# bad dev id
|
|
busted = dict(base, rounds=[[['remove', 'not an int']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# too many fields
|
|
busted = dict(base, rounds=[[['remove', 1, 2]]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_set_weight(self):
|
|
base = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0}
|
|
|
|
# no dev
|
|
busted = dict(base, rounds=[[['set_weight']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# no weight
|
|
busted = dict(base, rounds=[[['set_weight', 0]]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# bad dev id
|
|
busted = dict(base, rounds=[[['set_weight', 'not an int', 90]]])
|
|
expected = "Invalid device ID in set_weight (round 0, command 0): " \
|
|
"invalid literal for int() with base 10: 'not an int'"
|
|
try:
|
|
parse_scenario(json.dumps(busted))
|
|
except ValueError as e:
|
|
self.assertEqual(str(e), expected)
|
|
|
|
# negative weight
|
|
busted = dict(base, rounds=[[['set_weight', 1, -1]]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
# bogus weight
|
|
busted = dict(base, rounds=[[['set_weight', 1, 'bogus']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|
|
|
|
def test_bad_save(self):
|
|
base = {
|
|
'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0}
|
|
|
|
# no builder name
|
|
busted = dict(base, rounds=[[['save']]])
|
|
self.assertRaises(ValueError, parse_scenario, json.dumps(busted))
|