From 0a5268c34caa25487c48380a1821e4deac178538 Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Tue, 16 Sep 2014 14:46:08 +0000 Subject: [PATCH] Fix bug in swift-ring-builder list_parts The number of shown replicas in the partition list might differ from the actual number of replicas (as shown in the bugreport). This codes simply iterates for the builder._replica2part2dev and remembers the number of replicas for each partition. The code to find the partitions was moved to swift/common/ring/utils.py to make it easier to test, and a test to ensure the correct number of replicas is returned was added. Closes-Bug: 1370070 Change-Id: Id6a3ed437bb86df2f43f8b0b79aa8ccb50bbe13e --- swift/cli/ringbuilder.py | 26 ++++++++---------------- swift/common/ring/utils.py | 26 ++++++++++++++++++++++++ test/unit/common/ring/test_utils.py | 31 ++++++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 19 deletions(-) diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index 15de28fe77..42f9843f6b 100755 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -14,10 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from array import array from errno import EEXIST from itertools import islice, izip -from math import ceil from os import mkdir from os.path import basename, abspath, dirname, exists, join as pathjoin from sys import argv as sys_argv, exit, stderr @@ -29,7 +27,7 @@ from swift.common.ring import RingBuilder, Ring from swift.common.ring.builder import MAX_BALANCE from swift.common.utils import lock_parent_directory from swift.common.ring.utils import parse_search_value, parse_args, \ - build_dev_from_opts, parse_builder_ring_filename_args + build_dev_from_opts, parse_builder_ring_filename_args, find_parts MAJOR_VERSION = 1 MINOR_VERSION = 3 @@ -329,24 +327,16 @@ swift-ring-builder list_parts [] .. print print parse_search_value.__doc__.strip() exit(EXIT_ERROR) - devs = [] - for arg in argv[3:]: - devs.extend(builder.search_devs(parse_search_value(arg)) or []) - if not devs: + + sorted_partition_count = find_parts(builder, argv) + + if not sorted_partition_count: print 'No matching devices found' exit(EXIT_ERROR) - devs = [d['id'] for d in devs] - max_replicas = int(ceil(builder.replicas)) - matches = [array('i') for x in xrange(max_replicas)] - for part in xrange(builder.parts): - count = len([d for d in builder.get_part_devices(part) - if d['id'] in devs]) - if count: - matches[max_replicas - count].append(part) + print 'Partition Matches' - for index, parts in enumerate(matches): - for part in parts: - print '%9d %7d' % (part, max_replicas - index) + for partition, count in sorted_partition_count: + print '%9d %7d' % (partition, count) exit(EXIT_SUCCESS) def add(): diff --git a/swift/common/ring/utils.py b/swift/common/ring/utils.py index 1d795a4f49..f93326cc17 100644 --- a/swift/common/ring/utils.py +++ b/swift/common/ring/utils.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. from collections import defaultdict +from operator import itemgetter import optparse @@ -305,3 +306,28 @@ def build_dev_from_opts(opts): 'port': opts.port, 'device': opts.device, 'meta': opts.meta, 'replication_ip': replication_ip, 'replication_port': replication_port, 'weight': opts.weight} + + +def find_parts(builder, argv): + devs = [] + for arg in argv[3:]: + devs.extend(builder.search_devs(parse_search_value(arg)) or []) + + devs = [d['id'] for d in devs] + + if not devs: + return None + + partition_count = {} + for replica in builder._replica2part2dev: + for partition, device in enumerate(replica): + if device in devs: + if partition not in partition_count: + partition_count[partition] = 0 + partition_count[partition] += 1 + + # Sort by number of found replicas to keep the output format + sorted_partition_count = sorted( + partition_count.iteritems(), key=itemgetter(1), reverse=True) + + return sorted_partition_count diff --git a/test/unit/common/ring/test_utils.py b/test/unit/common/ring/test_utils.py index 849bf6383c..2a9cbfcf59 100644 --- a/test/unit/common/ring/test_utils.py +++ b/test/unit/common/ring/test_utils.py @@ -15,9 +15,10 @@ import unittest +from swift.common import ring from swift.common.ring.utils import (build_tier_tree, tiers_for_dev, parse_search_value, parse_args, - build_dev_from_opts, + build_dev_from_opts, find_parts, parse_builder_ring_filename_args) @@ -159,6 +160,34 @@ class TestUtils(unittest.TestCase): 'my.file.name', 'my.file.name.ring.gz' ), parse_builder_ring_filename_args(args.split())) + def test_find_parts(self): + rb = ring.RingBuilder(8, 3, 0) + rb.add_dev({'id': 0, 'region': 1, 'zone': 0, 'weight': 100, + 'ip': '127.0.0.1', 'port': 10000, 'device': 'sda1'}) + rb.add_dev({'id': 1, 'region': 1, 'zone': 1, 'weight': 100, + 'ip': '127.0.0.1', 'port': 10001, 'device': 'sda1'}) + rb.add_dev({'id': 2, 'region': 1, 'zone': 2, 'weight': 100, + 'ip': '127.0.0.1', 'port': 10002, 'device': 'sda1'}) + rb.rebalance() + + rb.add_dev({'id': 3, 'region': 2, 'zone': 1, 'weight': 10, + 'ip': '127.0.0.1', 'port': 10004, 'device': 'sda1'}) + rb.pretend_min_part_hours_passed() + rb.rebalance() + + argv = ['swift-ring-builder', 'object.builder', + 'list_parts', '127.0.0.1'] + sorted_partition_count = find_parts(rb, argv) + + # Expect 256 partitions in the output + self.assertEqual(256, len(sorted_partition_count)) + + # Each partitions should have 3 replicas + for partition, count in sorted_partition_count: + self.assertEqual( + 3, count, "Partition %d has only %d replicas" % + (partition, count)) + if __name__ == '__main__': unittest.main()