Ring rebalance respects co-builders' last_part_moves
- Add a CooperativeRingBuilder subclass of RingBuilder. The subclass takes a reference to a parent CompositeRingBuilder which is consulted about whether a part can be moved during rebalance. The parent builder in turn consults all component CooperativeRingBuilder's to decide if a part can be moved. - Make CompositeRingBuilder load CooperativeRingBuilder instances. - Add rebalance() method to CompositeRingBuilder class. - Add a load_components() method to CompositeRingBuilder class. - Change the CompositeRingBuilder compose() method to NOT by default raise a ValueError if component builders have not been modified since last loaded. With the load_components method being added it makes less sense insist by default on loaded components being modified, and it is desirable to have the same semantic for all methods that load components. Previously it has been necessary to use the 'force' flag with compose() to prevent these errors being raised, which has the unfortunate side effect of also disabling all other checks on component builders. A new 'require_modified' parameter is added to compose() which defaults to False but can be set to True if the previous default behaviour is required. Change-Id: I1b30cb3d776be441346a4131007d2487a5440a81
This commit is contained in:
Alistair Coles
parent
3c11f6b8a8
commit
71ec83f414
|
|
@ -156,6 +156,9 @@ class RingBuilder(object):
|
|||
return bool(self._part_moved_bitmap[byte] & (128 >> bit))
|
||||
|
||||
def _can_part_move(self, part):
|
||||
# if min_part_hours is zero then checking _last_part_moves will not
|
||||
# indicate if the part has already moved during the current rebalance,
|
||||
# but _has_part_moved will.
|
||||
return (self._last_part_moves[part] >= self.min_part_hours and
|
||||
not self._has_part_moved(part))
|
||||
|
||||
|
|
@ -457,6 +460,7 @@ class RingBuilder(object):
|
|||
below 1% or doesn't change by more than 1% (only happens with a ring
|
||||
that can't be balanced no matter what).
|
||||
|
||||
:param seed: a value for the random seed (optional)
|
||||
:returns: (number_of_partitions_altered, resulting_balance,
|
||||
number_of_removed_devices)
|
||||
"""
|
||||
|
|
@ -484,7 +488,6 @@ class RingBuilder(object):
|
|||
if self._last_part_moves is None:
|
||||
self.logger.debug("New builder; performing initial balance")
|
||||
self._last_part_moves = array('B', itertools.repeat(0, self.parts))
|
||||
self._part_moved_bitmap = bytearray(max(2 ** (self.part_power - 3), 1))
|
||||
self._update_last_part_moves()
|
||||
|
||||
replica_plan = self._build_replica_plan()
|
||||
|
|
@ -896,8 +899,9 @@ class RingBuilder(object):
|
|||
current time. The builder won't move a partition that has been moved
|
||||
more recently than min_part_hours.
|
||||
"""
|
||||
self._part_moved_bitmap = bytearray(max(2 ** (self.part_power - 3), 1))
|
||||
elapsed_hours = int(time() - self._last_part_moves_epoch) / 3600
|
||||
if elapsed_hours <= 0:
|
||||
if elapsed_hours <= 0 or not self._last_part_moves:
|
||||
return
|
||||
for part in range(self.parts):
|
||||
# The "min(self._last_part_moves[part] + elapsed_hours, 0xff)"
|
||||
|
|
@ -1651,7 +1655,7 @@ class RingBuilder(object):
|
|||
yield (part, replica)
|
||||
|
||||
@classmethod
|
||||
def load(cls, builder_file, open=open):
|
||||
def load(cls, builder_file, open=open, **kwargs):
|
||||
"""
|
||||
Obtain RingBuilder instance of the provided builder file
|
||||
|
||||
|
|
@ -1680,7 +1684,7 @@ class RingBuilder(object):
|
|||
|
||||
if not hasattr(builder, 'devs'):
|
||||
builder_dict = builder
|
||||
builder = RingBuilder(1, 1, 1)
|
||||
builder = cls(1, 1, 1, **kwargs)
|
||||
builder.copy_from(builder_dict)
|
||||
|
||||
if not hasattr(builder, '_id'):
|
||||
|
|
|
|||
|
|
@ -93,6 +93,9 @@ import copy
|
|||
import json
|
||||
import os
|
||||
|
||||
from random import shuffle
|
||||
|
||||
from swift.common.exceptions import RingBuilderError
|
||||
from swift.common.ring import RingBuilder
|
||||
from swift.common.ring import RingData
|
||||
from collections import defaultdict
|
||||
|
|
@ -363,13 +366,16 @@ def check_builder_ids(builders):
|
|||
|
||||
class CompositeRingBuilder(object):
|
||||
"""
|
||||
Provides facility to create, persist, load and update composite rings, for
|
||||
example::
|
||||
Provides facility to create, persist, load, rebalance and update composite
|
||||
rings, for example::
|
||||
|
||||
# create a CompositeRingBuilder instance with a list of
|
||||
# component builder files
|
||||
crb = CompositeRingBuilder(["region1.builder", "region2.builder"])
|
||||
|
||||
# perform a cooperative rebalance of the component builders
|
||||
crb.rebalance()
|
||||
|
||||
# call compose which will make a new RingData instance
|
||||
ring_data = crb.compose()
|
||||
|
||||
|
|
@ -432,6 +438,7 @@ class CompositeRingBuilder(object):
|
|||
self.ring_data = None
|
||||
self._builder_files = None
|
||||
self._set_builder_files(builder_files or [])
|
||||
self._builders = None # these are lazy loaded in _load_components
|
||||
|
||||
def _set_builder_files(self, builder_files):
|
||||
self._builder_files = [os.path.abspath(bf) for bf in builder_files]
|
||||
|
|
@ -500,10 +507,39 @@ class CompositeRingBuilder(object):
|
|||
metadata['serialization_version'] = 1
|
||||
json.dump(metadata, fp)
|
||||
|
||||
def compose(self, builder_files=None, force=False):
|
||||
def _load_components(self, builder_files=None, force=False,
|
||||
require_modified=False):
|
||||
if self._builders:
|
||||
return self._builder_files, self._builders
|
||||
|
||||
builder_files = builder_files or self._builder_files
|
||||
if len(builder_files) < 2:
|
||||
raise ValueError('Two or more component builders are required.')
|
||||
|
||||
builders = []
|
||||
for builder_file in builder_files:
|
||||
# each component builder gets a reference to this composite builder
|
||||
# so that it can delegate part movement decisions to the composite
|
||||
# builder during rebalance
|
||||
builders.append(CooperativeRingBuilder.load(builder_file,
|
||||
parent_builder=self))
|
||||
check_builder_ids(builders)
|
||||
new_metadata = _make_composite_metadata(builders)
|
||||
if self.components and self._builder_files and not force:
|
||||
modified = check_against_existing(self.to_dict(), new_metadata)
|
||||
if require_modified and not modified:
|
||||
raise ValueError(
|
||||
"None of the component builders has been modified"
|
||||
" since the existing composite ring was built.")
|
||||
self._set_builder_files(builder_files)
|
||||
self._builders = builders
|
||||
return self._builder_files, self._builders
|
||||
|
||||
def load_components(self, builder_files=None, force=False,
|
||||
require_modified=False):
|
||||
"""
|
||||
Builds a composite ring using component ring builders loaded from a
|
||||
list of builder files.
|
||||
Loads component ring builders from builder files. Previously loaded
|
||||
component ring builders will discarded and reloaded.
|
||||
|
||||
If a list of component ring builder files is given then that will be
|
||||
used to load component ring builders. Otherwise, component ring
|
||||
|
|
@ -515,6 +551,43 @@ class CompositeRingBuilder(object):
|
|||
with the existing composition of builders, unless the optional
|
||||
``force`` flag if set True.
|
||||
|
||||
:param builder_files: Optional list of paths to ring builder
|
||||
files that will be used to load the component ring builders.
|
||||
Typically the list of component builder files will have been set
|
||||
when the instance was constructed, for example when using the
|
||||
load() class method. However, this parameter may be used if the
|
||||
component builder file paths have moved, or, in conjunction with
|
||||
the ``force`` parameter, if a new list of component builders is to
|
||||
be used.
|
||||
:param force: if True then do not verify given builders are
|
||||
consistent with any existing composite ring (default is False).
|
||||
:param require_modified: if True and ``force`` is False, then
|
||||
verify that at least one of the given builders has been modified
|
||||
since the composite ring was last built (default is False).
|
||||
:return: A tuple of (builder files, loaded builders)
|
||||
:raises: ValueError if the component ring builders are not suitable for
|
||||
composing with each other, or are inconsistent with any existing
|
||||
composite ring, or if require_modified is True and there has been
|
||||
no change with respect to the existing ring.
|
||||
"""
|
||||
self._builders = None # force a reload of builders
|
||||
return self._load_components(
|
||||
builder_files, force, require_modified)
|
||||
|
||||
def compose(self, builder_files=None, force=False, require_modified=False):
|
||||
"""
|
||||
Builds a composite ring using component ring builders loaded from a
|
||||
list of builder files and updates composite ring metadata.
|
||||
|
||||
If a list of component ring builder files is given then that will be
|
||||
used to load component ring builders. Otherwise, component ring
|
||||
builders will be loaded using the list of builder files that was set
|
||||
when the instance was constructed.
|
||||
|
||||
In either case, if metadata for an existing composite ring has been
|
||||
loaded then the component ring builders are verified for consistency
|
||||
with the existing composition of builders, unless the optional
|
||||
``force`` flag if set True.
|
||||
|
||||
:param builder_files: Optional list of paths to ring builder
|
||||
files that will be used to load the component ring builders.
|
||||
|
|
@ -524,27 +597,139 @@ class CompositeRingBuilder(object):
|
|||
component builder file paths have moved, or, in conjunction with
|
||||
the ``force`` parameter, if a new list of component builders is to
|
||||
be used.
|
||||
:param force: if True then do not verify given builders are consistent
|
||||
with any existing composite ring.
|
||||
:param force: if True then do not verify given builders are
|
||||
consistent with any existing composite ring (default is False).
|
||||
:param require_modified: if True and ``force`` is False, then
|
||||
verify that at least one of the given builders has been modified
|
||||
since the composite ring was last built (default is False).
|
||||
:return: An instance of :class:`swift.common.ring.ring.RingData`
|
||||
:raises: ValueError if the component ring builders are not suitable for
|
||||
composing with each other, or are inconsistent with any existing
|
||||
composite ring, or if there has been no change with respect to the
|
||||
existing ring.
|
||||
composite ring, or if require_modified is True and there has been
|
||||
no change with respect to the existing ring.
|
||||
"""
|
||||
builder_files = builder_files or self._builder_files
|
||||
builders = [RingBuilder.load(f) for f in builder_files]
|
||||
check_builder_ids(builders)
|
||||
new_metadata = _make_composite_metadata(builders)
|
||||
if self.components and self._builder_files and not force:
|
||||
modified = check_against_existing(self.to_dict(), new_metadata)
|
||||
if not modified:
|
||||
raise ValueError(
|
||||
"None of the component builders has been modified"
|
||||
" since the existing composite ring was built.")
|
||||
|
||||
self.ring_data = compose_rings(builders)
|
||||
self.load_components(builder_files, force=force,
|
||||
require_modified=require_modified)
|
||||
self.ring_data = compose_rings(self._builders)
|
||||
self.version += 1
|
||||
new_metadata = _make_composite_metadata(self._builders)
|
||||
self.components = new_metadata['components']
|
||||
self._set_builder_files(builder_files)
|
||||
return self.ring_data
|
||||
|
||||
def rebalance(self):
|
||||
"""
|
||||
Cooperatively rebalances all component ring builders.
|
||||
|
||||
This method does not change the state of the composite ring; a
|
||||
subsequent call to :meth:`compose` is required to generate updated
|
||||
composite :class:`RingData`.
|
||||
|
||||
:return: A list of dicts, one per component builder, each having the
|
||||
following keys:
|
||||
|
||||
* 'builder_file' maps to the component builder file;
|
||||
* 'builder' maps to the corresponding instance of
|
||||
:class:`swift.common.ring.builder.RingBuilder`;
|
||||
* 'result' maps to the results of the rebalance of that component
|
||||
i.e. a tuple of: `(number_of_partitions_altered,
|
||||
resulting_balance, number_of_removed_devices)`
|
||||
|
||||
The list has the same order as components in the composite ring.
|
||||
:raises RingBuilderError: if there is an error while rebalancing any
|
||||
component builder.
|
||||
"""
|
||||
self._load_components()
|
||||
component_builders = zip(self._builder_files, self._builders)
|
||||
# don't let the same builder go first each time
|
||||
shuffle(component_builders)
|
||||
results = {}
|
||||
for builder_file, builder in component_builders:
|
||||
try:
|
||||
results[builder] = {
|
||||
'builder': builder,
|
||||
'builder_file': builder_file,
|
||||
'result': builder.rebalance()
|
||||
}
|
||||
builder.validate()
|
||||
except RingBuilderError as err:
|
||||
self._builders = None
|
||||
raise RingBuilderError(
|
||||
'An error occurred while rebalancing component %s: %s' %
|
||||
(builder_file, err))
|
||||
|
||||
for builder_file, builder in component_builders:
|
||||
builder.save(builder_file)
|
||||
# return results in component order
|
||||
return [results[builder] for builder in self._builders]
|
||||
|
||||
def can_part_move(self, part):
|
||||
"""
|
||||
Check with all component builders that it is ok to move a partition.
|
||||
|
||||
:param part: The partition to check.
|
||||
:return: True if all component builders agree that the partition can be
|
||||
moved, False otherwise.
|
||||
"""
|
||||
# Called by component builders.
|
||||
return all(b.can_part_move(part) for b in self._builders)
|
||||
|
||||
def update_last_part_moves(self):
|
||||
"""
|
||||
Updates the record of how many hours ago each partition was moved in
|
||||
all component builders.
|
||||
"""
|
||||
# Called by component builders. We need all component builders to be at
|
||||
# same last_part_moves epoch before any builder starts moving parts;
|
||||
# this will effectively be a no-op for builders that have already been
|
||||
# updated in last hour
|
||||
for b in self._builders:
|
||||
b.update_last_part_moves()
|
||||
|
||||
|
||||
class CooperativeRingBuilder(RingBuilder):
|
||||
"""
|
||||
A subclass of :class:`RingBuilder` that participates in cooperative
|
||||
rebalance.
|
||||
|
||||
During rebalance this subclass will consult with its `parent_builder`
|
||||
before moving a partition. The `parent_builder` may in turn check with
|
||||
co-builders (including this instance) to verify that none have moved that
|
||||
partition in the last `min_part_hours`.
|
||||
|
||||
:param part_power: number of partitions = 2**part_power.
|
||||
:param replicas: number of replicas for each partition.
|
||||
:param min_part_hours: minimum number of hours between partition changes.
|
||||
:param parent_builder: an instance of :class:`CompositeRingBuilder`.
|
||||
"""
|
||||
def __init__(self, part_power, replicas, min_part_hours, parent_builder):
|
||||
super(CooperativeRingBuilder, self).__init__(
|
||||
part_power, replicas, min_part_hours)
|
||||
self.parent_builder = parent_builder
|
||||
|
||||
def _can_part_move(self, part):
|
||||
# override superclass method to delegate to the parent builder
|
||||
return self.parent_builder.can_part_move(part)
|
||||
|
||||
def can_part_move(self, part):
|
||||
"""
|
||||
Check that in the context of this builder alone it is ok to move a
|
||||
partition.
|
||||
|
||||
:param part: The partition to check.
|
||||
:return: True if the partition can be moved, False otherwise.
|
||||
"""
|
||||
# called by parent_builder - now forward to the superclass
|
||||
return (not self._last_part_moves or
|
||||
super(CooperativeRingBuilder, self)._can_part_move(part))
|
||||
|
||||
def _update_last_part_moves(self):
|
||||
# overrides superclass method to delegate to parent builder
|
||||
return self.parent_builder.update_last_part_moves()
|
||||
|
||||
def update_last_part_moves(self):
|
||||
"""
|
||||
Updates the record of how many hours ago each partition was moved in
|
||||
in this builder.
|
||||
"""
|
||||
# called by parent_builder - now forward to the superclass
|
||||
return super(CooperativeRingBuilder, self)._update_last_part_moves()
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
import json
|
||||
from contextlib import contextmanager
|
||||
|
||||
import mock
|
||||
import os
|
||||
import random
|
||||
|
|
@ -21,9 +23,12 @@ import unittest
|
|||
import shutil
|
||||
import copy
|
||||
|
||||
from collections import defaultdict, Counter
|
||||
|
||||
from swift.common.exceptions import RingBuilderError
|
||||
from swift.common.ring import RingBuilder, Ring
|
||||
from swift.common.ring.composite_builder import (
|
||||
compose_rings, CompositeRingBuilder)
|
||||
compose_rings, CompositeRingBuilder, CooperativeRingBuilder)
|
||||
|
||||
|
||||
def make_device_iter():
|
||||
|
|
@ -81,7 +86,7 @@ class BaseTestCompositeBuilder(unittest.TestCase):
|
|||
builder_files.append(fname)
|
||||
return builder_files
|
||||
|
||||
def create_sample_ringbuilders(self, num_builders=2):
|
||||
def create_sample_ringbuilders(self, num_builders=2, rebalance=True):
|
||||
"""
|
||||
Create sample rings with four devices
|
||||
|
||||
|
|
@ -101,19 +106,25 @@ class BaseTestCompositeBuilder(unittest.TestCase):
|
|||
new_dev = self.pop_region_device(region)
|
||||
new_dev['weight'] = 0
|
||||
builder.add_dev(new_dev)
|
||||
builder.rebalance()
|
||||
if rebalance:
|
||||
builder.rebalance()
|
||||
builder.save(fname)
|
||||
self.assertTrue(os.path.exists(fname))
|
||||
builders.append(builder)
|
||||
|
||||
return builders
|
||||
|
||||
def add_dev_and_rebalance(self, builder, weight=None):
|
||||
dev = next(builder._iter_devs())
|
||||
new_dev = self.pop_region_device(dev['region'])
|
||||
def add_dev(self, builder, weight=None, region=None):
|
||||
if region is None:
|
||||
dev = next(builder._iter_devs())
|
||||
region = dev['region']
|
||||
new_dev = self.pop_region_device(region)
|
||||
if weight is not None:
|
||||
new_dev['weight'] = weight
|
||||
builder.add_dev(new_dev)
|
||||
|
||||
def add_dev_and_rebalance(self, builder, weight=None):
|
||||
self.add_dev(builder, weight)
|
||||
builder.rebalance()
|
||||
|
||||
def assertDevices(self, composite_ring, builders):
|
||||
|
|
@ -190,6 +201,14 @@ class BaseTestCompositeBuilder(unittest.TestCase):
|
|||
}
|
||||
self.assertEqual(expected_metadata, actual)
|
||||
|
||||
def _make_composite_builder(self, builders):
|
||||
# helper to compose a ring, save it and sanity check it
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
cb.compose().save(self.output_ring)
|
||||
self.check_composite_ring(self.output_ring, builders)
|
||||
return cb, builder_files
|
||||
|
||||
|
||||
class TestCompositeBuilder(BaseTestCompositeBuilder):
|
||||
def test_compose_rings(self):
|
||||
|
|
@ -308,7 +327,7 @@ class TestCompositeBuilder(BaseTestCompositeBuilder):
|
|||
def test_compose_rings_rebalance_needed(self):
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
|
||||
# add a new device to builider 1 but no rebalance
|
||||
# add a new device to builder 1 but no rebalance
|
||||
dev = self.pop_region_device(1)
|
||||
builders[1].add_dev(dev)
|
||||
self.assertTrue(builders[1].devs_changed) # sanity check
|
||||
|
|
@ -393,14 +412,6 @@ class TestCompositeRingBuilder(BaseTestCompositeBuilder):
|
|||
cb.compose().save(self.output_ring)
|
||||
self.check_composite_ring(self.output_ring, builders)
|
||||
|
||||
def _make_composite_builder(self, builders):
|
||||
# helper to compose a ring, save it and sanity check it
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
cb.compose().save(self.output_ring)
|
||||
self.check_composite_ring(self.output_ring, builders)
|
||||
return cb, builder_files
|
||||
|
||||
def test_compose_ok(self):
|
||||
cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json')
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
|
|
@ -413,13 +424,13 @@ class TestCompositeRingBuilder(BaseTestCompositeBuilder):
|
|||
# and reloads ok
|
||||
cb = CompositeRingBuilder.load(cb_file)
|
||||
self.assertEqual(1, cb.version)
|
||||
# composes after with no component builder changes will fail...
|
||||
# compose detects if no component builder changes, if we ask it to...
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
cb.compose(require_modified=True)
|
||||
self.assertIn('None of the component builders has been modified',
|
||||
cm.exception.message)
|
||||
self.assertEqual(1, cb.version)
|
||||
# ...unless we force it
|
||||
# ...but by default will compose again despite no changes to components
|
||||
cb.compose(force=True).save(self.output_ring)
|
||||
self.check_composite_ring(self.output_ring, builders)
|
||||
self.assertEqual(2, cb.version)
|
||||
|
|
@ -468,9 +479,10 @@ class TestCompositeRingBuilder(BaseTestCompositeBuilder):
|
|||
# modify builders and save in different files
|
||||
self.add_dev_and_rebalance(builders[1])
|
||||
with self.assertRaises(ValueError):
|
||||
cb.compose(builder_files) # sanity check - originals are unchanged
|
||||
# sanity check - originals are unchanged
|
||||
cb.compose(builder_files, require_modified=True)
|
||||
other_files = self.save_builders(builders, prefix='other')
|
||||
cb.compose(other_files).save(self.output_ring)
|
||||
cb.compose(other_files, require_modified=True).save(self.output_ring)
|
||||
self.check_composite_ring(self.output_ring, builders)
|
||||
# check composite builder persists ok
|
||||
cb.save(cb_file)
|
||||
|
|
@ -504,176 +516,6 @@ class TestCompositeRingBuilder(BaseTestCompositeBuilder):
|
|||
finally:
|
||||
os.chdir(cwd)
|
||||
|
||||
def test_compose_insufficient_builders(self):
|
||||
def do_test(builder_files):
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
self.assertIn('Two or more component builders are required',
|
||||
cm.exception.message)
|
||||
|
||||
cb = CompositeRingBuilder()
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose(builder_files)
|
||||
self.assertIn('Two or more component builders are required',
|
||||
cm.exception.message)
|
||||
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
builder_files = self.save_builders(builders)
|
||||
do_test([])
|
||||
do_test(builder_files[:1])
|
||||
|
||||
def test_compose_missing_builder_id(self):
|
||||
def check_missing_id(cb, builders):
|
||||
# not ok to compose with builder_files that have no id assigned
|
||||
orig_version = cb.version
|
||||
no_id = random.randint(0, len(builders) - 1)
|
||||
# rewrite the builder files so that one has missing id
|
||||
self.save_builders(builders, missing_ids=[no_id])
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Problem with builder at index %s" % no_id,
|
||||
error_lines[0])
|
||||
self.assertIn("id attribute has not been initialised",
|
||||
error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(orig_version, cb.version)
|
||||
# check with compose not previously called, cb has no existing metadata
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
check_missing_id(cb, builders)
|
||||
# now save good copies of builders and compose so this cb has
|
||||
# existing component metadata
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
cb.compose() # cb now has component metadata
|
||||
check_missing_id(cb, builders)
|
||||
|
||||
def test_compose_duplicate_builder_ids(self):
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
builders[2]._id = builders[0]._id
|
||||
cb = CompositeRingBuilder(self.save_builders(builders))
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Builder id %r used at indexes 0, 2" % builders[0].id,
|
||||
error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(0, cb.version)
|
||||
|
||||
def test_compose_ring_unchanged_builders(self):
|
||||
def do_test(cb, builder_files):
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose(builder_files)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("None of the component builders has been modified",
|
||||
error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(1, cb.version)
|
||||
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
# not ok to compose again with same *unchanged* builders
|
||||
do_test(cb, builder_files)
|
||||
# even if we rewrite the files
|
||||
builder_files = self.save_builders(builders)
|
||||
do_test(cb, builder_files)
|
||||
# even if we rename the files
|
||||
builder_files = self.save_builders(builders, prefix='other')
|
||||
do_test(cb, builder_files)
|
||||
|
||||
def test_compose_older_builder(self):
|
||||
# make first version of composite ring
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
old_builders = [copy.deepcopy(b) for b in builders]
|
||||
for i, b in enumerate(builders):
|
||||
self.add_dev_and_rebalance(b)
|
||||
self.assertLess(old_builders[i].version, b.version)
|
||||
self.save_builders(builders)
|
||||
cb.compose() # newer version
|
||||
self.assertEqual(2, cb.version) # sanity check
|
||||
# not ok to use old versions of same builders
|
||||
self.save_builders([old_builders[0], builders[1]])
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Invalid builder change at index 0", error_lines[0])
|
||||
self.assertIn("Older builder version", error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(2, cb.version)
|
||||
# not even if one component ring has changed
|
||||
self.add_dev_and_rebalance(builders[1])
|
||||
self.save_builders([old_builders[0], builders[1]])
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Invalid builder change at index 0", error_lines[0])
|
||||
self.assertIn("Older builder version", error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(2, cb.version)
|
||||
|
||||
def test_compose_different_number_builders(self):
|
||||
# not ok to use a different number of component rings
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
cb, builder_files = self._make_composite_builder(builders[:2])
|
||||
|
||||
def do_test(bad_builders):
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose(self.save_builders(bad_builders))
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(1, cb.version)
|
||||
return error_lines
|
||||
|
||||
error_lines = do_test(builders[:1]) # too few
|
||||
self.assertIn("Missing builder at index 1", error_lines[0])
|
||||
error_lines = do_test(builders) # too many
|
||||
self.assertIn("Unexpected extra builder at index 2", error_lines[0])
|
||||
|
||||
def test_compose_different_builders(self):
|
||||
# not ok to change component rings
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
cb, builder_files = self._make_composite_builder(builders[:2])
|
||||
# ensure builder[0] is newer version so that's not the problem
|
||||
self.add_dev_and_rebalance(builders[0])
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose(self.save_builders([builders[0], builders[2]]))
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Invalid builder change at index 1", error_lines[0])
|
||||
self.assertIn("Attribute mismatch for id", error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(1, cb.version)
|
||||
|
||||
def test_compose_different_builder_order(self):
|
||||
# not ok to change order of component rings
|
||||
builders = self.create_sample_ringbuilders(4)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
builder_files.reverse()
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose(builder_files)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
for i, line in enumerate(error_lines):
|
||||
self.assertIn("Invalid builder change at index %s" % i, line)
|
||||
self.assertIn("Attribute mismatch for id", line)
|
||||
self.assertEqual(1, cb.version)
|
||||
|
||||
def test_compose_replica_count_changed(self):
|
||||
# not ok to change the number of replicas in a ring
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
builders[0].set_replicas(4)
|
||||
self.save_builders(builders)
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
for i, line in enumerate(error_lines):
|
||||
self.assertIn("Invalid builder change at index 0", line)
|
||||
self.assertIn("Attribute mismatch for replicas", line)
|
||||
self.assertEqual(1, cb.version)
|
||||
|
||||
def test_load_errors(self):
|
||||
bad_file = os.path.join(self.tmpdir, 'bad_file.json')
|
||||
with self.assertRaises(IOError):
|
||||
|
|
@ -719,6 +561,610 @@ class TestCompositeRingBuilder(BaseTestCompositeBuilder):
|
|||
do_test(CompositeRingBuilder([]))
|
||||
do_test(CompositeRingBuilder(['file1', 'file2']))
|
||||
|
||||
def test_rebalance(self):
|
||||
@contextmanager
|
||||
def mock_rebalance():
|
||||
# captures component builder rebalance call results, yields a dict
|
||||
# that maps builder -> results
|
||||
calls = defaultdict(list)
|
||||
orig_func = RingBuilder.rebalance
|
||||
|
||||
def func(builder, **kwargs):
|
||||
result = orig_func(builder, **kwargs)
|
||||
calls[builder].append(result)
|
||||
return result
|
||||
|
||||
with mock.patch('swift.common.ring.RingBuilder.rebalance', func):
|
||||
yield calls
|
||||
|
||||
def check_results():
|
||||
self.assertEqual(2, len(rebalance_calls)) # 2 builders called
|
||||
for calls in rebalance_calls.values():
|
||||
self.assertFalse(calls[1:]) # 1 call to each builder
|
||||
|
||||
self.assertEqual(sorted(expected_ids),
|
||||
sorted([b.id for b in rebalance_calls]))
|
||||
self.assertEqual(sorted(expected_versions),
|
||||
sorted([b.version for b in rebalance_calls]))
|
||||
for b in rebalance_calls:
|
||||
self.assertEqual(set(rebalance_calls.keys()),
|
||||
set(b.parent_builder._builders))
|
||||
|
||||
# check the rebalanced builders were saved
|
||||
written_builders = [RingBuilder.load(f) for f in builder_files]
|
||||
self.assertEqual(expected_ids,
|
||||
[b.id for b in written_builders])
|
||||
self.assertEqual(expected_versions,
|
||||
[b.version for b in written_builders])
|
||||
|
||||
# check returned results, should be in component order
|
||||
self.assertEqual(2, len(results))
|
||||
self.assertEqual(builder_files,
|
||||
[r['builder_file'] for r in results])
|
||||
self.assertEqual(expected_versions,
|
||||
[r['builder'].version for r in results])
|
||||
self.assertEqual(expected_ids, [r['builder'].id for r in results])
|
||||
self.assertEqual(
|
||||
[rebalance_calls[r['builder']][0] for r in results],
|
||||
[r['result'] for r in results])
|
||||
|
||||
# N.B. the sample builders have zero min_part_hours
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
expected_versions = [b.version + 1 for b in builders]
|
||||
expected_ids = [b.id for b in builders]
|
||||
|
||||
# test rebalance loads component builders
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
with mock_rebalance() as rebalance_calls:
|
||||
results = cb.rebalance()
|
||||
check_results()
|
||||
|
||||
# test loading builder files via load_components
|
||||
# revert builder files to original builder state
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder()
|
||||
cb.load_components(builder_files)
|
||||
with mock_rebalance() as rebalance_calls:
|
||||
results = cb.rebalance()
|
||||
check_results()
|
||||
|
||||
def test_rebalance_errors(self):
|
||||
cb = CompositeRingBuilder()
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.rebalance()
|
||||
self.assertIn('Two or more component builders are required',
|
||||
cm.exception.message)
|
||||
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
with mock.patch('swift.common.ring.RingBuilder.rebalance',
|
||||
side_effect=RingBuilderError('test')):
|
||||
with mock.patch('swift.common.ring.composite_builder.shuffle',
|
||||
lambda x: x):
|
||||
with self.assertRaises(RingBuilderError) as cm:
|
||||
cb.rebalance()
|
||||
self.assertIn('An error occurred while rebalancing component %s' %
|
||||
builder_files[0], str(cm.exception))
|
||||
self.assertIsNone(cb._builders)
|
||||
|
||||
with mock.patch('swift.common.ring.RingBuilder.validate',
|
||||
side_effect=RingBuilderError('test')):
|
||||
with mock.patch('swift.common.ring.composite_builder.shuffle',
|
||||
lambda x: x):
|
||||
with self.assertRaises(RingBuilderError) as cm:
|
||||
cb.rebalance()
|
||||
self.assertIn('An error occurred while rebalancing component %s' %
|
||||
builder_files[0], str(cm.exception))
|
||||
self.assertIsNone(cb._builders)
|
||||
|
||||
def test_rebalance_with_unrebalanced_builders(self):
|
||||
# create 2 non-rebalanced rings
|
||||
builders = self.create_sample_ringbuilders(rebalance=False)
|
||||
# save builders
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
# sanity, it is impossible to compose un-rebalanced component rings
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
cb.compose()
|
||||
self.assertIn("Builder needs rebalance", cm.exception.message)
|
||||
# but ok to compose after rebalance
|
||||
cb.rebalance()
|
||||
rd = cb.compose()
|
||||
rd.save(self.output_ring)
|
||||
rebalanced_builders = [RingBuilder.load(f) for f in builder_files]
|
||||
self.check_composite_ring(self.output_ring, rebalanced_builders)
|
||||
|
||||
|
||||
class TestLoadComponents(BaseTestCompositeBuilder):
|
||||
# Tests for the loading of component builders.
|
||||
def _call_method_under_test(self, cb, *args, **kwargs):
|
||||
# Component builder loading is triggered by the load_components method
|
||||
# and the compose method. This method provides a hook for subclasses to
|
||||
# configure a different method to repeat the component loading tests.
|
||||
cb.load_components(*args, **kwargs)
|
||||
|
||||
def test_load_components(self):
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
# check lazy loading
|
||||
self.assertEqual(builder_files, cb._builder_files)
|
||||
self.assertFalse(cb._builders) # none loaded yet
|
||||
|
||||
# check loading configured files
|
||||
self._call_method_under_test(cb)
|
||||
self.assertEqual(builder_files, cb._builder_files)
|
||||
for i, builder in enumerate(cb._builders):
|
||||
self.assertEqual(builders[i].id, builder.id)
|
||||
self.assertEqual(builders[i].devs, builder.devs)
|
||||
|
||||
# modify builders and save in different files
|
||||
self.add_dev_and_rebalance(builders[0])
|
||||
other_files = self.save_builders(builders, prefix='other')
|
||||
# reload from other files
|
||||
self._call_method_under_test(cb, other_files)
|
||||
self.assertEqual(other_files, cb._builder_files)
|
||||
for i, builder in enumerate(cb._builders):
|
||||
self.assertEqual(builders[i].id, builder.id)
|
||||
self.assertEqual(builders[i].devs, builder.devs)
|
||||
|
||||
# modify builders again and save in same files
|
||||
self.add_dev_and_rebalance(builders[1])
|
||||
self.save_builders(builders, prefix='other')
|
||||
# reload from same files
|
||||
self._call_method_under_test(cb)
|
||||
self.assertEqual(other_files, cb._builder_files)
|
||||
for i, builder in enumerate(cb._builders):
|
||||
self.assertEqual(builders[i].id, builder.id)
|
||||
self.assertEqual(builders[i].devs, builder.devs)
|
||||
|
||||
def test_load_components_insufficient_builders(self):
|
||||
def do_test(builder_files, force):
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, builder_files,
|
||||
force=force)
|
||||
self.assertIn('Two or more component builders are required',
|
||||
cm.exception.message)
|
||||
|
||||
cb = CompositeRingBuilder()
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, builder_files,
|
||||
force=force)
|
||||
self.assertIn('Two or more component builders are required',
|
||||
cm.exception.message)
|
||||
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
builder_files = self.save_builders(builders)
|
||||
do_test([], force=False)
|
||||
do_test([], force=True) # this error is never ignored
|
||||
do_test(builder_files[:1], force=False)
|
||||
do_test(builder_files[:1], force=True) # this error is never ignored
|
||||
|
||||
def test_load_components_missing_builder_id(self):
|
||||
def check_missing_id(cb, builders):
|
||||
# not ok to load builder_files that have no id assigned
|
||||
orig_version = cb.version
|
||||
no_id = random.randint(0, len(builders) - 1)
|
||||
# rewrite the builder files so that one has missing id
|
||||
builder_files = self.save_builders(builders, missing_ids=[no_id])
|
||||
|
||||
def do_check(force):
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, builder_files,
|
||||
force=force)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Problem with builder at index %s" % no_id,
|
||||
error_lines[0])
|
||||
self.assertIn("id attribute has not been initialised",
|
||||
error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(orig_version, cb.version)
|
||||
|
||||
do_check(False)
|
||||
do_check(True) # we never ignore this error
|
||||
|
||||
# check with compose not previously called, cb has no existing metadata
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
cb = CompositeRingBuilder()
|
||||
check_missing_id(cb, builders)
|
||||
# now save good copies of builders and compose so this cb has
|
||||
# existing component metadata
|
||||
builder_files = self.save_builders(builders)
|
||||
cb = CompositeRingBuilder(builder_files)
|
||||
cb.compose() # cb now has component metadata
|
||||
check_missing_id(cb, builders)
|
||||
|
||||
def test_load_components_duplicate_builder_ids(self):
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
builders[2]._id = builders[0]._id
|
||||
cb = CompositeRingBuilder(self.save_builders(builders))
|
||||
|
||||
def do_check(force):
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, force=force)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Builder id %r used at indexes 0, 2" %
|
||||
builders[0].id, error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(0, cb.version)
|
||||
|
||||
do_check(False)
|
||||
do_check(True)
|
||||
|
||||
def test_load_components_unchanged_builders(self):
|
||||
def do_test(cb, builder_files, **kwargs):
|
||||
orig_version = cb.version
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, builder_files, **kwargs)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("None of the component builders has been modified",
|
||||
error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(orig_version, cb.version)
|
||||
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
# ok to load same *unchanged* builders
|
||||
self._call_method_under_test(cb, builder_files)
|
||||
# unless require_modified is set
|
||||
do_test(cb, builder_files, require_modified=True)
|
||||
# even if we rewrite the files
|
||||
builder_files = self.save_builders(builders)
|
||||
do_test(cb, builder_files, require_modified=True)
|
||||
# even if we rename the files
|
||||
builder_files = self.save_builders(builders, prefix='other')
|
||||
do_test(cb, builder_files, require_modified=True)
|
||||
# force trumps require_modified
|
||||
self._call_method_under_test(cb, builder_files, force=True,
|
||||
require_modified=True)
|
||||
|
||||
def test_load_components_older_builder(self):
|
||||
# make first version of composite ring
|
||||
builders = self.create_sample_ringbuilders(2)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
old_builders = [copy.deepcopy(b) for b in builders]
|
||||
# update components and reload
|
||||
for i, b in enumerate(builders):
|
||||
self.add_dev_and_rebalance(b)
|
||||
self.assertLess(old_builders[i].version, b.version)
|
||||
self.save_builders(builders)
|
||||
self._call_method_under_test(cb)
|
||||
orig_version = cb.version
|
||||
cb.compose() # compose with newer builder versions
|
||||
self.assertEqual(orig_version + 1, cb.version) # sanity check
|
||||
# not ok to use old versions of same builders
|
||||
self.save_builders([old_builders[0], builders[1]])
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Invalid builder change at index 0", error_lines[0])
|
||||
self.assertIn("Older builder version", error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(orig_version + 1, cb.version)
|
||||
# not even if one component ring has changed
|
||||
self.add_dev_and_rebalance(builders[1])
|
||||
self.save_builders([old_builders[0], builders[1]])
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Invalid builder change at index 0", error_lines[0])
|
||||
self.assertIn("Older builder version", error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(orig_version + 1, cb.version)
|
||||
# unless we ignore errors
|
||||
self._call_method_under_test(cb, force=True)
|
||||
self.assertEqual(old_builders[0].version, cb._builders[0].version)
|
||||
|
||||
def test_load_components_different_number_builders(self):
|
||||
# not ok to use a different number of component rings
|
||||
builders = self.create_sample_ringbuilders(4)
|
||||
|
||||
def do_test(bad_builders):
|
||||
cb, builder_files = self._make_composite_builder(builders[:3])
|
||||
# expect an error
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(
|
||||
cb, self.save_builders(bad_builders))
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(1, cb.version)
|
||||
# unless we ignore errors
|
||||
self._call_method_under_test(cb, self.save_builders(bad_builders),
|
||||
force=True)
|
||||
self.assertEqual(len(bad_builders), len(cb._builders))
|
||||
return error_lines
|
||||
|
||||
error_lines = do_test(builders[:2]) # too few
|
||||
self.assertIn("Missing builder at index 2", error_lines[0])
|
||||
error_lines = do_test(builders) # too many
|
||||
self.assertIn("Unexpected extra builder at index 3", error_lines[0])
|
||||
|
||||
def test_load_components_different_builders(self):
|
||||
# not ok to change component rings
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
cb, builder_files = self._make_composite_builder(builders[:2])
|
||||
# ensure builder[0] is newer version so that's not the problem
|
||||
self.add_dev_and_rebalance(builders[0])
|
||||
different_files = self.save_builders([builders[0], builders[2]])
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, different_files)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Invalid builder change at index 1", error_lines[0])
|
||||
self.assertIn("Attribute mismatch for id", error_lines[0])
|
||||
self.assertFalse(error_lines[1:])
|
||||
self.assertEqual(1, cb.version)
|
||||
# ok if we force
|
||||
self._call_method_under_test(cb, different_files, force=True)
|
||||
self.assertEqual(different_files, cb._builder_files)
|
||||
|
||||
def test_load_component_different_builder_order(self):
|
||||
# not ok to change order of component rings
|
||||
builders = self.create_sample_ringbuilders(4)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
builder_files.reverse()
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, builder_files)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
for i, line in enumerate(error_lines):
|
||||
self.assertIn("Invalid builder change at index %s" % i, line)
|
||||
self.assertIn("Attribute mismatch for id", line)
|
||||
self.assertEqual(1, cb.version)
|
||||
# ok if we force
|
||||
self._call_method_under_test(cb, builder_files, force=True)
|
||||
self.assertEqual(builder_files, cb._builder_files)
|
||||
|
||||
def test_load_components_replica_count_changed(self):
|
||||
# not ok to change the number of replicas in a ring
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
builders[0].set_replicas(4)
|
||||
self.save_builders(builders)
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
for i, line in enumerate(error_lines):
|
||||
self.assertIn("Invalid builder change at index 0", line)
|
||||
self.assertIn("Attribute mismatch for replicas", line)
|
||||
self.assertEqual(1, cb.version)
|
||||
# ok if we force
|
||||
self._call_method_under_test(cb, force=True)
|
||||
|
||||
|
||||
class TestComposeLoadComponents(TestLoadComponents):
|
||||
def _call_method_under_test(self, cb, *args, **kwargs):
|
||||
cb.compose(*args, **kwargs)
|
||||
|
||||
def test_load_components_replica_count_changed(self):
|
||||
# For compose method this test differs from superclass when the force
|
||||
# flag is used, because although the force flag causes load_components
|
||||
# to skip checks, the actual ring composition fails.
|
||||
# not ok to change the number of replicas in a ring
|
||||
builders = self.create_sample_ringbuilders(3)
|
||||
cb, builder_files = self._make_composite_builder(builders)
|
||||
builders[0].set_replicas(4)
|
||||
self.save_builders(builders)
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
for i, line in enumerate(error_lines):
|
||||
self.assertIn("Invalid builder change at index 0", line)
|
||||
self.assertIn("Attribute mismatch for replicas", line)
|
||||
self.assertEqual(1, cb.version)
|
||||
# if we force, then load_components succeeds but the compose pre
|
||||
# validate will fail because the builder needs rebalancing
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
self._call_method_under_test(cb, force=True)
|
||||
error_lines = cm.exception.message.split('\n')
|
||||
self.assertIn("Problem with builders", error_lines[0])
|
||||
self.assertIn("Builder needs rebalance", error_lines[1])
|
||||
self.assertFalse(error_lines[2:])
|
||||
self.assertEqual(1, cb.version)
|
||||
|
||||
|
||||
class TestCooperativeRingBuilder(BaseTestCompositeBuilder):
|
||||
def _make_coop_builder(self, region, composite_builder, rebalance=False):
|
||||
rb = CooperativeRingBuilder(8, 3, 1, composite_builder)
|
||||
if composite_builder._builders is None:
|
||||
composite_builder._builders = [rb]
|
||||
for i in range(3):
|
||||
self.add_dev(rb, region=region)
|
||||
if rebalance:
|
||||
rb.rebalance()
|
||||
self.assertEqual(self._partition_counts(rb),
|
||||
{0: 256, 1: 256, 2: 256}) # sanity check
|
||||
return rb
|
||||
|
||||
def _partition_counts(self, builder, key='id'):
|
||||
"""
|
||||
Returns a dictionary mapping the given device key to (number of
|
||||
partitions assigned to that key).
|
||||
"""
|
||||
return Counter(builder.devs[dev_id][key]
|
||||
for part2dev_id in builder._replica2part2dev
|
||||
for dev_id in part2dev_id)
|
||||
|
||||
@mock.patch('swift.common.ring.builder.time')
|
||||
def test_rebalance_respects_cobuilder_part_moves(self, mock_time):
|
||||
def do_rebalance(builder):
|
||||
old_part_devs = [builder._devs_for_part(part)
|
||||
for part in range(builder.parts)]
|
||||
num_moved, _, _ = builder.rebalance()
|
||||
moved_parts = {
|
||||
p for p in range(builder.parts)
|
||||
if old_part_devs[p] != builder._devs_for_part(p)}
|
||||
self.assertEqual(len(moved_parts), num_moved) # sanity check
|
||||
return num_moved, moved_parts
|
||||
|
||||
def num_parts_can_move(builder):
|
||||
# note that can_part_move() gives consideration to the
|
||||
# _part_moved_bitmap which is only reset when a rebalance starts
|
||||
return len(
|
||||
[p for p in range(builder.parts)
|
||||
if super(CooperativeRingBuilder, builder)._can_part_move(p)])
|
||||
|
||||
mock_time.return_value = 0
|
||||
cb = CompositeRingBuilder()
|
||||
rb1 = self._make_coop_builder(1, cb)
|
||||
rb2 = self._make_coop_builder(2, cb)
|
||||
rb3 = self._make_coop_builder(3, cb)
|
||||
cb._builders = [rb1, rb2, rb3]
|
||||
|
||||
# all cobuilders can perform initial rebalance
|
||||
for rb in (rb1, rb2, rb3):
|
||||
rb.rebalance()
|
||||
actual = self._partition_counts(rb)
|
||||
exp = {0: 256, 1: 256, 2: 256}
|
||||
self.assertEqual(exp, actual,
|
||||
'Expected %s but got %s for region %s' %
|
||||
(exp, actual, next(rb._iter_devs())['region']))
|
||||
|
||||
# jump forwards min_part_hours, both builders can move all parts
|
||||
mock_time.return_value = 3600
|
||||
self.add_dev(rb1)
|
||||
# sanity checks: rb1 and rb2 are both ready for rebalance
|
||||
self.assertEqual(0, rb2.min_part_seconds_left)
|
||||
self.assertEqual(0, rb1.min_part_seconds_left)
|
||||
# ... but last_part_moves not yet updated to current epoch
|
||||
self.assertEqual(0, num_parts_can_move(rb1))
|
||||
self.assertEqual(0, num_parts_can_move(rb2))
|
||||
# rebalancing rb1 will update epoch for both builders' last_part_moves
|
||||
num_moved, rb1_parts_moved = do_rebalance(rb1)
|
||||
self.assertEqual(192, num_moved)
|
||||
self.assertEqual(self._partition_counts(rb1),
|
||||
{0: 192, 1: 192, 2: 192, 3: 192})
|
||||
self.assertEqual(256, num_parts_can_move(rb2))
|
||||
self.assertEqual(64, num_parts_can_move(rb1))
|
||||
|
||||
# rebalancing rb2 - rb2 in isolation could potentially move all parts
|
||||
# so would move 192 parts to new device, but it is constrained by rb1
|
||||
# only having 64 parts that can move
|
||||
self.add_dev(rb2)
|
||||
num_moved, rb2_parts_moved = do_rebalance(rb2)
|
||||
self.assertEqual(64, num_moved)
|
||||
counts = self._partition_counts(rb2)
|
||||
self.assertEqual(counts[3], 64)
|
||||
self.assertEqual([234, 235, 235], sorted(counts.values()[:3]))
|
||||
self.assertFalse(rb2_parts_moved.intersection(rb1_parts_moved))
|
||||
self.assertEqual(192, num_parts_can_move(rb2))
|
||||
self.assertEqual(64, num_parts_can_move(rb1))
|
||||
|
||||
# rb3 can't rebalance - all parts moved while rebalancing rb1 and rb2
|
||||
self.add_dev(rb3)
|
||||
num_moved, rb3_parts_moved = do_rebalance(rb3)
|
||||
self.assertEqual(0, num_moved)
|
||||
|
||||
# jump forwards min_part_hours, both builders can move all parts again,
|
||||
# so now rb2 should be able to further rebalance
|
||||
mock_time.return_value = 7200
|
||||
do_rebalance(rb2)
|
||||
self.assertGreater(self._partition_counts(rb2)[3], 64)
|
||||
self.assertLess(num_parts_can_move(rb2), 256)
|
||||
self.assertEqual(256, num_parts_can_move(rb1)) # sanity check
|
||||
|
||||
# but cobuilders will not prevent a rb rebalancing for first time
|
||||
rb4 = self._make_coop_builder(4, cb, rebalance=False)
|
||||
cb._builders.append(rb4)
|
||||
num_moved, _, _ = rb4.rebalance()
|
||||
self.assertEqual(3 * 256, num_moved)
|
||||
|
||||
def test_rebalance_cobuilders(self):
|
||||
# verify that co-builder methods are called during one builder's
|
||||
# rebalance
|
||||
@contextmanager
|
||||
def mock_update_last_part_moves():
|
||||
# intercept calls to RingBuilder._update_last_part_moves (yes, the
|
||||
# superclass method) and populate a dict mapping builder instance
|
||||
# to a list of that builder's parent builder when method was called
|
||||
calls = []
|
||||
orig_func = RingBuilder._update_last_part_moves
|
||||
|
||||
def fake_update(builder):
|
||||
calls.append(builder)
|
||||
return orig_func(builder)
|
||||
|
||||
with mock.patch(
|
||||
'swift.common.ring.RingBuilder._update_last_part_moves',
|
||||
fake_update):
|
||||
yield calls
|
||||
|
||||
@contextmanager
|
||||
def mock_can_part_move():
|
||||
# intercept calls to RingBuilder._can_part_move (yes, the
|
||||
# superclass method) and populate a dict mapping builder instance
|
||||
# to a list of that builder's parent builder when method was called
|
||||
calls = defaultdict(list)
|
||||
orig_func = RingBuilder._can_part_move
|
||||
|
||||
def fake_can_part_move(builder, part):
|
||||
calls[builder].append(part)
|
||||
return orig_func(builder, part)
|
||||
with mock.patch('swift.common.ring.RingBuilder._can_part_move',
|
||||
fake_can_part_move):
|
||||
yield calls
|
||||
|
||||
# single component builder in parent builder
|
||||
cb = CompositeRingBuilder()
|
||||
rb1 = self._make_coop_builder(1, cb)
|
||||
with mock_update_last_part_moves() as update_calls:
|
||||
with mock_can_part_move() as can_part_move_calls:
|
||||
rb1.rebalance()
|
||||
self.assertEqual([rb1], update_calls)
|
||||
self.assertEqual([rb1], can_part_move_calls.keys())
|
||||
self.assertEqual(512, len(can_part_move_calls[rb1]))
|
||||
|
||||
# two component builders with same parent builder
|
||||
cb = CompositeRingBuilder()
|
||||
rb1 = self._make_coop_builder(1, cb)
|
||||
rb2 = self._make_coop_builder(2, cb)
|
||||
cb._builders = [rb1, rb2]
|
||||
with mock_update_last_part_moves() as update_calls:
|
||||
with mock_can_part_move() as can_part_move_calls:
|
||||
rb2.rebalance()
|
||||
# both builders get updated
|
||||
self.assertEqual(sorted([rb1, rb2]), sorted(update_calls))
|
||||
# rb1 has never been rebalanced so no calls propagate from its
|
||||
# can_part_move method to to its superclass _can_part_move method
|
||||
self.assertEqual([rb2], can_part_move_calls.keys())
|
||||
|
||||
with mock_update_last_part_moves() as update_calls:
|
||||
with mock_can_part_move() as can_part_move_calls:
|
||||
rb1.rebalance()
|
||||
# both builders get updated
|
||||
self.assertEqual(sorted([rb1, rb2]), sorted(update_calls))
|
||||
|
||||
# rb1 is being rebalanced so gets checked, and rb2 also gets checked
|
||||
self.assertEqual(sorted([rb1, rb2]), sorted(can_part_move_calls))
|
||||
self.assertEqual(512, len(can_part_move_calls[rb1]))
|
||||
self.assertEqual(512, len(can_part_move_calls[rb2]))
|
||||
|
||||
def test_save_then_load(self):
|
||||
cb = CompositeRingBuilder()
|
||||
coop_rb = self._make_coop_builder(1, cb, rebalance=True)
|
||||
builder_file = os.path.join(self.tmpdir, 'test.builder')
|
||||
coop_rb.save(builder_file)
|
||||
cb = CompositeRingBuilder()
|
||||
loaded_coop_rb = CooperativeRingBuilder.load(builder_file,
|
||||
parent_builder=cb)
|
||||
self.assertIs(cb, loaded_coop_rb.parent_builder)
|
||||
self.assertEqual(coop_rb.to_dict(), loaded_coop_rb.to_dict())
|
||||
|
||||
# check can be loaded as superclass
|
||||
loaded_rb = RingBuilder.load(builder_file)
|
||||
self.assertEqual(coop_rb.to_dict(), loaded_rb.to_dict())
|
||||
|
||||
# check can load a saved superclass
|
||||
rb = RingBuilder(6, 3, 0)
|
||||
for _ in range(3):
|
||||
self.add_dev(rb, region=1)
|
||||
rb.save(builder_file)
|
||||
cb = CompositeRingBuilder()
|
||||
loaded_coop_rb = CooperativeRingBuilder.load(builder_file,
|
||||
parent_builder=cb)
|
||||
self.assertIs(cb, loaded_coop_rb.parent_builder)
|
||||
self.assertEqual(rb.to_dict(), loaded_coop_rb.to_dict())
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
|
|
|||
Loading…
Reference in New Issue