)]}'
{"swift/common/ring/builder.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"9113ac9e21ff507b59edce5755cf372ed6962ca1","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"                key\u003dlambda dr: dr[0][\u0027parts_wanted\u0027])"},{"line_number":1066,"context_line":"            for dev, replica in overweight_dev_replica:"},{"line_number":1067,"context_line":"                if (not self._can_part_move(part)):"},{"line_number":1068,"context_line":"                    break"},{"line_number":1069,"context_line":"                if any(replica_plan[tier][\u0027min\u0027] \u003c\u003d"},{"line_number":1070,"context_line":"                       replicas_at_tier[tier] \u003c"},{"line_number":1071,"context_line":"                       replica_plan[tier][\u0027max\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_20e835e2","side":"PARENT","line":1068,"updated":"2017-01-16 09:36:21.000000000","message":"It looks correct, because L1044 already asserts same thing and part is never re-assigned after that so we can remove the if condition here to skip.","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"9113ac9e21ff507b59edce5755cf372ed6962ca1","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                key\u003dlambda dr: dr[0][\u0027parts_wanted\u0027])"},{"line_number":1142,"context_line":"            for dev, replica in overweight_dev_replica:"},{"line_number":1143,"context_line":"                if (not self._can_part_move(part)):"},{"line_number":1144,"context_line":"                    break"},{"line_number":1145,"context_line":"                # this is the most overweight_device holding a replica of this"},{"line_number":1146,"context_line":"                # part we don\u0027t know where it\u0027s going to end up - but we\u0027ll"},{"line_number":1147,"context_line":"                # pick it up and hope for the best."}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_8002c909","side":"PARENT","line":1144,"updated":"2017-01-16 09:36:21.000000000","message":"Ah, this is duplication too with L1126. Make sense why nothing failed when re-adding this two lines for making sure the behavior.","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"9113ac9e21ff507b59edce5755cf372ed6962ca1","unresolved":false,"context_lines":[{"line_number":1150,"context_line":"                    part, replica, dev[\u0027id\u0027])"},{"line_number":1151,"context_line":"                self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":1152,"context_line":"                self._set_part_moved(part)"},{"line_number":1153,"context_line":"                break"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"    def _reassign_parts(self, reassign_parts, replica_plan):"},{"line_number":1156,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_6a277bb6","line":1153,"updated":"2017-01-16 09:36:21.000000000","message":"why not removing for loop here?","commit_id":"244e6400051dd45f905d09de5aa93af5cc51f686"}],"test/unit/common/ring/test_builder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef230070473f45a56f0f7c7bd899c3ff603ac927","unresolved":false,"context_lines":[{"line_number":734,"context_line":"            for replica in range(rb.replicas):"},{"line_number":735,"context_line":"                zones.add(rb.devs[rb._replica2part2dev[replica][part]][\u0027zone\u0027])"},{"line_number":736,"context_line":"            moved_replicas \u003d len(zones - {0, 1})"},{"line_number":737,"context_line":"            parts_with_moved_count[moved_replicas] +\u003d 1"},{"line_number":738,"context_line":""},{"line_number":739,"context_line":"        # We expect that every partition moved exactly one replica"},{"line_number":740,"context_line":"        expected \u003d {1: 256}"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_1bcd3585","side":"PARENT","line":737,"updated":"2017-01-11 20:10:16.000000000","message":"the idea here I suppose was that no matter how many part replicas were/are in zone 0 or 1 (one of them *must* have had two?) now at least *one* part replica is in zone *not* {0, 1}\n\nbetter we can count the number of parts that are in *two* zones not {0, 1}!\n\nI could be thinking about it wrong since I think I might have wrote this test :\\","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"585c4e2e818ca4bef2781b45360e2872aec19009","unresolved":false,"context_lines":[{"line_number":734,"context_line":"            for replica in range(rb.replicas):"},{"line_number":735,"context_line":"                zones.add(rb.devs[rb._replica2part2dev[replica][part]][\u0027zone\u0027])"},{"line_number":736,"context_line":"            moved_replicas \u003d len(zones - {0, 1})"},{"line_number":737,"context_line":"            parts_with_moved_count[moved_replicas] +\u003d 1"},{"line_number":738,"context_line":""},{"line_number":739,"context_line":"        # We expect that every partition moved exactly one replica"},{"line_number":740,"context_line":"        expected \u003d {1: 256}"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_214c8ac7","side":"PARENT","line":737,"in_reply_to":"ba5201f7_1bcd3585","updated":"2017-01-11 20:56:03.000000000","message":"IDK, I found it weird to try to reason about, in part no small part because of the set-difference. Maybe next patchset is something we\u0027ll both like more?","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef230070473f45a56f0f7c7bd899c3ff603ac927","unresolved":false,"context_lines":[{"line_number":738,"context_line":""},{"line_number":739,"context_line":"        # We expect that every partition moved exactly one replica"},{"line_number":740,"context_line":"        expected \u003d {1: 256}"},{"line_number":741,"context_line":"        self.assertEqual(parts_with_moved_count, expected)"},{"line_number":742,"context_line":""},{"line_number":743,"context_line":"        # After rebalancing two more times, we expect that everything is in a"},{"line_number":744,"context_line":"        # good state"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_db372d4b","side":"PARENT","line":741,"updated":"2017-01-11 20:10:16.000000000","message":"I think this assertion is sorta strong, and the failure when you have a few parts that moved too many part_replicas to different zones is gunna look pretty good :\\","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef230070473f45a56f0f7c7bd899c3ff603ac927","unresolved":false,"context_lines":[{"line_number":742,"context_line":""},{"line_number":743,"context_line":"        # After rebalancing two more times, we expect that everything is in a"},{"line_number":744,"context_line":"        # good state"},{"line_number":745,"context_line":"        rb.rebalance(seed\u003d3)"},{"line_number":746,"context_line":"        rb.rebalance(seed\u003d3)"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"        self.assertEqual(0, rb.dispersion)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_fb4cf143","side":"PARENT","line":745,"updated":"2017-01-11 20:10:16.000000000","message":"ah, so *this* is the extra rebalance that get\u0027s dropped - sweet - faster tests!\n\nRan it in a loop with random seeds and it seems fine!  Maybe the idea was just that after three rebalance every replica that might want to move had a chance.  There\u0027s probably some invariant I\u0027m missing that says it should only take one more...\n\nI guess I sort of assumed *someone* would want to end up with a replica in just {2, 3, 4} - /shrug","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"585c4e2e818ca4bef2781b45360e2872aec19009","unresolved":false,"context_lines":[{"line_number":742,"context_line":""},{"line_number":743,"context_line":"        # After rebalancing two more times, we expect that everything is in a"},{"line_number":744,"context_line":"        # good state"},{"line_number":745,"context_line":"        rb.rebalance(seed\u003d3)"},{"line_number":746,"context_line":"        rb.rebalance(seed\u003d3)"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"        self.assertEqual(0, rb.dispersion)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_41534e39","side":"PARENT","line":745,"in_reply_to":"ba5201f7_fb4cf143","updated":"2017-01-11 20:56:03.000000000","message":"We\u0027ve got enough devices that we can be happy without ever needing to move all three replicas, so inertia dictates that one should stay.","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef230070473f45a56f0f7c7bd899c3ff603ac927","unresolved":false,"context_lines":[{"line_number":787,"context_line":"            1: 52,"},{"line_number":788,"context_line":"            2: 204,"},{"line_number":789,"context_line":"        }"},{"line_number":790,"context_line":"        self.assertEqual(parts_with_moved_count, expected)"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"    def test_rerebalance(self):"},{"line_number":793,"context_line":"        rb \u003d ring.RingBuilder(8, 3, 1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_266020cf","side":"PARENT","line":790,"updated":"2017-01-11 20:10:16.000000000","message":"hrmm... that same pattern is used again here - and yeah... man I don\u0027t know - seems like a fairly strong story /shrug","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef230070473f45a56f0f7c7bd899c3ff603ac927","unresolved":false,"context_lines":[{"line_number":738,"context_line":"                zones.add(rb.devs[rb._replica2part2dev[replica][part]][\u0027zone\u0027])"},{"line_number":739,"context_line":"            self.assertEqual(3, len(zones))"},{"line_number":740,"context_line":"            self.assertIn(0, zones)"},{"line_number":741,"context_line":"            self.assertIn(1, zones)"},{"line_number":742,"context_line":""},{"line_number":743,"context_line":"        # After rebalancing again, we expect that everything is in a good state"},{"line_number":744,"context_line":"        rb.rebalance(seed\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_9bbcc5bc","line":741,"updated":"2017-01-11 20:10:16.000000000","message":"having the asserts in the loop here would bust you out at the first failure - a lot of times working on rings you need to see things at an overview level to understand why this one part was wrong.\n\nWas it the only one?  Is the issue systemic?  Is it exactly one replicanth?  etc.","commit_id":"4a2d569e68a306c4763e504bb1b61de74cd29f17"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"585c4e2e818ca4bef2781b45360e2872aec19009","unresolved":false,"context_lines":[{"line_number":738,"context_line":"                zones.add(rb.devs[rb._replica2part2dev[replica][part]][\u0027zone\u0027])"},{"line_number":739,"context_line":"            self.assertEqual(3, len(zones))"},{"line_number":740,"context_line":"            self.assertIn(0, zones)"},{"line_number":741,"context_line":"            self.assertIn(1, zones)"},{"line_number":742,"context_line":""},{"line_number":743,"context_line":"        # After rebalancing again, we expect that everything is in a good state"},{"line_number":744,"context_line":"        rb.rebalance(seed\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_61893226","line":741,"in_reply_to":"ba5201f7_9bbcc5bc","updated":"2017-01-11 20:56:03.000000000","message":"Fair.","commit_id":"4a2d569e68a306c4763e504bb1b61de74cd29f17"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf467c0b773668a2ad260a285cb73f0c47b59677","unresolved":false,"context_lines":[{"line_number":725,"context_line":"        # There was only zone 0 and 1 before adding more devices. Only one"},{"line_number":726,"context_line":"        # replica should have been moved, therefore we expect 256 parts in zone"},{"line_number":727,"context_line":"        # 0 and 1, and a total of 256 in zone 2,3, and 4"},{"line_number":728,"context_line":"        expected \u003d defaultdict(int, {0: 256, 1: 256, 2: 86, 3: 85, 4: 85})"},{"line_number":729,"context_line":"        self.assertEqual(expected, self._partition_counts(rb, key\u003d\u0027zone\u0027))"},{"line_number":730,"context_line":""},{"line_number":731,"context_line":"        parts_with_moved_count \u003d defaultdict(int)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5201f7_6c05a978","side":"PARENT","line":728,"updated":"2017-01-11 22:15:19.000000000","message":"nice, it\u0027s like re-enforcing that these part replicas in zones 2, 3, 4 are for different parts and still have two part_replicas back in zone 0, 1!","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf467c0b773668a2ad260a285cb73f0c47b59677","unresolved":false,"context_lines":[{"line_number":741,"context_line":"            (0, 1, 3): 85,"},{"line_number":742,"context_line":"            (0, 1, 4): 85,"},{"line_number":743,"context_line":"        }"},{"line_number":744,"context_line":"        self.assertEqual(zone_histogram, expected)"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        # After rebalancing two more times, we expect that everything is in a"},{"line_number":747,"context_line":"        # good state"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5201f7_6cbbe9e5","line":744,"updated":"2017-01-11 22:15:19.000000000","message":"yeah... i mean that\u0027s beautiful :D","commit_id":"244e6400051dd45f905d09de5aa93af5cc51f686"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"9113ac9e21ff507b59edce5755cf372ed6962ca1","unresolved":false,"context_lines":[{"line_number":743,"context_line":"        }"},{"line_number":744,"context_line":"        self.assertEqual(zone_histogram, expected)"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        # After rebalancing two more times, we expect that everything is in a"},{"line_number":747,"context_line":"        # good state"},{"line_number":748,"context_line":"        rb.rebalance(seed\u003d3)"},{"line_number":749,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_458553ab","line":746,"range":{"start_line":746,"start_character":28,"end_line":746,"end_character":42},"updated":"2017-01-16 09:36:21.000000000","message":"nits) With this change, that\u0027s not two more times but one more time, is it?","commit_id":"244e6400051dd45f905d09de5aa93af5cc51f686"}]}