)]}'
{"swift/common/ring/builder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c1fe767bb0e492098bf6be6d584d97e301f3fbf9","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def _set_part_moved_bit(self, part):"},{"line_number":131,"context_line":"        byte, bit \u003d divmod(part, 8)"},{"line_number":132,"context_line":"        self._part_moved_bitmap[byte] |\u003d (128 \u003e\u003e bit)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def _get_part_moved_bit(self, part):"},{"line_number":135,"context_line":"        byte, bit \u003d divmod(part, 8)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_3c733782","line":132,"updated":"2016-07-28 21:09:28.000000000","message":"I think the `_bit` part of this name is more of an implementation detail\n\nIt should just be named just `_set_part_moved` [1]\n\nhaving `bit` in the name is unneeded cognitive load and would introduce extra maintenance burden or cruft if we refactor the implementation.  I think a more abstract name may also encourage future us to stay outside of the implementation and stick to the interface.\n\n1. ... or maybe even `_part_set_moved` if you go with `_part_has_moved` ?","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1fad312fc8dacdb9ce138238ec7fff59738ccbb1","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def _set_part_moved_bit(self, part):"},{"line_number":131,"context_line":"        byte, bit \u003d divmod(part, 8)"},{"line_number":132,"context_line":"        self._part_moved_bitmap[byte] |\u003d (128 \u003e\u003e bit)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def _get_part_moved_bit(self, part):"},{"line_number":135,"context_line":"        byte, bit \u003d divmod(part, 8)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_c0858907","line":132,"in_reply_to":"bacf61ea_3c733782","updated":"2016-07-30 11:44:15.000000000","message":"Great, agreed.","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c1fe767bb0e492098bf6be6d584d97e301f3fbf9","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        byte, bit \u003d divmod(part, 8)"},{"line_number":132,"context_line":"        self._part_moved_bitmap[byte] |\u003d (128 \u003e\u003e bit)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def _get_part_moved_bit(self, part):"},{"line_number":135,"context_line":"        byte, bit \u003d divmod(part, 8)"},{"line_number":136,"context_line":"        return bool(self._part_moved_bitmap[byte] \u0026 (128 \u003e\u003e bit))"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_9c2fc33d","line":134,"updated":"2016-07-28 21:09:28.000000000","message":"... and `_has_part_moved` or `_get_part_moved` or `_part_has_moved` or something like that...","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"62e5abe028da5e3cf99afedd39a394c2a20bc031","unresolved":false,"context_lines":[{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"        for part, replace_replicas in reassign_parts:"},{"line_number":1234,"context_line":"            # always update part_moves for min_part_hours"},{"line_number":1235,"context_line":"            self._last_part_moves[part] \u003d 0"},{"line_number":1236,"context_line":"            # count up where these replicas be"},{"line_number":1237,"context_line":"            replicas_at_tier \u003d defaultdict(int)"},{"line_number":1238,"context_line":"            for dev in self._devs_for_part(part):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_5f0c2694","line":1235,"updated":"2016-07-25 08:08:19.000000000","message":"I\u0027m wondering if we should call self._set_part_moved_bit(part) here as well (or only here?). Actually this is the place where the partition movement really happens, and a partition is assigned to a new device.","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"b4f7a43619535c131f161133806cd8d0f8257e1d","unresolved":false,"context_lines":[{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"        for part, replace_replicas in reassign_parts:"},{"line_number":1234,"context_line":"            # always update part_moves for min_part_hours"},{"line_number":1235,"context_line":"            self._last_part_moves[part] \u003d 0"},{"line_number":1236,"context_line":"            # count up where these replicas be"},{"line_number":1237,"context_line":"            replicas_at_tier \u003d defaultdict(int)"},{"line_number":1238,"context_line":"            for dev in self._devs_for_part(part):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_c8b5f9d0","line":1235,"in_reply_to":"dada55a8_5f0c2694","updated":"2016-07-25 09:34:19.000000000","message":"I think here is not the right place to call self._set_part_moved_bit(part).\n\nWe should set bit as soon as possible, because this will affect self._get_part_moved_bit(part), which be called many times before _reassign_parts.","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"53d2c62d2cfb0603d76776ece84feedb130396dd","unresolved":false,"context_lines":[{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"        for part, replace_replicas in reassign_parts:"},{"line_number":1234,"context_line":"            # always update part_moves for min_part_hours"},{"line_number":1235,"context_line":"            self._last_part_moves[part] \u003d 0"},{"line_number":1236,"context_line":"            # count up where these replicas be"},{"line_number":1237,"context_line":"            replicas_at_tier \u003d defaultdict(int)"},{"line_number":1238,"context_line":"            for dev in self._devs_for_part(part):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_57a3619e","line":1235,"in_reply_to":"dada55a8_c8b5f9d0","updated":"2016-07-25 14:28:07.000000000","message":"Agreed, it will never be evaluated here with the current codebase.","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e2877cb3487878f4f22acd7c1bff851dcd5b4cd7","unresolved":false,"context_lines":[{"line_number":447,"context_line":"        if self._last_part_moves is None:"},{"line_number":448,"context_line":"            self.logger.debug(\"New builder; performing initial balance\")"},{"line_number":449,"context_line":"            self._last_part_moves \u003d array(\u0027B\u0027, itertools.repeat(0, self.parts))"},{"line_number":450,"context_line":"        self._part_moved_bitmap \u003d bytearray((self.parts + 7) // 8)"},{"line_number":451,"context_line":"        self._update_last_part_moves()"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"        replica_plan \u003d self._build_replica_plan()"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ac06dbe_73406708","line":450,"updated":"2016-08-24 23:00:05.000000000","message":"so we\u0027re trying to make enough byte size buckets to have a bit for every part right?  We know our part_power, and parts is just 2 ** part_power\n\nI think wolfram alpha says [1] this is more easily described as:\n\n    2 ** (part_power - 3)\n\npresumably this has something to do with 2 ** 3 \u003d\u003d 8\n\n1. https://www.wolframalpha.com/input/?i\u003d2%5Ex+%3D+y+*+8","commit_id":"d75e3fc1fb003918561a72e511dff400c9b0fba3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e2877cb3487878f4f22acd7c1bff851dcd5b4cd7","unresolved":false,"context_lines":[{"line_number":888,"context_line":"                    if dev_id in dev_ids:"},{"line_number":889,"context_line":"                        self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":890,"context_line":"                        self._last_part_moves[part] \u003d 0"},{"line_number":891,"context_line":"                        self._set_part_moved(part)"},{"line_number":892,"context_line":"                        assign_parts[part].append(replica)"},{"line_number":893,"context_line":"                        self.logger.debug("},{"line_number":894,"context_line":"                            \"Gathered %d/%d from dev %d [dev removed]\","}],"source_content_type":"text/x-python","patch_set":6,"id":"1ac06dbe_7b3fa3d6","line":891,"updated":"2016-08-24 23:00:05.000000000","message":"it seems annoying to have to update both data structures and only one is abstracted?  What if we just called `set_part_moved` and had it also take care of `last_part_moves` and any other state we may need to encapsulate for this action?","commit_id":"d75e3fc1fb003918561a72e511dff400c9b0fba3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e2877cb3487878f4f22acd7c1bff851dcd5b4cd7","unresolved":false,"context_lines":[{"line_number":977,"context_line":"        # currently sufficient spread out across the cluster."},{"line_number":978,"context_line":"        for part in range(self.parts):"},{"line_number":979,"context_line":"            if (self._last_part_moves[part] \u003c self.min_part_hours or"},{"line_number":980,"context_line":"                    self._has_part_moved(part)):"},{"line_number":981,"context_line":"                continue"},{"line_number":982,"context_line":"            # First, add up the count of replicas at each tier for each"},{"line_number":983,"context_line":"            # partition."}],"source_content_type":"text/x-python","patch_set":6,"id":"1ac06dbe_164be65a","line":980,"updated":"2016-08-24 23:00:05.000000000","message":"same reasoning here, we shouldn\u0027t ever check one with out the other - let\u0027s put them together in a `_can_part_move(part)` or similar helper?","commit_id":"d75e3fc1fb003918561a72e511dff400c9b0fba3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f37d2602da1392d52b118de9c3928e46b8e56d36","unresolved":false,"context_lines":[{"line_number":130,"context_line":"    def _set_part_moved(self, part):"},{"line_number":131,"context_line":"        self._last_part_moves[part] \u003d 0"},{"line_number":132,"context_line":"        byte, bit \u003d divmod(part, 8)"},{"line_number":133,"context_line":"        self._part_moved_bitmap[byte] |\u003d (128 \u003e\u003e bit)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def _has_part_moved(self, part):"},{"line_number":136,"context_line":"        byte, bit \u003d divmod(part, 8)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5201f7_1b1e1cbb","line":133,"range":{"start_line":133,"start_character":42,"end_line":133,"end_character":52},"updated":"2017-01-11 19:22:13.000000000","message":"I might\u0027ve gone the other way (1 \u003c\u003c bit) but it doesn\u0027t matter at all. 128 just feels weird to me :P","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    def _can_part_move(self, part):"},{"line_number":140,"context_line":"        return (self._last_part_moves[part] \u003e\u003d self.min_part_hours and"},{"line_number":141,"context_line":"                not self._has_part_moved(part))"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    @contextmanager"},{"line_number":144,"context_line":"    def debug(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_0b7d31e7","line":141,"updated":"2016-12-02 12:10:46.000000000","message":"these are pretty good acctually!","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        if self._last_part_moves is None:"},{"line_number":453,"context_line":"            self.logger.debug(\"New builder; performing initial balance\")"},{"line_number":454,"context_line":"            self._last_part_moves \u003d array(\u0027B\u0027, itertools.repeat(0, self.parts))"},{"line_number":455,"context_line":"        self._part_moved_bitmap \u003d bytearray(max(2 ** (self.part_power - 3), 1))"},{"line_number":456,"context_line":"        self._update_last_part_moves()"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        replica_plan \u003d self._build_replica_plan()"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_abab5d1c","line":455,"updated":"2016-12-02 12:10:46.000000000","message":"oh wow, it\u0027s a bytearray - that\u0027s fancy dense\n\nso 2 ** 3 is like 8 or something right?  So somehow that\u0027s the number of bytes we need to map a 2 ** part_power bits - yeah that\u0027s probably right.  sam or tim or christian would know for sure.","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"cf2f012854d7981b345ca76beb5e02fc7daf53b6","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        if self._last_part_moves is None:"},{"line_number":453,"context_line":"            self.logger.debug(\"New builder; performing initial balance\")"},{"line_number":454,"context_line":"            self._last_part_moves \u003d array(\u0027B\u0027, itertools.repeat(0, self.parts))"},{"line_number":455,"context_line":"        self._part_moved_bitmap \u003d bytearray(max(2 ** (self.part_power - 3), 1))"},{"line_number":456,"context_line":"        self._update_last_part_moves()"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        replica_plan \u003d self._build_replica_plan()"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_f5927d4a","line":455,"in_reply_to":"3a71b18c_abab5d1c","updated":"2016-12-03 02:49:10.000000000","message":"yes, 1byte \u003d 8 bit \u003d 2 ** 3 bit","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f37d2602da1392d52b118de9c3928e46b8e56d36","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        if self._last_part_moves is None:"},{"line_number":453,"context_line":"            self.logger.debug(\"New builder; performing initial balance\")"},{"line_number":454,"context_line":"            self._last_part_moves \u003d array(\u0027B\u0027, itertools.repeat(0, self.parts))"},{"line_number":455,"context_line":"        self._part_moved_bitmap \u003d bytearray(max(2 ** (self.part_power - 3), 1))"},{"line_number":456,"context_line":"        self._update_last_part_moves()"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        replica_plan \u003d self._build_replica_plan()"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5201f7_9bda0c4a","line":455,"in_reply_to":"3a71b18c_f5927d4a","updated":"2017-01-11 19:22:13.000000000","message":"Good call on the max(..., 1)","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":499,"context_line":"        self.version +\u003d 1"},{"line_number":500,"context_line":"        changed_parts \u003d self._build_dispersion_graph(old_replica2part2dev)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"        # clean up the cache"},{"line_number":503,"context_line":"        for dev in self._iter_devs():"},{"line_number":504,"context_line":"            dev.pop(\u0027tiers\u0027, None)"},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_d17da6b2","line":502,"updated":"2016-12-02 12:10:46.000000000","message":"i think this would be a good idea\n\n\tdiff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py\n\tindex 2e526ab..41f9720 100644\n\t--- a/swift/common/ring/builder.py\n\t+++ b/swift/common/ring/builder.py\n\t@@ -500,6 +500,7 @@ class RingBuilder(object):\n\t\t changed_parts \u003d self._build_dispersion_graph(old_replica2part2dev)\n\t \n\t\t # clean up the cache\n\t+        self._part_moved_bitmap \u003d None\n\t\t for dev in self._iter_devs():\n\t\t     dev.pop(\u0027tiers\u0027, None)","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"cf2f012854d7981b345ca76beb5e02fc7daf53b6","unresolved":false,"context_lines":[{"line_number":499,"context_line":"        self.version +\u003d 1"},{"line_number":500,"context_line":"        changed_parts \u003d self._build_dispersion_graph(old_replica2part2dev)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"        # clean up the cache"},{"line_number":503,"context_line":"        for dev in self._iter_devs():"},{"line_number":504,"context_line":"            dev.pop(\u0027tiers\u0027, None)"},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_753bcd38","line":502,"in_reply_to":"3a71b18c_d17da6b2","updated":"2016-12-03 02:49:10.000000000","message":"sure","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":892,"context_line":"                    dev_id \u003d self._replica2part2dev[replica][part]"},{"line_number":893,"context_line":"                    if dev_id in dev_ids:"},{"line_number":894,"context_line":"                        self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":895,"context_line":"                        self._set_part_moved(part)"},{"line_number":896,"context_line":"                        assign_parts[part].append(replica)"},{"line_number":897,"context_line":"                        self.logger.debug("},{"line_number":898,"context_line":"                            \"Gathered %d/%d from dev %d [dev removed]\","}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_8b7301e7","line":895,"updated":"2016-12-02 12:10:46.000000000","message":"that acctually looks great - love keeping all the bookkeeping behind this interface - very clear","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":980,"context_line":"        # Now we gather partitions that are \"at risk\" because they aren\u0027t"},{"line_number":981,"context_line":"        # currently sufficient spread out across the cluster."},{"line_number":982,"context_line":"        for part in range(self.parts):"},{"line_number":983,"context_line":"            if (not self._can_part_move(part)):"},{"line_number":984,"context_line":"                continue"},{"line_number":985,"context_line":"            # First, add up the count of replicas at each tier for each"},{"line_number":986,"context_line":"            # partition."}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_eb4ab582","line":983,"updated":"2016-12-02 12:10:46.000000000","message":"same, kudos","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f37d2602da1392d52b118de9c3928e46b8e56d36","unresolved":false,"context_lines":[{"line_number":1083,"context_line":"                self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":1084,"context_line":"                for tier in dev[\u0027tiers\u0027]:"},{"line_number":1085,"context_line":"                    replicas_at_tier[tier] -\u003d 1"},{"line_number":1086,"context_line":"                self._set_part_moved(part)"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"    def _gather_parts_for_balance(self, assign_parts, replica_plan):"},{"line_number":1089,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5201f7_767add89","line":1086,"updated":"2017-01-11 19:22:13.000000000","message":"Not really related to this change, but I wonder if we should just break immediately, rather than waiting for the next iteration and checking\n\n not self._can_part_move(part)\n\nabove.","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b7e893436db3b3fb3d8969ad8bfed63fa119a8d1","unresolved":false,"context_lines":[{"line_number":1083,"context_line":"                self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":1084,"context_line":"                for tier in dev[\u0027tiers\u0027]:"},{"line_number":1085,"context_line":"                    replicas_at_tier[tier] -\u003d 1"},{"line_number":1086,"context_line":"                self._set_part_moved(part)"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"    def _gather_parts_for_balance(self, assign_parts, replica_plan):"},{"line_number":1089,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5201f7_b8098f86","line":1086,"in_reply_to":"ba5201f7_767add89","updated":"2017-01-11 19:37:19.000000000","message":"it\u0027s *totally* related to this change!  the reason it\u0027s so stupid currently is because 0 \u003c 0 is false so the code had to not break to allow for this stupid behavior!\n\nstupid min_part_hours \u003d 0\n\nIt\u0027s going to look ever more obviously useless after this diff merges; but it\u0027s certainly non-material","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f37d2602da1392d52b118de9c3928e46b8e56d36","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"                    \"Gathered %d/%d from dev %d [weight forced]\","},{"line_number":1153,"context_line":"                    part, replica, dev[\u0027id\u0027])"},{"line_number":1154,"context_line":"                self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":1155,"context_line":"                self._set_part_moved(part)"},{"line_number":1156,"context_line":""},{"line_number":1157,"context_line":"    def _reassign_parts(self, reassign_parts, replica_plan):"},{"line_number":1158,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5201f7_16e51137","line":1155,"updated":"2017-01-11 19:22:13.000000000","message":"Similar to above.","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":"        for part, replace_replicas in reassign_parts:"},{"line_number":1232,"context_line":"            # always update part_moves for min_part_hours"},{"line_number":1233,"context_line":"            self._last_part_moves[part] \u003d 0"},{"line_number":1234,"context_line":"            # count up where these replicas be"},{"line_number":1235,"context_line":"            replicas_at_tier \u003d defaultdict(int)"},{"line_number":1236,"context_line":"            for dev in self._devs_for_part(part):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_169f14b5","line":1233,"updated":"2016-12-02 12:10:46.000000000","message":"Is there a good reason this isn\u0027t _set_part_moved?  everything seems to work when I change it?\n\nI mean granted; I don\u0027t know why it\u0027s here in reassign parts in the first place since I think we mostly _set_part_moved when we pick them up... but if the assertion is this doesn\u0027t matter we should remove it ... if we think that\u0027s an investigation for later we update it.\n\n\tdiff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py\n\tindex 2e526ab..51197e3 100644\n\t--- a/swift/common/ring/builder.py\n\t+++ b/swift/common/ring/builder.py\n\t@@ -1229,8 +1229,7 @@ class RingBuilder(object):\n\t\t     depth +\u003d 1\n\t \n\t\t for part, replace_replicas in reassign_parts:\n\t-            # always update part_moves for min_part_hours\n\t-            self._last_part_moves[part] \u003d 0\n\t+            self._set_part_moved(part)\n\t\t     # count up where these replicas be\n\t\t     replicas_at_tier \u003d defaultdict(int)\n\t\t     for dev in self._devs_for_part(part):","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"cf2f012854d7981b345ca76beb5e02fc7daf53b6","unresolved":false,"context_lines":[{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":"        for part, replace_replicas in reassign_parts:"},{"line_number":1232,"context_line":"            # always update part_moves for min_part_hours"},{"line_number":1233,"context_line":"            self._last_part_moves[part] \u003d 0"},{"line_number":1234,"context_line":"            # count up where these replicas be"},{"line_number":1235,"context_line":"            replicas_at_tier \u003d defaultdict(int)"},{"line_number":1236,"context_line":"            for dev in self._devs_for_part(part):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_31223301","line":1233,"in_reply_to":"3a71b18c_169f14b5","updated":"2016-12-03 02:49:10.000000000","message":"Following is my thinking:\nWe should had _set_part_moved for partitions when we collected partitions, so we can assign them here.\nIf we didn\u0027t _set_part_moved when collecting parts for some reason, then we may neither do _set_part_moved here.","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"}],"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":"c1fe767bb0e492098bf6be6d584d97e301f3fbf9","unresolved":false,"context_lines":[{"line_number":716,"context_line":"        rb.validate()"},{"line_number":717,"context_line":"        self.assertEqual(0, rb.dispersion)"},{"line_number":718,"context_line":"        self.assertTrue(parts \u003c\u003d 2 ** 8)"},{"line_number":719,"context_line":"        self.assertTrue(balance \u003e 0)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"        parts, balance, removed_devs \u003d rb.rebalance(seed\u003d3)"},{"line_number":722,"context_line":"        rb.validate()"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_fcca3fa0","line":719,"updated":"2016-07-28 21:09:28.000000000","message":"I think this is fairly week assertion compared to the part move counts that were there before.  Ring tests are well know to be brittle - the idea is to make you think long an hard when you have to change a ring unittest - this test was specifically here to document the current thinking behind min_part_hours 0 movement.  We need a similarly *strong* test to document how we should be thinking about the new behavior.\n\n... I\u0027ll think about this","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1fad312fc8dacdb9ce138238ec7fff59738ccbb1","unresolved":false,"context_lines":[{"line_number":716,"context_line":"        rb.validate()"},{"line_number":717,"context_line":"        self.assertEqual(0, rb.dispersion)"},{"line_number":718,"context_line":"        self.assertTrue(parts \u003c\u003d 2 ** 8)"},{"line_number":719,"context_line":"        self.assertTrue(balance \u003e 0)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"        parts, balance, removed_devs \u003d rb.rebalance(seed\u003d3)"},{"line_number":722,"context_line":"        rb.validate()"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_808f1129","line":719,"in_reply_to":"bacf61ea_fcca3fa0","updated":"2016-07-30 11:44:15.000000000","message":"I am going to test parts moved in next patch","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":695,"context_line":"                    \"Partition %d not in zones 0 and 1 (got %r)\" %"},{"line_number":696,"context_line":"                    (part, zones))"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"    def test_min_part_hours_zero_will_move_one_replica(self):"},{"line_number":699,"context_line":"        rb \u003d ring.RingBuilder(8, 3, 0)"},{"line_number":700,"context_line":"        # there\u0027ll be at least one replica in z0 and z1"},{"line_number":701,"context_line":"        rb.add_dev({\u0027id\u0027: 0, \u0027region\u0027: 0, \u0027zone\u0027: 0, \u0027weight\u0027: 0.5,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_91924e9e","line":698,"updated":"2016-12-02 12:10:46.000000000","message":"ha, love it!","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"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)"},{"line_number":749,"context_line":"        # a balance of w/i a 1% isn\u0027t too bad for 3 replicas on 7"},{"line_number":750,"context_line":"        # devices when part power is only 8"},{"line_number":751,"context_line":"        self.assertAlmostEqual(rb.get_balance(), 0, delta\u003d0.5)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_f182a251","line":748,"updated":"2016-12-02 12:10:46.000000000","message":"this is fabulous","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"}],"test/unit/common/ring/test_utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c1fe767bb0e492098bf6be6d584d97e301f3fbf9","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        report \u003d dispersion_report(rb, verbose\u003dTrue)"},{"line_number":742,"context_line":"        self.assertEqual(rb.dispersion, 9.375)"},{"line_number":743,"context_line":"        self.assertEqual(report[\u0027worst_tier\u0027], \u0027r1z1-127.0.0.1\u0027)"},{"line_number":744,"context_line":"        self.assertEqual(report[\u0027max_dispersion\u0027], 7.18562874251497)"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        # ... but overload can square it"},{"line_number":747,"context_line":"        rb.set_overload(rb.get_required_overload())"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_bcee0789","line":744,"updated":"2016-07-28 21:09:28.000000000","message":"this doesn\u0027t make sense to me immediately?  this ring has min_part_hours of 0, and this change makes dispersion *better*?  Like *drastically* better?  If anything it should make it worse...","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1fad312fc8dacdb9ce138238ec7fff59738ccbb1","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        report \u003d dispersion_report(rb, verbose\u003dTrue)"},{"line_number":742,"context_line":"        self.assertEqual(rb.dispersion, 9.375)"},{"line_number":743,"context_line":"        self.assertEqual(report[\u0027worst_tier\u0027], \u0027r1z1-127.0.0.1\u0027)"},{"line_number":744,"context_line":"        self.assertEqual(report[\u0027max_dispersion\u0027], 7.18562874251497)"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        # ... but overload can square it"},{"line_number":747,"context_line":"        rb.set_overload(rb.get_required_overload())"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_609e5d64","line":744,"in_reply_to":"bacf61ea_bcee0789","updated":"2016-07-30 11:44:15.000000000","message":"Normally, the dispersion is good eventually, but it may take more than one times of rebalance to get it for min_part_hours of 0.\nthe process of multiple rebalance may looks like this:\ngood -\u003e bad -\u003e good\nSo it may seems bad after one rebalance.\n\nBut for this case, the eventually dispersion is bad, then the process is different. It\u0027s like,\ngood -\u003e bad -\u003e worse\n\nI am going to fix this in next patch","commit_id":"6910d046822671d8c0285a079bedefba46e323e9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        report \u003d dispersion_report(rb, verbose\u003dTrue)"},{"line_number":670,"context_line":"        self.assertEqual(rb.dispersion, 9.375)"},{"line_number":671,"context_line":"        self.assertEqual(report[\u0027worst_tier\u0027], \u0027r1z1-127.0.0.1\u0027)"},{"line_number":672,"context_line":"        self.assertEqual(report[\u0027max_dispersion\u0027], 7.18562874251497)"},{"line_number":673,"context_line":"        # do a sencond rebalance"},{"line_number":674,"context_line":"        rb.rebalance(seed\u003d100)"},{"line_number":675,"context_line":"        report \u003d dispersion_report(rb, verbose\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_a30f012d","line":672,"updated":"2016-12-02 12:10:46.000000000","message":"I guess I\u0027m starting to get it a little bit - this is the part where r1z0 would *over-compensate* with extra capacity but since it can only move one part replica - the problem stays back in r1z1 (just slightly better)\n\nthat could really use a comment.\n\nand why is rb.dispersion \u003e report[\u0027max_dispersion\u0027]  (?)","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"cf2f012854d7981b345ca76beb5e02fc7daf53b6","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        report \u003d dispersion_report(rb, verbose\u003dTrue)"},{"line_number":670,"context_line":"        self.assertEqual(rb.dispersion, 9.375)"},{"line_number":671,"context_line":"        self.assertEqual(report[\u0027worst_tier\u0027], \u0027r1z1-127.0.0.1\u0027)"},{"line_number":672,"context_line":"        self.assertEqual(report[\u0027max_dispersion\u0027], 7.18562874251497)"},{"line_number":673,"context_line":"        # do a sencond rebalance"},{"line_number":674,"context_line":"        rb.rebalance(seed\u003d100)"},{"line_number":675,"context_line":"        report \u003d dispersion_report(rb, verbose\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_ccb888e8","line":672,"in_reply_to":"3a71b18c_a30f012d","updated":"2016-12-03 02:49:10.000000000","message":"As I understand,\nrb.dispersion is the global dispersion:\nrb.dispersion \u003d all_risk_parts/all_parts\n\nWhile in dispersion_report, we calculate dispersion by tier,\ndispersion[tier1] \u003d risk_parts[tier1]/placed_parts[tier1]\ndispersion[tier2] \u003d risk_parts[tier2]/placed_parts[tier2]\n\nrisk_parts[tier1] + risk_parts[tier2] \u003d all_risk_parts\nbut,\nplaced_parts[tier1] + places_parts[tier2] \u003e\u003d all_parts\n\nbecause one part has 3 copies, they can be assigned to both tier1 and tier2.\n\nSo it could happen that\nrb.dispersion \u003e report[\u0027max_dispersion\u0027]","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"782e3875042d7a0000af9631cd74dbab326587fd","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        report \u003d dispersion_report(rb, verbose\u003dTrue)"},{"line_number":676,"context_line":"        self.assertEqual(rb.dispersion, 50.0)"},{"line_number":677,"context_line":"        self.assertEqual(report[\u0027worst_tier\u0027], \u0027r1z0-127.0.0.3\u0027)"},{"line_number":678,"context_line":"        self.assertEqual(report[\u0027max_dispersion\u0027], 50.0)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"        # ... but overload can square it"},{"line_number":681,"context_line":"        rb.set_overload(rb.get_required_overload())"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_ad9fea4e","line":678,"updated":"2016-12-02 12:10:46.000000000","message":"and I guess it will only ever get this bad?\n\n\tDispersion is 50.000000, Balance is 1.562500, Overload is 0.00%\n\tRequired overload is 166.666667%\n\tWorst tier is 50.000000 (r1z0-127.0.0.3)\n\t--------------------------------------------------------------------------\n\tTier                           Parts      %    Max     0     1     2     3\n\t--------------------------------------------------------------------------\n\tr1                               256   0.00      3     0     0     0   256\n\tr1z0                             256   0.00      2     0    52   204     0\n\tr1z0-127.0.0.0                    76   0.00      1   180    76     0     0\n\tr1z0-127.0.0.0/sda1               19   0.00      1   237    19     0     0\n\tr1z0-127.0.0.0/sdb1               19   0.00      1   237    19     0     0\n\tr1z0-127.0.0.0/sdc1               19   0.00      1   237    19     0     0\n\tr1z0-127.0.0.0/sdd1               19   0.00      1   237    19     0     0\n\tr1z0-127.0.0.3                   256  50.00      1     0   128   128     0\n\tr1z0-127.0.0.3/sda1               96   0.00      1   160    96     0     0\n\tr1z0-127.0.0.3/sdb1               96   0.00      1   160    96     0     0\n\tr1z0-127.0.0.3/sdc1               96   0.00      1   160    96     0     0\n\tr1z0-127.0.0.3/sdd1               96   0.00      1   160    96     0     0\n\tr1z1                             256   0.00      2     0   204    52     0\n\tr1z1-127.0.0.1                   154   0.00      1   102   154     0     0\n\tr1z1-127.0.0.1/sda1               39   0.00      1   217    39     0     0\n\tr1z1-127.0.0.1/sdb1               39   0.00      1   217    39     0     0\n\tr1z1-127.0.0.1/sdc1               38   0.00      1   218    38     0     0\n\tr1z1-127.0.0.1/sdd1               38   0.00      1   218    38     0     0\n\tr1z1-127.0.0.2                   154   0.00      1   102   154     0     0\n\tr1z1-127.0.0.2/sda1               39   0.00      1   217    39     0     0\n\tr1z1-127.0.0.2/sdb1               39   0.00      1   217    39     0     0\n\tr1z1-127.0.0.2/sdc1               38   0.00      1   218    38     0     0\n\tr1z1-127.0.0.2/sdd1               38   0.00      1   218    38     0     0","commit_id":"ce26e78902a8c2feff5706a07750e34e9a94536e"}]}