Skip to content

Commit

Permalink
Make RingBuilders deep-copy-able
Browse files Browse the repository at this point in the history
We used to be able to deep-copy RingBuilder objects, but the addition
of debug logging (8d3b3b2) broke that since you can't deep-copy a
Python logger. This commit fixes that.

Swift doesn't really deep-copy RingBuilders anywhere, but third-party
code might.

Change-Id: If8bdadd93d9980db3d8a093f32d76ca604de9301
  • Loading branch information
smerritt committed Apr 22, 2015
1 parent ad66801 commit 43ace3c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 2 deletions.
3 changes: 1 addition & 2 deletions swift/cli/ringbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1073,8 +1073,7 @@ def write_builder():
'_last_part_gather_start': 0,
'_remove_devs': [],
}
builder = RingBuilder(1, 1, 1)
builder.copy_from(builder_dict)
builder = RingBuilder.from_dict(builder_dict)
for parts in builder._replica2part2dev:
for dev_id in parts:
builder.devs[dev_id]['parts'] += 1
Expand Down
12 changes: 12 additions & 0 deletions swift/common/ring/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import math
import random
import cPickle as pickle
from copy import deepcopy

from array import array
from collections import defaultdict
Expand Down Expand Up @@ -125,6 +126,12 @@ def weight_of_one_part(self):
'ring, or all devices have been '
'deleted')

@classmethod
def from_dict(cls, builder_data):
b = cls(1, 1, 1) # Dummy values
b.copy_from(builder_data)
return b

def copy_from(self, builder):
"""
Reinitializes this RingBuilder instance from data obtained from the
Expand Down Expand Up @@ -173,6 +180,11 @@ def copy_from(self, builder):
for dev in self._iter_devs():
dev.setdefault("region", 1)

def __deepcopy__(self, memo):
the_copy = type(self).from_dict(deepcopy(self.to_dict(), memo))
memo[id(self)] = the_copy
return the_copy

def to_dict(self):
"""
Returns a dict that can be used later with copy_from to
Expand Down
21 changes: 21 additions & 0 deletions test/unit/common/ring/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import copy
import errno
import mock
import operator
Expand Down Expand Up @@ -84,6 +85,26 @@ def test_negative_min_part_hours(self):
ring.RingBuilder(8, 3, 0) # passes by not crashing
self.assertRaises(ValueError, ring.RingBuilder, 8, 3, -1)

def test_deepcopy(self):
rb = ring.RingBuilder(8, 3, 1)
rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,
'ip': '127.0.0.1', 'port': 10000, 'device': 'sda1'})
rb.add_dev({'id': 1, 'region': 0, 'zone': 1, 'weight': 1,
'ip': '127.0.0.1', 'port': 10001, 'device': 'sda1'})
rb.add_dev({'id': 2, 'region': 0, 'zone': 2, 'weight': 1,
'ip': '127.0.0.1', 'port': 10002, 'device': 'sda1'})
rb.add_dev({'id': 3, 'region': 0, 'zone': 1, 'weight': 1,
'ip': '127.0.0.1', 'port': 10004, 'device': 'sda1'})
rb.rebalance()
rb_copy = copy.deepcopy(rb)

self.assertEqual(rb.to_dict(), rb_copy.to_dict())
self.assertTrue(rb.devs is not rb_copy.devs)
self.assertTrue(rb._replica2part2dev is not rb_copy._replica2part2dev)
self.assertTrue(rb._last_part_moves is not rb_copy._last_part_moves)
self.assertTrue(rb._remove_devs is not rb_copy._remove_devs)
self.assertTrue(rb._dispersion_graph is not rb_copy._dispersion_graph)

def test_get_ring(self):
rb = ring.RingBuilder(8, 3, 1)
rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,
Expand Down

0 comments on commit 43ace3c

Please sign in to comment.