)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dce09c53df5f54f39107a6dee6c87b023888913c","unresolved":true,"context_lines":[{"line_number":18,"context_line":"identifing local devices. This uses the ring.devs, so does include 0 weighted"},{"line_number":19,"context_line":"devices."},{"line_number":20,"context_line":"This allows the sharder to find a location to write the shard_broker in"},{"line_number":21,"context_line":"a handoff location while sharding."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ic38698e9ca0397770c7362229baef1101a72788f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3ebeb848_3c2b67e5","line":21,"updated":"2022-07-28 09:44:57.000000000","message":"please line wrap the commit message at column 80\n\nTBH, I\u0027m not sure what the etiquette is on commit message width - it\u0027s obviously not enforced as it is for code, but IMHO it\u0027s a good think and if nothing else makes it easier to read in gerrit.","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":18,"context_line":"identifing local devices. This uses the ring.devs, so does include 0 weighted"},{"line_number":19,"context_line":"devices."},{"line_number":20,"context_line":"This allows the sharder to find a location to write the shard_broker in"},{"line_number":21,"context_line":"a handoff location while sharding."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ic38698e9ca0397770c7362229baef1101a72788f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"26c93096_c0a33675","line":21,"in_reply_to":"3ebeb848_3c2b67e5","updated":"2022-07-29 14:22:22.000000000","message":"Done","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c218fce8a179d6be4f6949242ea8c19085a4c48","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"00e900e5_23be6df2","updated":"2022-07-21 15:08:51.000000000","message":"I tried this in prod with a sharding DB on a node with all zero-weight devices. It worked!","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"27b0e0342ad87e7d140546fc6ec933a8b495b22f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"69b61522_e4744c00","updated":"2022-07-22 09:26:02.000000000","message":"nice, but unit test needs fixing","commit_id":"0576b4619e31845993a01c49cca83e586d541bfe"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"41fccfd722e851f8d95aabcbb1b2588548bb69fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"05a06f7a_346abb13","updated":"2022-07-27 05:51:27.000000000","message":"Will push up a new version soon","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"cf5b4efda44d529c65646ce98597c79014ac0bd9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c935636e_18982a8d","updated":"2022-07-27 11:32:45.000000000","message":"I noticed there were no tests for `find_local_handoff_for_part` not even in the replicator. So might need to add some coverage for that.\n\nRan out of time and wanted to get the latest versoin of this up.","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"263bb7f2b24af717b9a9c6a18a8fe6313c53efaf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"002fb1d9_52cdeb08","updated":"2022-07-28 09:45:42.000000000","message":"I think the merge conflict may be because patchset 7 was based on patchset 5 - I rebased on patchset 6","commit_id":"ac013541bed82ddabbb3e97dfb74fc120523094c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"422dcbcf893b100ddb68e84381cebab9c917b5e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3968c57f_a5c413ac","updated":"2022-07-28 21:18:28.000000000","message":"See what you guys think about https://review.opendev.org/c/openstack/swift/+/851471","commit_id":"ac013541bed82ddabbb3e97dfb74fc120523094c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"12eec467af691ec3a2d7878c3779ca7f401e2415","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"87b022c3_4d067e1a","updated":"2022-07-29 14:04:44.000000000","message":"squashed in Tim\u0027s follow-on patch with Matt\u0027s diff ","commit_id":"c4e00eb89f34de79d5fb123dd044621ef4df679c"}],"swift/container/replicator.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"920d0d388965b04f09e69b7f1de53c8130fe5d52","unresolved":true,"context_lines":[{"line_number":168,"context_line":"                \"falling back to a local device %s\" %"},{"line_number":169,"context_line":"                (part, node[\u0027device\u0027]))"},{"line_number":170,"context_line":"        except IndexError:"},{"line_number":171,"context_line":"            pass"},{"line_number":172,"context_line":"        return node"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def get_reconciler_broker(self, timestamp):"}],"source_content_type":"text/x-python","patch_set":5,"id":"d533b8f2_379c797a","line":171,"updated":"2022-07-27 15:47:46.000000000","message":"hmmm, pass or let it raise? do we ever expect to get here if there are no local device ids?","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"422dcbcf893b100ddb68e84381cebab9c917b5e2","unresolved":true,"context_lines":[{"line_number":168,"context_line":"                \"falling back to a local device %s\" %"},{"line_number":169,"context_line":"                (part, node[\u0027device\u0027]))"},{"line_number":170,"context_line":"        except IndexError:"},{"line_number":171,"context_line":"            pass"},{"line_number":172,"context_line":"        return node"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def get_reconciler_broker(self, timestamp):"}],"source_content_type":"text/x-python","patch_set":5,"id":"697c95cd_fba2d277","line":171,"in_reply_to":"2adf7ba2_88822175","updated":"2022-07-28 21:18:28.000000000","message":"I\u0027m partial towards rewriting the function to something like\n\n if not self._local_device_ids:\n     raise RuntimeError(\u0027Cannot find local handoff; no local devices\u0027)\n for node in self.ring.get_part_nodes(part):\n     if node[\u0027id\u0027] in self._local_device_ids:\n         return node\n # not much point in trying to minimize handoff depth\n return choice(list(self._local_device_ids.values()))","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"559054308a7fbb8df9614431551b55a0ca2160a7","unresolved":true,"context_lines":[{"line_number":168,"context_line":"                \"falling back to a local device %s\" %"},{"line_number":169,"context_line":"                (part, node[\u0027device\u0027]))"},{"line_number":170,"context_line":"        except IndexError:"},{"line_number":171,"context_line":"            pass"},{"line_number":172,"context_line":"        return node"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def get_reconciler_broker(self, timestamp):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ed487360_11dfaa8d","line":171,"in_reply_to":"d533b8f2_379c797a","updated":"2022-07-28 07:24:21.000000000","message":"I figure pass as this will just make it return None, ie no node, and we catch it back in the sharder. Although I guess we could just as easily catch the IndexError in the sharder.. though that might be less counter intuitive.\n\nIf this method returns a node \u003d\u003d None means we couldn\u0027t find a local handoff, which is true in this case right?","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dce09c53df5f54f39107a6dee6c87b023888913c","unresolved":true,"context_lines":[{"line_number":168,"context_line":"                \"falling back to a local device %s\" %"},{"line_number":169,"context_line":"                (part, node[\u0027device\u0027]))"},{"line_number":170,"context_line":"        except IndexError:"},{"line_number":171,"context_line":"            pass"},{"line_number":172,"context_line":"        return node"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def get_reconciler_broker(self, timestamp):"}],"source_content_type":"text/x-python","patch_set":5,"id":"2adf7ba2_88822175","line":171,"in_reply_to":"ed487360_11dfaa8d","updated":"2022-07-28 09:44:57.000000000","message":"my concern is that we obscure a coding error - we should never get an IndexError (I think?), but if we catch it and then raise DeviceUnavailable we have obscured the bug\n\nin fact, we\u0027re going to a lot of effort to annotate the exception message as if the problem might be specific to a partition, but in fact the problem is \"_local_device_ids is empty, how can that be!?!\"","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"920d0d388965b04f09e69b7f1de53c8130fe5d52","unresolved":true,"context_lines":[{"line_number":169,"context_line":"                (part, node[\u0027device\u0027]))"},{"line_number":170,"context_line":"        except IndexError:"},{"line_number":171,"context_line":"            pass"},{"line_number":172,"context_line":"        return node"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def get_reconciler_broker(self, timestamp):"},{"line_number":175,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"080c2603_fe185cc9","line":172,"updated":"2022-07-27 15:47:46.000000000","message":"ok, this method is also used to place reconciler container DBs. The locations of these are stored and then explicitly replicated at end of the replication cycle so it\u0027s OK for them to be on zero-weight devices.","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":169,"context_line":"                (part, node[\u0027device\u0027]))"},{"line_number":170,"context_line":"        except IndexError:"},{"line_number":171,"context_line":"            pass"},{"line_number":172,"context_line":"        return node"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def get_reconciler_broker(self, timestamp):"},{"line_number":175,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"b8adfbaf_1bd26634","line":172,"in_reply_to":"080c2603_fe185cc9","updated":"2022-07-29 14:22:22.000000000","message":"Ack","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dce09c53df5f54f39107a6dee6c87b023888913c","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        except DeviceUnavailable as ex:"},{"line_number":196,"context_line":"            raise DeviceUnavailable("},{"line_number":197,"context_line":"                \u0027Failed to create a handoff reconciler broker for %s: %s\u0027 %"},{"line_number":198,"context_line":"                (container, str(ex)))"},{"line_number":199,"context_line":"        broker \u003d ContainerBroker.create_broker("},{"line_number":200,"context_line":"            os.path.join(self.root, node[\u0027device\u0027]), part, account, container,"},{"line_number":201,"context_line":"            logger\u003dself.logger, put_timestamp\u003dtimestamp,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dacc8ed5_793b1ef9","line":198,"updated":"2022-07-28 09:44:57.000000000","message":"that didn\u0027t work out quite as nicely as I had hoped - I had hoped we could have find_local_handoff_for_part just raise the exception and not have a try/except here - but I guess there is the customised exception message to deal with.\n\nThis is now more lines of code than the last version for the same outcome :/\n\nBut as commented above, maybe there is no value in annotating the exception - it\u0027s going to be raised for *every* partition that we try to get a handoff for.","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"422dcbcf893b100ddb68e84381cebab9c917b5e2","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        except DeviceUnavailable as ex:"},{"line_number":196,"context_line":"            raise DeviceUnavailable("},{"line_number":197,"context_line":"                \u0027Failed to create a handoff reconciler broker for %s: %s\u0027 %"},{"line_number":198,"context_line":"                (container, str(ex)))"},{"line_number":199,"context_line":"        broker \u003d ContainerBroker.create_broker("},{"line_number":200,"context_line":"            os.path.join(self.root, node[\u0027device\u0027]), part, account, container,"},{"line_number":201,"context_line":"            logger\u003dself.logger, put_timestamp\u003dtimestamp,"}],"source_content_type":"text/x-python","patch_set":7,"id":"eb2ef489_358aeb78","line":198,"in_reply_to":"dacc8ed5_793b1ef9","updated":"2022-07-28 21:18:28.000000000","message":"...do we have to (re)raise DeviceUnavailable here? It\u0027s not clearly valuable to me. I kind of want to just get rid of the if block (or try/except block) and let the context of \"we hit this problem while trying to create a handoff reconciler broker\" be passed along by the fact that there was a get_reconciler_broker frame in the stack.","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        except DeviceUnavailable as ex:"},{"line_number":196,"context_line":"            raise DeviceUnavailable("},{"line_number":197,"context_line":"                \u0027Failed to create a handoff reconciler broker for %s: %s\u0027 %"},{"line_number":198,"context_line":"                (container, str(ex)))"},{"line_number":199,"context_line":"        broker \u003d ContainerBroker.create_broker("},{"line_number":200,"context_line":"            os.path.join(self.root, node[\u0027device\u0027]), part, account, container,"},{"line_number":201,"context_line":"            logger\u003dself.logger, put_timestamp\u003dtimestamp,"}],"source_content_type":"text/x-python","patch_set":7,"id":"804cfb97_d8c1ade8","line":198,"in_reply_to":"eb2ef489_358aeb78","updated":"2022-07-29 14:22:22.000000000","message":"Done","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"422dcbcf893b100ddb68e84381cebab9c917b5e2","unresolved":true,"context_lines":[{"line_number":219,"context_line":"            reconciler \u003d self.get_reconciler_broker(container)"},{"line_number":220,"context_line":"        except DeviceUnavailable as e:"},{"line_number":221,"context_line":"            self.logger.warning(\u0027DeviceUnavailable: %s\u0027, e)"},{"line_number":222,"context_line":"            return False"},{"line_number":223,"context_line":"        self.logger.debug(\u0027Adding %d objects to the reconciler at %s\u0027,"},{"line_number":224,"context_line":"                          len(item_list), reconciler.db_file)"},{"line_number":225,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"55c41110_3bfaaf91","line":222,"updated":"2022-07-28 21:18:28.000000000","message":"...and then down here we could more closely mimic the merge_items block with something like\n\n try:\n     reconciler \u003d self.get_reconciler_broker(container)\n except Exception:\n     self.logger.exception(\u0027Failed to get reconciler broker for container %s\u0027, container)\n     return False","commit_id":"ac013541bed82ddabbb3e97dfb74fc120523094c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            reconciler \u003d self.get_reconciler_broker(container)"},{"line_number":220,"context_line":"        except DeviceUnavailable as e:"},{"line_number":221,"context_line":"            self.logger.warning(\u0027DeviceUnavailable: %s\u0027, e)"},{"line_number":222,"context_line":"            return False"},{"line_number":223,"context_line":"        self.logger.debug(\u0027Adding %d objects to the reconciler at %s\u0027,"},{"line_number":224,"context_line":"                          len(item_list), reconciler.db_file)"},{"line_number":225,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"d1a936c5_244a6887","line":222,"in_reply_to":"55c41110_3bfaaf91","updated":"2022-07-29 14:22:22.000000000","message":"Done","commit_id":"ac013541bed82ddabbb3e97dfb74fc120523094c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a7f90da7a46d2ae8356e07ed9a4cd760cf364e4","unresolved":true,"context_lines":[{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        # we have to return something, so choose any local device.."},{"line_number":171,"context_line":"        node \u003d choice(list(self._local_device_ids.values()))"},{"line_number":172,"context_line":"        self.logger.warning("},{"line_number":173,"context_line":"            \"Could not find a non-zero weight device for handoff partition \""},{"line_number":174,"context_line":"            \"%d, falling back device %s\" %"},{"line_number":175,"context_line":"            (part, node[\u0027device\u0027]))"}],"source_content_type":"text/x-python","patch_set":9,"id":"a0eefde5_a7e8dd65","line":172,"range":{"start_line":172,"start_character":20,"end_line":172,"end_character":27},"updated":"2022-07-29 16:02:37.000000000","message":"I made this a warning because we do not expect it to happen often and if it does an op might like a clue as to why DBs are appearing on a zero weight node.\n\nopinions?","commit_id":"c4e00eb89f34de79d5fb123dd044621ef4df679c"}],"swift/container/sharder.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c218fce8a179d6be4f6949242ea8c19085a4c48","unresolved":true,"context_lines":[{"line_number":1051,"context_line":"            :class:`~swift.container.backend.ContainerBroker`,"},{"line_number":1052,"context_line":"            ``node_id`` is the id of the selected node,"},{"line_number":1053,"context_line":"            ``put_timestamp`` is the put_timestamp if the broker needed to"},{"line_number":1054,"context_line":"            be initialized."},{"line_number":1055,"context_line":"        \"\"\""},{"line_number":1056,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1057,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ccaa7d68_6b730042","line":1054,"updated":"2022-07-21 15:08:51.000000000","message":"+1 for adding docstring\n\noff-topic: the interface to this method could surely be better to avoid returning a 4-element tuple","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0d867c06d5620811bfa7e7b5f32629e7c93597e2","unresolved":false,"context_lines":[{"line_number":1051,"context_line":"            :class:`~swift.container.backend.ContainerBroker`,"},{"line_number":1052,"context_line":"            ``node_id`` is the id of the selected node,"},{"line_number":1053,"context_line":"            ``put_timestamp`` is the put_timestamp if the broker needed to"},{"line_number":1054,"context_line":"            be initialized."},{"line_number":1055,"context_line":"        \"\"\""},{"line_number":1056,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1057,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7902fbcd_b4a52bf9","line":1054,"in_reply_to":"c8640aa5_c5f13a18","updated":"2022-07-27 11:34:21.000000000","message":"Ack","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1ef8c046405282116ff55c685f8b66f0e880ab5b","unresolved":true,"context_lines":[{"line_number":1051,"context_line":"            :class:`~swift.container.backend.ContainerBroker`,"},{"line_number":1052,"context_line":"            ``node_id`` is the id of the selected node,"},{"line_number":1053,"context_line":"            ``put_timestamp`` is the put_timestamp if the broker needed to"},{"line_number":1054,"context_line":"            be initialized."},{"line_number":1055,"context_line":"        \"\"\""},{"line_number":1056,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1057,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c8640aa5_c5f13a18","line":1054,"in_reply_to":"ccaa7d68_6b730042","updated":"2022-07-22 00:42:09.000000000","message":"yeah, totally agree, but figure thats out of scope","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c218fce8a179d6be4f6949242ea8c19085a4c48","unresolved":true,"context_lines":[{"line_number":1061,"context_line":"            # not a primary for the shard or a get_more_nodes candidate."},{"line_number":1062,"context_line":"            # we may also be zero weight and draining, but we still need to"},{"line_number":1063,"context_line":"            # do work. So if we fall back to one of our devices then we should"},{"line_number":1064,"context_line":"            # be good to go."},{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"}],"source_content_type":"text/x-python","patch_set":1,"id":"50c9a2c1_54cf1f95","line":1064,"updated":"2022-07-21 15:08:51.000000000","message":"I found the comment a little confusing. How about:\n\n  # uncommon case: the ring can\u0027t assign any local device for the shard\n  # partition (perhaps because all local devices have zero weight) so \n  # fall back to picking *any* local device; the shard DB will only be\n  # on the device temporarily","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1ef8c046405282116ff55c685f8b66f0e880ab5b","unresolved":true,"context_lines":[{"line_number":1061,"context_line":"            # not a primary for the shard or a get_more_nodes candidate."},{"line_number":1062,"context_line":"            # we may also be zero weight and draining, but we still need to"},{"line_number":1063,"context_line":"            # do work. So if we fall back to one of our devices then we should"},{"line_number":1064,"context_line":"            # be good to go."},{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"}],"source_content_type":"text/x-python","patch_set":1,"id":"96896a0e_4c76b7a0","line":1064,"in_reply_to":"50c9a2c1_54cf1f95","updated":"2022-07-22 00:42:09.000000000","message":"thanks, I\u0027ll fix it. I tend to write the comment while I\u0027m writing to function to help me think about what I\u0027m about to do.. and I have a bad habbit of not going back and re-reading it makes sense when I\u0027m done :P","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6de72558c942d21a100b36d1d42256fee7c1549c","unresolved":false,"context_lines":[{"line_number":1061,"context_line":"            # not a primary for the shard or a get_more_nodes candidate."},{"line_number":1062,"context_line":"            # we may also be zero weight and draining, but we still need to"},{"line_number":1063,"context_line":"            # do work. So if we fall back to one of our devices then we should"},{"line_number":1064,"context_line":"            # be good to go."},{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"}],"source_content_type":"text/x-python","patch_set":1,"id":"aad284fd_da8fc4d6","line":1064,"in_reply_to":"96896a0e_4c76b7a0","updated":"2022-07-25 06:59:36.000000000","message":"Done","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c218fce8a179d6be4f6949242ea8c19085a4c48","unresolved":true,"context_lines":[{"line_number":1063,"context_line":"            # do work. So if we fall back to one of our devices then we should"},{"line_number":1064,"context_line":"            # be good to go."},{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"},{"line_number":1068,"context_line":"                    if dev and dev[\u0027id\u0027] in self._local_device_ids])"},{"line_number":1069,"context_line":"                self.logger.info("}],"source_content_type":"text/x-python","patch_set":1,"id":"32329c18_6da2ed3d","line":1066,"updated":"2022-07-21 15:08:51.000000000","message":"I think it would be good enough to just use the device that the sharding container is on, but that device *dict* isn\u0027t easy to access here, so I guess we need to go searching anyway.","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1ef8c046405282116ff55c685f8b66f0e880ab5b","unresolved":true,"context_lines":[{"line_number":1063,"context_line":"            # do work. So if we fall back to one of our devices then we should"},{"line_number":1064,"context_line":"            # be good to go."},{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"},{"line_number":1068,"context_line":"                    if dev and dev[\u0027id\u0027] in self._local_device_ids])"},{"line_number":1069,"context_line":"                self.logger.info("}],"source_content_type":"text/x-python","patch_set":1,"id":"c7f943b1_5fa3e50a","line":1066,"in_reply_to":"32329c18_6da2ed3d","updated":"2022-07-22 00:42:09.000000000","message":"yeah, we do pass in the node when we start processing a db, but not when we get to cleave. I started out by trying to plumb it through, but started to seem like alot of churn. When we already store the local dev ids.\n\nI figured might as well make it random one in case we\u0027re draining for a reason.. seemed safer. But yeah, needing to go searching is annoyting, but at least we\u0027re only doing if we can\u0027t get a local handoff from the replica2part2dev table.","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6de72558c942d21a100b36d1d42256fee7c1549c","unresolved":false,"context_lines":[{"line_number":1063,"context_line":"            # do work. So if we fall back to one of our devices then we should"},{"line_number":1064,"context_line":"            # be good to go."},{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"},{"line_number":1068,"context_line":"                    if dev and dev[\u0027id\u0027] in self._local_device_ids])"},{"line_number":1069,"context_line":"                self.logger.info("}],"source_content_type":"text/x-python","patch_set":1,"id":"76083fc1_f377dcc8","line":1066,"in_reply_to":"c7f943b1_5fa3e50a","updated":"2022-07-25 06:59:36.000000000","message":"Done","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c218fce8a179d6be4f6949242ea8c19085a4c48","unresolved":true,"context_lines":[{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"},{"line_number":1068,"context_line":"                    if dev and dev[\u0027id\u0027] in self._local_device_ids])"},{"line_number":1069,"context_line":"                self.logger.info("},{"line_number":1070,"context_line":"                    \"Could not find a local handoff for %s in partition %d \""},{"line_number":1071,"context_line":"                    \"falling back to a local device %s\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"a0203b8d_d1b2db84","line":1068,"range":{"start_line":1068,"start_character":44,"end_line":1068,"end_character":66},"updated":"2022-07-21 15:08:51.000000000","message":"might well be worth looking at making _local_device_ids a dict mapping dev[\u0027id\u0027]-\u003e node dict, so that we can just pick one from the dict without having to scan the ring again.\n\nnit: also, the variable naming could be more consistent w.r.t. \u0027node\u0027 and \u0027dev\u0027","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6de72558c942d21a100b36d1d42256fee7c1549c","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"},{"line_number":1068,"context_line":"                    if dev and dev[\u0027id\u0027] in self._local_device_ids])"},{"line_number":1069,"context_line":"                self.logger.info("},{"line_number":1070,"context_line":"                    \"Could not find a local handoff for %s in partition %d \""},{"line_number":1071,"context_line":"                    \"falling back to a local device %s\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"2ead99db_4889ab6c","line":1068,"range":{"start_line":1068,"start_character":44,"end_line":1068,"end_character":66},"in_reply_to":"1ecc4bfa_550cb2a1","updated":"2022-07-25 06:59:36.000000000","message":"Done","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1ef8c046405282116ff55c685f8b66f0e880ab5b","unresolved":true,"context_lines":[{"line_number":1065,"context_line":"            try:"},{"line_number":1066,"context_line":"                node \u003d choice(["},{"line_number":1067,"context_line":"                    dev for dev in self.ring.devs"},{"line_number":1068,"context_line":"                    if dev and dev[\u0027id\u0027] in self._local_device_ids])"},{"line_number":1069,"context_line":"                self.logger.info("},{"line_number":1070,"context_line":"                    \"Could not find a local handoff for %s in partition %d \""},{"line_number":1071,"context_line":"                    \"falling back to a local device %s\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ecc4bfa_550cb2a1","line":1068,"range":{"start_line":1068,"start_character":44,"end_line":1068,"end_character":66},"in_reply_to":"a0203b8d_d1b2db84","updated":"2022-07-22 00:42:09.000000000","message":"Now that is a brillant idea! I didn\u0027t like having to scan again.","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"27b0e0342ad87e7d140546fc6ec933a8b495b22f","unresolved":true,"context_lines":[{"line_number":1064,"context_line":"            # weight) so fall back to picking *any* local device; the shard"},{"line_number":1065,"context_line":"            # DB will only be on the device temporarily"},{"line_number":1066,"context_line":"            try:"},{"line_number":1067,"context_line":"                node \u003d choice(list(self._local_device_ids.values()))"},{"line_number":1068,"context_line":"                self.logger.info("},{"line_number":1069,"context_line":"                    \"Could not find a local handoff for %s in partition %d \""},{"line_number":1070,"context_line":"                    \"falling back to a local device %s\" %"}],"source_content_type":"text/x-python","patch_set":3,"id":"0d00617b_93bdea71","line":1067,"range":{"start_line":1067,"start_character":35,"end_line":1067,"end_character":58},"updated":"2022-07-22 09:26:02.000000000","message":"ok! that switch to a dict wasn\u0027t too disruptive :)","commit_id":"0576b4619e31845993a01c49cca83e586d541bfe"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dce09c53df5f54f39107a6dee6c87b023888913c","unresolved":false,"context_lines":[{"line_number":1064,"context_line":"            # weight) so fall back to picking *any* local device; the shard"},{"line_number":1065,"context_line":"            # DB will only be on the device temporarily"},{"line_number":1066,"context_line":"            try:"},{"line_number":1067,"context_line":"                node \u003d choice(list(self._local_device_ids.values()))"},{"line_number":1068,"context_line":"                self.logger.info("},{"line_number":1069,"context_line":"                    \"Could not find a local handoff for %s in partition %d \""},{"line_number":1070,"context_line":"                    \"falling back to a local device %s\" %"}],"source_content_type":"text/x-python","patch_set":3,"id":"5db39ba7_3ce9a6fa","line":1067,"range":{"start_line":1067,"start_character":35,"end_line":1067,"end_character":58},"in_reply_to":"0d00617b_93bdea71","updated":"2022-07-28 09:44:57.000000000","message":"Done","commit_id":"0576b4619e31845993a01c49cca83e586d541bfe"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8e55d9875559a61cbdcc41c7fcd9c21dc3f79154","unresolved":true,"context_lines":[{"line_number":1056,"context_line":"            be initialized."},{"line_number":1057,"context_line":"        \"\"\""},{"line_number":1058,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1059,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"},{"line_number":1060,"context_line":"        put_timestamp \u003d Timestamp.now().internal"},{"line_number":1061,"context_line":"        if not node:"},{"line_number":1062,"context_line":"            # uncommon case: the ring can\u0027t assign any local device for the"}],"source_content_type":"text/x-python","patch_set":4,"id":"5887f0f5_9c52c095","line":1059,"updated":"2022-07-25 20:16:28.000000000","message":"I wonder if this fallback behavior should go up in find_local_handoff_for_part() -- we can run into the same problem feeding the reconciler queue, too, right?\n\nI guess the good news is, if we popped a similar error there, we\u0027ve got a catch-all to log it, but then we\u0027ll proceed to cleanup: https://github.com/openstack/swift/blob/2.29.1/swift/common/db_replicator.py#L686-L692\n\nIn fact, I wonder if it\u0027d be better for find_local_handoff_for_part() to check whether we\u0027ve got a local primary disk, then immediately fall back to a random local device. If we\u0027ve got hundreds of A/C disks, IDK that it\u0027s worth potentially going through the whole list just to get a node that\u0027s closer to being accessible to a proxy.","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"41fccfd722e851f8d95aabcbb1b2588548bb69fd","unresolved":true,"context_lines":[{"line_number":1056,"context_line":"            be initialized."},{"line_number":1057,"context_line":"        \"\"\""},{"line_number":1058,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1059,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"},{"line_number":1060,"context_line":"        put_timestamp \u003d Timestamp.now().internal"},{"line_number":1061,"context_line":"        if not node:"},{"line_number":1062,"context_line":"            # uncommon case: the ring can\u0027t assign any local device for the"}],"source_content_type":"text/x-python","patch_set":4,"id":"9823a15a_7ffd8e8a","line":1059,"in_reply_to":"5887f0f5_9c52c095","updated":"2022-07-27 05:51:27.000000000","message":"Yeah, I like the idea of moving it back into find_local_handoff_for_part.\n\nGot think some more the if a node doesn\u0027t have a primary disk just return a random local disk. This could reduce our loops, but it\u0027s only part_nodes more. And having it within proxies reach for a partition does sound like a good and expected thing.. a random disk would loose this nice trait.","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0d867c06d5620811bfa7e7b5f32629e7c93597e2","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"            be initialized."},{"line_number":1057,"context_line":"        \"\"\""},{"line_number":1058,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1059,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"},{"line_number":1060,"context_line":"        put_timestamp \u003d Timestamp.now().internal"},{"line_number":1061,"context_line":"        if not node:"},{"line_number":1062,"context_line":"            # uncommon case: the ring can\u0027t assign any local device for the"}],"source_content_type":"text/x-python","patch_set":4,"id":"3c9edd4d_d4c68eaf","line":1059,"in_reply_to":"9823a15a_7ffd8e8a","updated":"2022-07-27 11:34:21.000000000","message":"Done","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8e55d9875559a61cbdcc41c7fcd9c21dc3f79154","unresolved":true,"context_lines":[{"line_number":1070,"context_line":"                    \"falling back to a local device %s\" %"},{"line_number":1071,"context_line":"                    (quote(shard_range.name), part, node[\u0027device\u0027]))"},{"line_number":1072,"context_line":"            except IndexError:"},{"line_number":1073,"context_line":"                raise DeviceUnavailable("},{"line_number":1074,"context_line":"                    \u0027No mounted devices found suitable for creating shard \u0027"},{"line_number":1075,"context_line":"                    \u0027broker for %s in partition %s\u0027 %"},{"line_number":1076,"context_line":"                    (quote(shard_range.name), part))"}],"source_content_type":"text/x-python","patch_set":4,"id":"50b9a08f_9ea2db6e","line":1073,"updated":"2022-07-25 20:16:28.000000000","message":"I think I\u0027d be fine with blowing up harder here -- this should *never happen*, right? We need a local handoff because we\u0027re currently processing a DB we found on disk, and we would have added the disk this DB is on to the dict, right?","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":1070,"context_line":"                    \"falling back to a local device %s\" %"},{"line_number":1071,"context_line":"                    (quote(shard_range.name), part, node[\u0027device\u0027]))"},{"line_number":1072,"context_line":"            except IndexError:"},{"line_number":1073,"context_line":"                raise DeviceUnavailable("},{"line_number":1074,"context_line":"                    \u0027No mounted devices found suitable for creating shard \u0027"},{"line_number":1075,"context_line":"                    \u0027broker for %s in partition %s\u0027 %"},{"line_number":1076,"context_line":"                    (quote(shard_range.name), part))"}],"source_content_type":"text/x-python","patch_set":4,"id":"69ee3b22_7a088b63","line":1073,"in_reply_to":"50b9a08f_9ea2db6e","updated":"2022-07-29 14:22:22.000000000","message":"Done","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8e55d9875559a61cbdcc41c7fcd9c21dc3f79154","unresolved":true,"context_lines":[{"line_number":2165,"context_line":"            if os.path.isdir(datadir):"},{"line_number":2166,"context_line":"                # Populate self._local_device_ids so we can find devices for"},{"line_number":2167,"context_line":"                # shard containers later"},{"line_number":2168,"context_line":"                self._local_device_ids[node[\u0027id\u0027]] \u003d node"},{"line_number":2169,"context_line":"                if node[\u0027device\u0027] not in devices_to_shard:"},{"line_number":2170,"context_line":"                    continue"},{"line_number":2171,"context_line":"                part_filt \u003d self._partition_dir_filter("}],"source_content_type":"text/x-python","patch_set":4,"id":"b28a00ea_e451a382","line":2168,"updated":"2022-07-25 20:16:28.000000000","message":"Off-topic: It\u0027s kinda weird that we have to do this again here, instead of *only* populating it in db_replicator...","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":2165,"context_line":"            if os.path.isdir(datadir):"},{"line_number":2166,"context_line":"                # Populate self._local_device_ids so we can find devices for"},{"line_number":2167,"context_line":"                # shard containers later"},{"line_number":2168,"context_line":"                self._local_device_ids[node[\u0027id\u0027]] \u003d node"},{"line_number":2169,"context_line":"                if node[\u0027device\u0027] not in devices_to_shard:"},{"line_number":2170,"context_line":"                    continue"},{"line_number":2171,"context_line":"                part_filt \u003d self._partition_dir_filter("}],"source_content_type":"text/x-python","patch_set":4,"id":"948dadc4_2c91d14b","line":2168,"in_reply_to":"afa382d9_7814d1c9","updated":"2022-07-29 14:22:22.000000000","message":"Ack","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"920d0d388965b04f09e69b7f1de53c8130fe5d52","unresolved":true,"context_lines":[{"line_number":2165,"context_line":"            if os.path.isdir(datadir):"},{"line_number":2166,"context_line":"                # Populate self._local_device_ids so we can find devices for"},{"line_number":2167,"context_line":"                # shard containers later"},{"line_number":2168,"context_line":"                self._local_device_ids[node[\u0027id\u0027]] \u003d node"},{"line_number":2169,"context_line":"                if node[\u0027device\u0027] not in devices_to_shard:"},{"line_number":2170,"context_line":"                    continue"},{"line_number":2171,"context_line":"                part_filt \u003d self._partition_dir_filter("}],"source_content_type":"text/x-python","patch_set":4,"id":"afa382d9_7814d1c9","line":2168,"in_reply_to":"b28a00ea_e451a382","updated":"2022-07-27 15:47:46.000000000","message":"+1 I have also wondered why this is repeated - is there a subtle difference? but, different topic","commit_id":"eb0cecf682005d99d16a154472faf878b6bfadb2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"920d0d388965b04f09e69b7f1de53c8130fe5d52","unresolved":true,"context_lines":[{"line_number":1057,"context_line":"        \"\"\""},{"line_number":1058,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1059,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"},{"line_number":1060,"context_line":"        put_timestamp \u003d Timestamp.now().internal"},{"line_number":1061,"context_line":"        if not node:"},{"line_number":1062,"context_line":"            raise DeviceUnavailable("},{"line_number":1063,"context_line":"                \u0027No mounted devices found suitable for creating shard \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"22385817_5ed2fe91","line":1060,"updated":"2022-07-27 15:47:46.000000000","message":"nit: move this line after the \u0027if not node\u0027 check","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"559054308a7fbb8df9614431551b55a0ca2160a7","unresolved":false,"context_lines":[{"line_number":1057,"context_line":"        \"\"\""},{"line_number":1058,"context_line":"        part \u003d self.ring.get_part(shard_range.account, shard_range.container)"},{"line_number":1059,"context_line":"        node \u003d self.find_local_handoff_for_part(part)"},{"line_number":1060,"context_line":"        put_timestamp \u003d Timestamp.now().internal"},{"line_number":1061,"context_line":"        if not node:"},{"line_number":1062,"context_line":"            raise DeviceUnavailable("},{"line_number":1063,"context_line":"                \u0027No mounted devices found suitable for creating shard \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"2ebdb575_c6f6b48b","line":1060,"in_reply_to":"22385817_5ed2fe91","updated":"2022-07-28 07:24:21.000000000","message":"Done","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"920d0d388965b04f09e69b7f1de53c8130fe5d52","unresolved":true,"context_lines":[{"line_number":1062,"context_line":"            raise DeviceUnavailable("},{"line_number":1063,"context_line":"                \u0027No mounted devices found suitable for creating shard \u0027"},{"line_number":1064,"context_line":"                \u0027broker for %s in partition %s\u0027 %"},{"line_number":1065,"context_line":"                (quote(shard_range.name), part))"},{"line_number":1066,"context_line":""},{"line_number":1067,"context_line":"        shard_broker \u003d ContainerBroker.create_broker("},{"line_number":1068,"context_line":"            os.path.join(self.root, node[\u0027device\u0027]), part, shard_range.account,"}],"source_content_type":"text/x-python","patch_set":5,"id":"b715c8bd_e7a3400c","line":1065,"updated":"2022-07-27 15:47:46.000000000","message":"I think we could move the raise into find_local_handoff_for_part method - the other place it is called in replicator.py for reconciler containers the pattern is very similar - we might just need to pass in a string to customize the exception message.","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"559054308a7fbb8df9614431551b55a0ca2160a7","unresolved":false,"context_lines":[{"line_number":1062,"context_line":"            raise DeviceUnavailable("},{"line_number":1063,"context_line":"                \u0027No mounted devices found suitable for creating shard \u0027"},{"line_number":1064,"context_line":"                \u0027broker for %s in partition %s\u0027 %"},{"line_number":1065,"context_line":"                (quote(shard_range.name), part))"},{"line_number":1066,"context_line":""},{"line_number":1067,"context_line":"        shard_broker \u003d ContainerBroker.create_broker("},{"line_number":1068,"context_line":"            os.path.join(self.root, node[\u0027device\u0027]), part, shard_range.account,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7832447f_a03867e2","line":1065,"in_reply_to":"b715c8bd_e7a3400c","updated":"2022-07-28 07:24:21.000000000","message":"oh yeah thanks very true. Moved it into the function too.","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"422dcbcf893b100ddb68e84381cebab9c917b5e2","unresolved":true,"context_lines":[{"line_number":1796,"context_line":"            shard_part, shard_broker, node_id, put_timestamp \u003d \\"},{"line_number":1797,"context_line":"                self._get_shard_broker(shard_range, broker.root_path,"},{"line_number":1798,"context_line":"                                       policy_index)"},{"line_number":1799,"context_line":"        except DeviceUnavailable as duex:"},{"line_number":1800,"context_line":"            self.logger.warning(str(duex))"},{"line_number":1801,"context_line":"            self._increment_stat(\u0027cleaved\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1802,"context_line":"            return CLEAVE_FAILED"}],"source_content_type":"text/x-python","patch_set":8,"id":"ca51e5ea_a2b72af9","line":1799,"updated":"2022-07-28 21:18:28.000000000","message":"This is the only place we would handle the DeviceUnavailable specifically, yeah? Once we have this change, we would expect *every* shard range to blow up here if this is ever going to blow up, right? So everything\u0027s going to stall out anyway -- I kind of wonder if we should just let it bubble up to the general per-db exception handling in _one_shard_cycle...","commit_id":"ac013541bed82ddabbb3e97dfb74fc120523094c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":1796,"context_line":"            shard_part, shard_broker, node_id, put_timestamp \u003d \\"},{"line_number":1797,"context_line":"                self._get_shard_broker(shard_range, broker.root_path,"},{"line_number":1798,"context_line":"                                       policy_index)"},{"line_number":1799,"context_line":"        except DeviceUnavailable as duex:"},{"line_number":1800,"context_line":"            self.logger.warning(str(duex))"},{"line_number":1801,"context_line":"            self._increment_stat(\u0027cleaved\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1802,"context_line":"            return CLEAVE_FAILED"}],"source_content_type":"text/x-python","patch_set":8,"id":"b5263c40_3a140617","line":1799,"in_reply_to":"ca51e5ea_a2b72af9","updated":"2022-07-29 14:22:22.000000000","message":"Done","commit_id":"ac013541bed82ddabbb3e97dfb74fc120523094c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7516d53288fc368154e3d84a11a63da016d5957e","unresolved":true,"context_lines":[{"line_number":1800,"context_line":"                    \u0027 %s successes, %s required.\u0027, shard_range,"},{"line_number":1801,"context_line":"                    quote(broker.path),"},{"line_number":1802,"context_line":"                    replication_successes, replication_quorum)"},{"line_number":1803,"context_line":"                self._increment_stat(\u0027cleaved\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1804,"context_line":"                result \u003d CLEAVE_FAILED"},{"line_number":1805,"context_line":"            else:"},{"line_number":1806,"context_line":"                elapsed \u003d round(time.time() - start, 3)"}],"source_content_type":"text/x-python","patch_set":9,"id":"44156f4a_0e7f587a","line":1803,"updated":"2022-07-29 17:36:50.000000000","message":"Right, so we still have the failure stat sometimes -- and now we always know what it means.","commit_id":"c4e00eb89f34de79d5fb123dd044621ef4df679c"}],"test/unit/container/test_replicator.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dce09c53df5f54f39107a6dee6c87b023888913c","unresolved":true,"context_lines":[{"line_number":2659,"context_line":"        node \u003d daemon.find_local_handoff_for_part(0)"},{"line_number":2660,"context_line":"        self.assertEqual(node[\u0027id\u0027], ring_node[\u0027id\u0027])"},{"line_number":2661,"context_line":""},{"line_number":2662,"context_line":"        # Now let\u0027s assume there local_handoff_for_part fails because the"},{"line_number":2663,"context_line":"        # node we\u0027re on is at zero weight for all disks. So it wont appear in"},{"line_number":2664,"context_line":"        # the replica2part2dev table, meaning we wont get a node back."},{"line_number":2665,"context_line":"        # in this case, we\u0027ll fall back to one of our own devices which we"}],"source_content_type":"text/x-python","patch_set":7,"id":"183b1043_157cc79b","line":2662,"updated":"2022-07-28 09:44:57.000000000","message":"comment needs updating now that the fallback happens in local_handoff_for_part","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":2659,"context_line":"        node \u003d daemon.find_local_handoff_for_part(0)"},{"line_number":2660,"context_line":"        self.assertEqual(node[\u0027id\u0027], ring_node[\u0027id\u0027])"},{"line_number":2661,"context_line":""},{"line_number":2662,"context_line":"        # Now let\u0027s assume there local_handoff_for_part fails because the"},{"line_number":2663,"context_line":"        # node we\u0027re on is at zero weight for all disks. So it wont appear in"},{"line_number":2664,"context_line":"        # the replica2part2dev table, meaning we wont get a node back."},{"line_number":2665,"context_line":"        # in this case, we\u0027ll fall back to one of our own devices which we"}],"source_content_type":"text/x-python","patch_set":7,"id":"213c37af_90aa5d00","line":2662,"in_reply_to":"183b1043_157cc79b","updated":"2022-07-29 14:22:22.000000000","message":"Ack","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dce09c53df5f54f39107a6dee6c87b023888913c","unresolved":true,"context_lines":[{"line_number":2665,"context_line":"        # in this case, we\u0027ll fall back to one of our own devices which we"},{"line_number":2666,"context_line":"        # determine from the ring.devs not the replica2part2dev table."},{"line_number":2667,"context_line":"        mock_more_nodes.return_value \u003d []"},{"line_number":2668,"context_line":"        local_dev_ids \u003d {dev[\u0027id\u0027]: dev for dev in daemon.ring.devs[-1:]}"},{"line_number":2669,"context_line":"        daemon._local_device_ids \u003d local_dev_ids"},{"line_number":2670,"context_line":"        node \u003d daemon.find_local_handoff_for_part(0)"},{"line_number":2671,"context_line":"        self.assertIn(node[\u0027id\u0027], local_dev_ids)"},{"line_number":2672,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3b4f7394_7ee73256","line":2669,"range":{"start_line":2668,"start_character":1,"end_line":2669,"end_character":48},"updated":"2022-07-28 09:44:57.000000000","message":"this initialisation could be done earlier at line 2652 I think - or is there significance in changing the local_device_ids here?","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d8b4e31efa64c7ef7b0484cd1bfeb9adcea6f10e","unresolved":false,"context_lines":[{"line_number":2665,"context_line":"        # in this case, we\u0027ll fall back to one of our own devices which we"},{"line_number":2666,"context_line":"        # determine from the ring.devs not the replica2part2dev table."},{"line_number":2667,"context_line":"        mock_more_nodes.return_value \u003d []"},{"line_number":2668,"context_line":"        local_dev_ids \u003d {dev[\u0027id\u0027]: dev for dev in daemon.ring.devs[-1:]}"},{"line_number":2669,"context_line":"        daemon._local_device_ids \u003d local_dev_ids"},{"line_number":2670,"context_line":"        node \u003d daemon.find_local_handoff_for_part(0)"},{"line_number":2671,"context_line":"        self.assertIn(node[\u0027id\u0027], local_dev_ids)"},{"line_number":2672,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"f222505e_558720c7","line":2669,"range":{"start_line":2668,"start_character":1,"end_line":2669,"end_character":48},"in_reply_to":"3b4f7394_7ee73256","updated":"2022-07-29 14:22:22.000000000","message":"Ack","commit_id":"24398845e7e08867117bc93b79de3b9d10319b1e"}],"test/unit/container/test_sharder.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c218fce8a179d6be4f6949242ea8c19085a4c48","unresolved":true,"context_lines":[{"line_number":6337,"context_line":"             sharder.ring.devs[node_id][\u0027device\u0027]))"},{"line_number":6338,"context_line":"        self.assertIn(expected_log_line, info_lines)"},{"line_number":6339,"context_line":""},{"line_number":6340,"context_line":"        # if there are more then 1 local_dev_id it\u0027ll randomly pick one"},{"line_number":6341,"context_line":"        with self._mock_sharder() as sharder:"},{"line_number":6342,"context_line":"            local_dev_ids \u003d {dev[\u0027id\u0027] for dev in sharder.ring.devs[-2:]}"},{"line_number":6343,"context_line":"            sharder._local_device_ids \u003d local_dev_ids"}],"source_content_type":"text/x-python","patch_set":1,"id":"e8e6e9dd_2a2a23ad","line":6340,"range":{"start_line":6340,"start_character":10,"end_line":6340,"end_character":71},"updated":"2022-07-21 15:08:51.000000000","message":"not sure the test actually verifies that the choice is random","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6de72558c942d21a100b36d1d42256fee7c1549c","unresolved":false,"context_lines":[{"line_number":6337,"context_line":"             sharder.ring.devs[node_id][\u0027device\u0027]))"},{"line_number":6338,"context_line":"        self.assertIn(expected_log_line, info_lines)"},{"line_number":6339,"context_line":""},{"line_number":6340,"context_line":"        # if there are more then 1 local_dev_id it\u0027ll randomly pick one"},{"line_number":6341,"context_line":"        with self._mock_sharder() as sharder:"},{"line_number":6342,"context_line":"            local_dev_ids \u003d {dev[\u0027id\u0027] for dev in sharder.ring.devs[-2:]}"},{"line_number":6343,"context_line":"            sharder._local_device_ids \u003d local_dev_ids"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ed95230_a2cf231c","line":6340,"range":{"start_line":6340,"start_character":10,"end_line":6340,"end_character":71},"in_reply_to":"e8e6e9dd_2a2a23ad","updated":"2022-07-25 06:59:36.000000000","message":"Done","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c218fce8a179d6be4f6949242ea8c19085a4c48","unresolved":true,"context_lines":[{"line_number":6353,"context_line":"             sharder.ring.devs[node_id][\u0027device\u0027]))"},{"line_number":6354,"context_line":"        self.assertIn(expected_log_line, info_lines)"},{"line_number":6355,"context_line":""},{"line_number":6356,"context_line":"        # If there are also no local_dev_ids, not that I\u0027m sure this is even"},{"line_number":6357,"context_line":"        # possible, then we\u0027ll get the DeviceUnavailable error"},{"line_number":6358,"context_line":"        with self._mock_sharder() as sharder:"},{"line_number":6359,"context_line":"            sharder._local_device_ids \u003d {}"},{"line_number":6360,"context_line":"            with self.assertRaises(DeviceUnavailable) as dev_err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"15bbfca3_1dfdc639","line":6357,"range":{"start_line":6356,"start_character":46,"end_line":6357,"end_character":18},"updated":"2022-07-21 15:08:51.000000000","message":"maybe possible if the ring has changed since local_device_ids was populated?","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6de72558c942d21a100b36d1d42256fee7c1549c","unresolved":false,"context_lines":[{"line_number":6353,"context_line":"             sharder.ring.devs[node_id][\u0027device\u0027]))"},{"line_number":6354,"context_line":"        self.assertIn(expected_log_line, info_lines)"},{"line_number":6355,"context_line":""},{"line_number":6356,"context_line":"        # If there are also no local_dev_ids, not that I\u0027m sure this is even"},{"line_number":6357,"context_line":"        # possible, then we\u0027ll get the DeviceUnavailable error"},{"line_number":6358,"context_line":"        with self._mock_sharder() as sharder:"},{"line_number":6359,"context_line":"            sharder._local_device_ids \u003d {}"},{"line_number":6360,"context_line":"            with self.assertRaises(DeviceUnavailable) as dev_err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"b84857b1_011c345a","line":6357,"range":{"start_line":6356,"start_character":46,"end_line":6357,"end_character":18},"in_reply_to":"15bbfca3_1dfdc639","updated":"2022-07-25 06:59:36.000000000","message":"yeah, I guess that could be possible. In any case it\u0027s good we\u0027re testing what would happen if it was empty.","commit_id":"cf4e688347cea13c98c65bf469b03260c57ee980"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"27b0e0342ad87e7d140546fc6ec933a8b495b22f","unresolved":true,"context_lines":[{"line_number":750,"context_line":"            expected \u003d \u0027Skipping %s as it is not mounted\u0027 % \\"},{"line_number":751,"context_line":"                unmounted_dev[\u0027device\u0027]"},{"line_number":752,"context_line":"            self.assertIn(expected, lines[0])"},{"line_number":753,"context_line":"            self.assertEqual(device_ids, sharder._local_device_ids.keys())"},{"line_number":754,"context_line":"            self.assertEqual(3, mock_process_broker.call_count)"},{"line_number":755,"context_line":"            processed_paths \u003d [call[0][0].path"},{"line_number":756,"context_line":"                               for call in mock_process_broker.call_args_list]"}],"source_content_type":"text/x-python","patch_set":3,"id":"6c5e533c_42b22658","line":753,"updated":"2022-07-22 09:26:02.000000000","message":"for py2.7 you\u0027ll need to cast the dict keys to a set for the equality to be true\n\n{1,2,3} \u003d\u003d {1:1, 2:2, 3:3}.keys() is False in py2.7 but true in py3","commit_id":"0576b4619e31845993a01c49cca83e586d541bfe"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6de72558c942d21a100b36d1d42256fee7c1549c","unresolved":false,"context_lines":[{"line_number":750,"context_line":"            expected \u003d \u0027Skipping %s as it is not mounted\u0027 % \\"},{"line_number":751,"context_line":"                unmounted_dev[\u0027device\u0027]"},{"line_number":752,"context_line":"            self.assertIn(expected, lines[0])"},{"line_number":753,"context_line":"            self.assertEqual(device_ids, sharder._local_device_ids.keys())"},{"line_number":754,"context_line":"            self.assertEqual(3, mock_process_broker.call_count)"},{"line_number":755,"context_line":"            processed_paths \u003d [call[0][0].path"},{"line_number":756,"context_line":"                               for call in mock_process_broker.call_args_list]"}],"source_content_type":"text/x-python","patch_set":3,"id":"aedd5282_2407800b","line":753,"in_reply_to":"6c5e533c_42b22658","updated":"2022-07-25 06:59:36.000000000","message":"Done","commit_id":"0576b4619e31845993a01c49cca83e586d541bfe"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"920d0d388965b04f09e69b7f1de53c8130fe5d52","unresolved":true,"context_lines":[{"line_number":6395,"context_line":"        expected_error_string \u003d ("},{"line_number":6396,"context_line":"            \u0027No mounted devices found suitable for creating shard broker \u0027"},{"line_number":6397,"context_line":"            \u0027for %s in partition %d\u0027 % (quote(shard_ranges[0].name), part))"},{"line_number":6398,"context_line":"        self.assertEqual(str(dev_err.exception), expected_error_string)"},{"line_number":6399,"context_line":""},{"line_number":6400,"context_line":""},{"line_number":6401,"context_line":"class TestCleavingContext(BaseTestSharder):"}],"source_content_type":"text/x-python","patch_set":5,"id":"2cd630e4_ea9e0503","line":6398,"updated":"2022-07-27 15:47:46.000000000","message":"do we need a similar test for the replicator reconciler containers?","commit_id":"a210079b8feac07cc9893e4d6f17858b57b349e0"}]}