Merge "Ring rebalance respects co-builders' last_part_moves"

swift/common/ring/builder.py

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

swift/common/ring/composite_builder.py

@@ -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
+    Provides facility to create, persist, load, rebalance  and update composite
-    example::
+    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
+        Loads component ring builders from builder files. Previously loaded
-        list of builder files.
+        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
+        :param force: if True then do not verify given builders are
-                      with any existing composite ring.
+            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
+            composite ring, or if require_modified is True and there has been
-            existing ring.
+            no change with respect to the existing ring.
         """
-        builder_files = builder_files or self._builder_files
+        self.load_components(builder_files, force=force,
-        builders = [RingBuilder.load(f) for f in builder_files]
+                             require_modified=require_modified)
-        check_builder_ids(builders)
+        self.ring_data = compose_rings(self._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.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()

test/unit/common/ring/test_composite_builder.py

@@ -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,6 +106,7 @@ class BaseTestCompositeBuilder(unittest.TestCase):
             new_dev = self.pop_region_device(region)
             new_dev['weight'] = 0
             builder.add_dev(new_dev)
+            if rebalance:
                 builder.rebalance()
             builder.save(fname)
             self.assertTrue(os.path.exists(fname))
@@ -108,12 +114,17 @@ class BaseTestCompositeBuilder(unittest.TestCase):
 
         return builders
 
-    def add_dev_and_rebalance(self, builder, weight=None):
+    def add_dev(self, builder, weight=None, region=None):
+        if region is None:
             dev = next(builder._iter_devs())
-        new_dev = self.pop_region_device(dev['region'])
+            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()