)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"9b3ecce48a7cf099f425fd82aa6217571baa912d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6c9bc00b_fea6fe07","updated":"2022-08-12 06:59:53.000000000","message":"I like the approach, only pull from root if we have a gap (ie a problem) and only merge them if it\u0027ll fix the issue.","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49e2fb2bc48accc2dae510e0251b26ee347f19ef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9ff45982_5a7a5a41","updated":"2022-09-21 18:29:42.000000000","message":"I think this maybe closes a bug?  well, but maybe only if you\u0027ve done something less than ideal, which you maybe can\u0027t even do anymore, with s-m-s-r \n\nI need to go through the unittests more","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7ba9359f0c109bee2bc21f78666747472b24b1f2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"e80f87fb_e72ab47c","updated":"2022-09-29 20:36:34.000000000","message":"getting closer to done","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5a7de2e86a9d8a9ece2ec01c768f2f4ee9c26b91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"9228acbd_aaef2102","updated":"2022-09-30 10:24:01.000000000","message":"ready for review","commit_id":"79a036a0379f981d76c2661a545fbf95db3be7e8"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"19da9d2235ba34217541050ed3e5b4b4bb84b2ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"7d31ece5_ecaf56aa","updated":"2022-10-31 02:47:55.000000000","message":"I am coming on board. I do like the idea doing best effort in checking ancestors and checking for gaps and overlaps.\n\nSome random questions and thoughts inline. Pushing them up, while I think on them some more :)\nI haven\u0027t had much loading it into my head time yet, so things are probably all accounted for, just getting thoughts in order :) ","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2e73a4d2842748e7181aa2638627b5cfeef5b338","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"87481592_178c9e61","updated":"2022-11-14 01:40:14.000000000","message":"OK, I feel like I\u0027ve looked over this enough, and it seems like a win. Trying to think of edge cases, and I think we\u0027d see things like overlaps and other warnings. I do wonder if we can use some of this parent/child/grandparent logic in audit, so we we do dectect an ACTIVE grandparent, we can do an automatic repair. Because an ancester should be SHARDED/SHRUNK. ","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4446d4e09ba5be9400bfd7256ad581b5f982feb6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"c4830138_7bd4e95f","updated":"2022-10-19 21:41:35.000000000","message":"recheck\n\nPretty sure those py27 failures should be resolved now that https://review.opendev.org/c/openstack/swift/+/861583 landed.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"318bbc8370d55bf758d75450f450a767e84c26fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"9b074ef1_5a4dd9fb","updated":"2022-11-17 03:22:56.000000000","message":"I feel this patch is good enough, even though there may be a very rare case but Alistair already has a follow up is_grandchild_of patch to address it.","commit_id":"2bcf3d1a8eb2e465b3d7afea7a6efb3b4a7d43e8"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"90e9a11adaa2cc00f5b46881d2a002cd1f4e8ada","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"b7728014_e37b87af","updated":"2022-11-17 08:18:41.000000000","message":"Looks good, shall we land this.. I say.. yes, let\u0027s give it a whirl!","commit_id":"2bcf3d1a8eb2e465b3d7afea7a6efb3b4a7d43e8"}],"swift/common/utils.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f3f131c204867a67989bd3531d041f1cf365d7fc","unresolved":true,"context_lines":[{"line_number":5290,"context_line":"            return False"},{"line_number":5291,"context_line":"        try:"},{"line_number":5292,"context_line":"            root, parent_hash, timestamp, index \u003d self.parse_container_name()"},{"line_number":5293,"context_line":"            if parent.container \u003d\u003d root:"},{"line_number":5294,"context_line":"                return True"},{"line_number":5295,"context_line":"            parent_root \u003d parent.parse_container_name()[0]"},{"line_number":5296,"context_line":"            if (self.container \u003d\u003d self._make_container_name("}],"source_content_type":"text/x-python","patch_set":2,"id":"7ace8146_7401b0e2","line":5293,"range":{"start_line":5293,"start_character":12,"end_line":5293,"end_character":40},"updated":"2022-08-15 07:08:16.000000000","message":"So if the parent.container is the root container... but that can never happen here because if the parent is the root OSR then it\u0027ll fail on the account check on line 5289 right, because \u003caccount\u003e !\u003d .sharded_\u003caccount\u003e.","commit_id":"e187a46476bb98ce81db0382ddd2e192550ec32a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":5290,"context_line":"            return False"},{"line_number":5291,"context_line":"        try:"},{"line_number":5292,"context_line":"            root, parent_hash, timestamp, index \u003d self.parse_container_name()"},{"line_number":5293,"context_line":"            if parent.container \u003d\u003d root:"},{"line_number":5294,"context_line":"                return True"},{"line_number":5295,"context_line":"            parent_root \u003d parent.parse_container_name()[0]"},{"line_number":5296,"context_line":"            if (self.container \u003d\u003d self._make_container_name("}],"source_content_type":"text/x-python","patch_set":2,"id":"f92dbb70_e5662a7b","line":5293,"range":{"start_line":5293,"start_character":12,"end_line":5293,"end_character":40},"in_reply_to":"553c5598_fc0d8b8b","updated":"2022-09-21 15:59:39.000000000","message":"Done","commit_id":"e187a46476bb98ce81db0382ddd2e192550ec32a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"590cf9e3de876f0d43b7ea4858a687f7813261f9","unresolved":true,"context_lines":[{"line_number":5290,"context_line":"            return False"},{"line_number":5291,"context_line":"        try:"},{"line_number":5292,"context_line":"            root, parent_hash, timestamp, index \u003d self.parse_container_name()"},{"line_number":5293,"context_line":"            if parent.container \u003d\u003d root:"},{"line_number":5294,"context_line":"                return True"},{"line_number":5295,"context_line":"            parent_root \u003d parent.parse_container_name()[0]"},{"line_number":5296,"context_line":"            if (self.container \u003d\u003d self._make_container_name("}],"source_content_type":"text/x-python","patch_set":2,"id":"553c5598_fc0d8b8b","line":5293,"range":{"start_line":5293,"start_character":12,"end_line":5293,"end_character":40},"in_reply_to":"7ace8146_7401b0e2","updated":"2022-08-22 17:31:37.000000000","message":"oh, yes I think you are correct\n\nneed to fix this so that it works for children of root","commit_id":"e187a46476bb98ce81db0382ddd2e192550ec32a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f3f131c204867a67989bd3531d041f1cf365d7fc","unresolved":true,"context_lines":[{"line_number":5294,"context_line":"                return True"},{"line_number":5295,"context_line":"            parent_root \u003d parent.parse_container_name()[0]"},{"line_number":5296,"context_line":"            if (self.container \u003d\u003d self._make_container_name("},{"line_number":5297,"context_line":"                    parent_root, parent.container, timestamp, index)):"},{"line_number":5298,"context_line":"                return True"},{"line_number":5299,"context_line":"        except:  # noqa"},{"line_number":5300,"context_line":"            # TODO: so many unit tests do not construct valid names!"}],"source_content_type":"text/x-python","patch_set":2,"id":"58a4b82a_b5dc141a","line":5297,"updated":"2022-08-15 07:08:16.000000000","message":"So this check is basically checking to see if the parent container (when hashed) matches what we have?\n\nIf we\u0027re comparing accounts, assuming were could be mix of shards, do we also need to check parent_root and root, incase we\u0027ve got same account and different root containers?","commit_id":"e187a46476bb98ce81db0382ddd2e192550ec32a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":5294,"context_line":"                return True"},{"line_number":5295,"context_line":"            parent_root \u003d parent.parse_container_name()[0]"},{"line_number":5296,"context_line":"            if (self.container \u003d\u003d self._make_container_name("},{"line_number":5297,"context_line":"                    parent_root, parent.container, timestamp, index)):"},{"line_number":5298,"context_line":"                return True"},{"line_number":5299,"context_line":"        except:  # noqa"},{"line_number":5300,"context_line":"            # TODO: so many unit tests do not construct valid names!"}],"source_content_type":"text/x-python","patch_set":2,"id":"fcffbb8c_0abc6ca1","line":5297,"in_reply_to":"58a4b82a_b5dc141a","updated":"2022-09-21 15:59:39.000000000","message":"Done","commit_id":"e187a46476bb98ce81db0382ddd2e192550ec32a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7ba9359f0c109bee2bc21f78666747472b24b1f2","unresolved":true,"context_lines":[{"line_number":5430,"context_line":"        return None"},{"line_number":5431,"context_line":""},{"line_number":5432,"context_line":"    def find_root(self, shard_ranges):"},{"line_number":5433,"context_line":"        try:"},{"line_number":5434,"context_line":"            self_parsed_name \u003d ShardName.parse(self.name)"},{"line_number":5435,"context_line":"        except ValueError:"},{"line_number":5436,"context_line":"            # not a shard"}],"source_content_type":"text/x-python","patch_set":12,"id":"f5bcec9b_dfff177b","line":5433,"updated":"2022-09-29 20:36:34.000000000","message":"needs unit test","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743f2d16a6287bc99e19a9e3b27301360ee41776","unresolved":false,"context_lines":[{"line_number":5430,"context_line":"        return None"},{"line_number":5431,"context_line":""},{"line_number":5432,"context_line":"    def find_root(self, shard_ranges):"},{"line_number":5433,"context_line":"        try:"},{"line_number":5434,"context_line":"            self_parsed_name \u003d ShardName.parse(self.name)"},{"line_number":5435,"context_line":"        except ValueError:"},{"line_number":5436,"context_line":"            # not a shard"}],"source_content_type":"text/x-python","patch_set":12,"id":"600f3f17_a9f2057e","line":5433,"in_reply_to":"f5bcec9b_dfff177b","updated":"2022-09-30 10:21:30.000000000","message":"Done","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"058cfaf0bbde4f5c386ad6ad83d3a6533edbe1b3","unresolved":true,"context_lines":[{"line_number":5473,"context_line":"        \"\"\""},{"line_number":5474,"context_line":"        ancestors \u003d []"},{"line_number":5475,"context_line":"        if not shard_ranges:"},{"line_number":5476,"context_line":"            return ancestors"},{"line_number":5477,"context_line":""},{"line_number":5478,"context_line":"        try:"},{"line_number":5479,"context_line":"            self_parsed_name \u003d ShardName.parse(self.name)"}],"source_content_type":"text/x-python","patch_set":14,"id":"a170dc5b_9e1b346d","line":5476,"updated":"2022-11-16 06:10:48.000000000","message":"nit: return []","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9913a1a1c9dc131deb9700fac583e5e327505575","unresolved":false,"context_lines":[{"line_number":5473,"context_line":"        \"\"\""},{"line_number":5474,"context_line":"        ancestors \u003d []"},{"line_number":5475,"context_line":"        if not shard_ranges:"},{"line_number":5476,"context_line":"            return ancestors"},{"line_number":5477,"context_line":""},{"line_number":5478,"context_line":"        try:"},{"line_number":5479,"context_line":"            self_parsed_name \u003d ShardName.parse(self.name)"}],"source_content_type":"text/x-python","patch_set":14,"id":"0e6ee461_c06e6f41","line":5476,"in_reply_to":"a170dc5b_9e1b346d","updated":"2022-11-16 16:19:56.000000000","message":"Done","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"058cfaf0bbde4f5c386ad6ad83d3a6533edbe1b3","unresolved":true,"context_lines":[{"line_number":5479,"context_line":"            self_parsed_name \u003d ShardName.parse(self.name)"},{"line_number":5480,"context_line":"        except ValueError:"},{"line_number":5481,"context_line":"            # not a shard"},{"line_number":5482,"context_line":"            return ancestors"},{"line_number":5483,"context_line":""},{"line_number":5484,"context_line":"        for sr in shard_ranges:"},{"line_number":5485,"context_line":"            if self.is_child_of(sr):"}],"source_content_type":"text/x-python","patch_set":14,"id":"93d79e37_363c4ad1","line":5482,"updated":"2022-11-16 06:10:48.000000000","message":"nit: return []","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9913a1a1c9dc131deb9700fac583e5e327505575","unresolved":false,"context_lines":[{"line_number":5479,"context_line":"            self_parsed_name \u003d ShardName.parse(self.name)"},{"line_number":5480,"context_line":"        except ValueError:"},{"line_number":5481,"context_line":"            # not a shard"},{"line_number":5482,"context_line":"            return ancestors"},{"line_number":5483,"context_line":""},{"line_number":5484,"context_line":"        for sr in shard_ranges:"},{"line_number":5485,"context_line":"            if self.is_child_of(sr):"}],"source_content_type":"text/x-python","patch_set":14,"id":"71159e7e_0b88493f","line":5482,"in_reply_to":"93d79e37_363c4ad1","updated":"2022-11-16 16:19:56.000000000","message":"Done","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"59199b84ce3d0cdcb1fde49339084811b24ca363","unresolved":true,"context_lines":[{"line_number":5488,"context_line":"        if ancestors:"},{"line_number":5489,"context_line":"            ancestors.extend(ancestors[0].find_ancestors(shard_ranges))"},{"line_number":5490,"context_line":"        else:"},{"line_number":5491,"context_line":"            root_sr \u003d self._find_root(self_parsed_name, shard_ranges)"},{"line_number":5492,"context_line":"            if root_sr:"},{"line_number":5493,"context_line":"                ancestors.append(root_sr)"},{"line_number":5494,"context_line":"        return ancestors"}],"source_content_type":"text/x-python","patch_set":14,"id":"48467efd_57536c08","line":5491,"updated":"2022-10-28 05:57:30.000000000","message":"should \u0027_find_root\u0027 be called for the case of line 5488 too? for example, in case of missing great-grandparent shard range as mentioned in the comments above.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"957cc623cd02b60802756065b1034c0c6123eff6","unresolved":true,"context_lines":[{"line_number":5488,"context_line":"        if ancestors:"},{"line_number":5489,"context_line":"            ancestors.extend(ancestors[0].find_ancestors(shard_ranges))"},{"line_number":5490,"context_line":"        else:"},{"line_number":5491,"context_line":"            root_sr \u003d self._find_root(self_parsed_name, shard_ranges)"},{"line_number":5492,"context_line":"            if root_sr:"},{"line_number":5493,"context_line":"                ancestors.append(root_sr)"},{"line_number":5494,"context_line":"        return ancestors"}],"source_content_type":"text/x-python","patch_set":14,"id":"7222214f_04b57135","line":5491,"in_reply_to":"48467efd_57536c08","updated":"2022-11-07 16:10:04.000000000","message":"If there are ancestors, then the call to ancestors[0].find_ancestors(shard_ranges) will append the root (i.e. we\u0027re delegating adding the root to the final ancestor). We don\u0027t want to append the root multiple times.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"058cfaf0bbde4f5c386ad6ad83d3a6533edbe1b3","unresolved":false,"context_lines":[{"line_number":5488,"context_line":"        if ancestors:"},{"line_number":5489,"context_line":"            ancestors.extend(ancestors[0].find_ancestors(shard_ranges))"},{"line_number":5490,"context_line":"        else:"},{"line_number":5491,"context_line":"            root_sr \u003d self._find_root(self_parsed_name, shard_ranges)"},{"line_number":5492,"context_line":"            if root_sr:"},{"line_number":5493,"context_line":"                ancestors.append(root_sr)"},{"line_number":5494,"context_line":"        return ancestors"}],"source_content_type":"text/x-python","patch_set":14,"id":"1bebbdf0_96cf6a9a","line":5491,"in_reply_to":"7222214f_04b57135","updated":"2022-11-16 06:10:48.000000000","message":"Ack","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"}],"swift/container/sharder.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7371089b6c87057fa5e2006eb83b99cac6a057f4","unresolved":true,"context_lines":[{"line_number":1275,"context_line":"                                           ShardRange.SHARDED):"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"                # TODO: should we include deleted? (no?)"},{"line_number":1278,"context_line":"                # TODO: do we need to filter out SHRINKING? (no?)"},{"line_number":1279,"context_line":"                shard_ranges \u003d [sr for sr in broker.get_shard_ranges()"},{"line_number":1280,"context_line":"                                if sr.state !\u003d ShardRange.SHRINKING]"},{"line_number":1281,"context_line":"                paths_with_gaps \u003d find_paths_with_gaps(shard_ranges,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1bd7e536_d5a4fbc2","line":1278,"updated":"2022-08-12 06:59:12.000000000","message":"Hmm, I guess if something is deleted something out there must think there is a better solution right? So maybe it\u0027s ok to not include deleted. In the off chance its a stuck range?\n\nHow would it get something as deleted though, I guess from the root, or a replica (which must\u0027ve got it from the root).","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":1275,"context_line":"                                           ShardRange.SHARDED):"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"                # TODO: should we include deleted? (no?)"},{"line_number":1278,"context_line":"                # TODO: do we need to filter out SHRINKING? (no?)"},{"line_number":1279,"context_line":"                shard_ranges \u003d [sr for sr in broker.get_shard_ranges()"},{"line_number":1280,"context_line":"                                if sr.state !\u003d ShardRange.SHRINKING]"},{"line_number":1281,"context_line":"                paths_with_gaps \u003d find_paths_with_gaps(shard_ranges,"}],"source_content_type":"text/x-python","patch_set":1,"id":"50c818bc_4cb05387","line":1278,"in_reply_to":"1bd7e536_d5a4fbc2","updated":"2022-09-21 15:59:39.000000000","message":"Done","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7371089b6c87057fa5e2006eb83b99cac6a057f4","unresolved":true,"context_lines":[{"line_number":1297,"context_line":"                    # what will we end up with IF we did merge root ranges..."},{"line_number":1298,"context_line":"                    combined_shard_ranges \u003d combine_shard_ranges("},{"line_number":1299,"context_line":"                        root_shard_ranges, shard_ranges)"},{"line_number":1300,"context_line":"                    overlaps \u003d find_overlapping_ranges(combined_shard_ranges)"},{"line_number":1301,"context_line":"                    if not overlaps:"},{"line_number":1302,"context_line":"                        # TODO: what about gaps in root ranges??"},{"line_number":1303,"context_line":"                        broker.merge_shard_ranges(combined_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ce20af38_aafa1302","line":1300,"updated":"2022-08-12 06:59:12.000000000","message":"root_shard_ranges I think includes the root OSR, although it should be in SHARDED state. But could it be found as an overlap?\n\nWe probably need a test for that.","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":1297,"context_line":"                    # what will we end up with IF we did merge root ranges..."},{"line_number":1298,"context_line":"                    combined_shard_ranges \u003d combine_shard_ranges("},{"line_number":1299,"context_line":"                        root_shard_ranges, shard_ranges)"},{"line_number":1300,"context_line":"                    overlaps \u003d find_overlapping_ranges(combined_shard_ranges)"},{"line_number":1301,"context_line":"                    if not overlaps:"},{"line_number":1302,"context_line":"                        # TODO: what about gaps in root ranges??"},{"line_number":1303,"context_line":"                        broker.merge_shard_ranges(combined_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e2d54bd8_3ce572f2","line":1300,"in_reply_to":"ce20af38_aafa1302","updated":"2022-09-21 15:59:39.000000000","message":"Done","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7371089b6c87057fa5e2006eb83b99cac6a057f4","unresolved":true,"context_lines":[{"line_number":1299,"context_line":"                        root_shard_ranges, shard_ranges)"},{"line_number":1300,"context_line":"                    overlaps \u003d find_overlapping_ranges(combined_shard_ranges)"},{"line_number":1301,"context_line":"                    if not overlaps:"},{"line_number":1302,"context_line":"                        # TODO: what about gaps in root ranges??"},{"line_number":1303,"context_line":"                        broker.merge_shard_ranges(combined_shard_ranges)"},{"line_number":1304,"context_line":""},{"line_number":1305,"context_line":"        delete_age \u003d time.time() - self.reclaim_age"}],"source_content_type":"text/x-python","patch_set":1,"id":"d968f850_3e9d7242","line":1302,"updated":"2022-08-12 06:59:12.000000000","message":"if there are gaps in root ranges, the root audit will find that right?\n\nI guess there might be chance that there is a gap in the root, and somehow a stale shard not in the root is in this shard that does makes a full path.. but I think those chances are slim.","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":1299,"context_line":"                        root_shard_ranges, shard_ranges)"},{"line_number":1300,"context_line":"                    overlaps \u003d find_overlapping_ranges(combined_shard_ranges)"},{"line_number":1301,"context_line":"                    if not overlaps:"},{"line_number":1302,"context_line":"                        # TODO: what about gaps in root ranges??"},{"line_number":1303,"context_line":"                        broker.merge_shard_ranges(combined_shard_ranges)"},{"line_number":1304,"context_line":""},{"line_number":1305,"context_line":"        delete_age \u003d time.time() - self.reclaim_age"}],"source_content_type":"text/x-python","patch_set":1,"id":"cbd9e87f_1ab385b9","line":1302,"in_reply_to":"d968f850_3e9d7242","updated":"2022-09-21 15:59:39.000000000","message":"Ack","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5bc8e857284749a1cf6cab922f3c54ca59e1c4d5","unresolved":true,"context_lines":[{"line_number":1309,"context_line":"                        sr for sr in shard_ranges_from_root"},{"line_number":1310,"context_line":"                        if (sr is not own_shard_range_from_root"},{"line_number":1311,"context_line":"                            and not own_shard_range.is_child_of(sr)"},{"line_number":1312,"context_line":"                            # TODO: what about ancestors above parent?"},{"line_number":1313,"context_line":"                            and sr.name !\u003d broker.root_path"},{"line_number":1314,"context_line":"                            and sr.state not in ShardRange.CLEAVING_STATES)]"},{"line_number":1315,"context_line":"                    combined_shard_ranges \u003d combine_shard_ranges("}],"source_content_type":"text/x-python","patch_set":2,"id":"c689749e_c439382b","line":1312,"range":{"start_line":1312,"start_character":28,"end_line":1312,"end_character":70},"updated":"2022-08-12 17:45:47.000000000","message":"we can compare the timestamps embedded in the shard range names and assert that we\u0027d never shard into something younger than ourself","commit_id":"e187a46476bb98ce81db0382ddd2e192550ec32a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":1309,"context_line":"                        sr for sr in shard_ranges_from_root"},{"line_number":1310,"context_line":"                        if (sr is not own_shard_range_from_root"},{"line_number":1311,"context_line":"                            and not own_shard_range.is_child_of(sr)"},{"line_number":1312,"context_line":"                            # TODO: what about ancestors above parent?"},{"line_number":1313,"context_line":"                            and sr.name !\u003d broker.root_path"},{"line_number":1314,"context_line":"                            and sr.state not in ShardRange.CLEAVING_STATES)]"},{"line_number":1315,"context_line":"                    combined_shard_ranges \u003d combine_shard_ranges("}],"source_content_type":"text/x-python","patch_set":2,"id":"430075be_9e1b367f","line":1312,"range":{"start_line":1312,"start_character":28,"end_line":1312,"end_character":70},"in_reply_to":"c689749e_c439382b","updated":"2022-09-21 15:59:39.000000000","message":"but we may need to shard to an acceptor that IS younger","commit_id":"e187a46476bb98ce81db0382ddd2e192550ec32a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c37381aa407a98045fff9972ceede90a3c8311fa","unresolved":true,"context_lines":[{"line_number":1284,"context_line":"        other_shard_ranges_from_root \u003d ["},{"line_number":1285,"context_line":"            sr for sr in shard_ranges_from_root or []"},{"line_number":1286,"context_line":"            if sr is not own_shard_range_from_root]"},{"line_number":1287,"context_line":"        children_shard_ranges \u003d [sr for sr in other_shard_ranges_from_root"},{"line_number":1288,"context_line":"                                 if sr.is_child_of(own_shard_range)]"},{"line_number":1289,"context_line":"        if children_shard_ranges:"},{"line_number":1290,"context_line":"            self.logger.debug(\u0027Updating children shard ranges from root\u0027)"},{"line_number":1291,"context_line":"            broker.merge_shard_ranges(children_shard_ranges)"},{"line_number":1292,"context_line":""},{"line_number":1293,"context_line":"        # other shard ranges returned from the root may need to be merged"},{"line_number":1294,"context_line":"        # for the purposes of sharding or shrinking this shard, but exclude"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f799e6e_037ec718","line":1291,"range":{"start_line":1287,"start_character":8,"end_line":1291,"end_character":60},"updated":"2022-09-14 17:14:43.000000000","message":"I wonder if this part could be broken out as a preceding patch i.e. always accept children shard ranges from root","commit_id":"1d39734d9170bb7fa49b67bdecf666a988508aee"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":1284,"context_line":"        other_shard_ranges_from_root \u003d ["},{"line_number":1285,"context_line":"            sr for sr in shard_ranges_from_root or []"},{"line_number":1286,"context_line":"            if sr is not own_shard_range_from_root]"},{"line_number":1287,"context_line":"        children_shard_ranges \u003d [sr for sr in other_shard_ranges_from_root"},{"line_number":1288,"context_line":"                                 if sr.is_child_of(own_shard_range)]"},{"line_number":1289,"context_line":"        if children_shard_ranges:"},{"line_number":1290,"context_line":"            self.logger.debug(\u0027Updating children shard ranges from root\u0027)"},{"line_number":1291,"context_line":"            broker.merge_shard_ranges(children_shard_ranges)"},{"line_number":1292,"context_line":""},{"line_number":1293,"context_line":"        # other shard ranges returned from the root may need to be merged"},{"line_number":1294,"context_line":"        # for the purposes of sharding or shrinking this shard, but exclude"}],"source_content_type":"text/x-python","patch_set":4,"id":"09426e93_0f50a701","line":1291,"range":{"start_line":1287,"start_character":8,"end_line":1291,"end_character":60},"in_reply_to":"5f799e6e_037ec718","updated":"2022-09-21 15:59:39.000000000","message":"Done","commit_id":"1d39734d9170bb7fa49b67bdecf666a988508aee"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c37381aa407a98045fff9972ceede90a3c8311fa","unresolved":true,"context_lines":[{"line_number":1312,"context_line":"        elif (other_shard_ranges_from_root"},{"line_number":1313,"context_line":"              and own_shard_range.state in ShardRange.SHARDING_STATES"},{"line_number":1314,"context_line":"                and not broker.is_sharded()):"},{"line_number":1315,"context_line":"            # If the up-to-date own_shard_range state indicates that"},{"line_number":1316,"context_line":"            # sharding is in progress then this shard DB would normally"},{"line_number":1317,"context_line":"            # have a complete set of shard ranges, either directly set or"},{"line_number":1318,"context_line":"            # eventually replicated from its peers. However, if the current"},{"line_number":1319,"context_line":"            # shard ranges are incomplete then sharding won\u0027t be able to"},{"line_number":1320,"context_line":"            # complete, so consider merging any useful shard ranges"},{"line_number":1321,"context_line":"            # returned from the root. This is only necessary until this DB"},{"line_number":1322,"context_line":"            # is fully cleaved and reaches SHARDED state, after which it is"},{"line_number":1323,"context_line":"            # normal for sub-shards to also shard and be deleted in the"},{"line_number":1324,"context_line":"            # root\u0027s shard range table, resulting in further unnecessary"},{"line_number":1325,"context_line":"            # updates to this shard\u0027s sub-shards."},{"line_number":1326,"context_line":"            # TODO: for debugging it seems useful for sharded shards to be"},{"line_number":1327,"context_line":"            #   frozen with the set of children that they sharded, but we"},{"line_number":1328,"context_line":"            #   could choose to have them continue to be updated from root"},{"line_number":1329,"context_line":"            #   with the current set of *active* descendant shard ranges"},{"line_number":1330,"context_line":"            # Normally a shard will shard to its own children. We never"},{"line_number":1331,"context_line":"            # expect a shard DB to shard to any ancestor including the"},{"line_number":1332,"context_line":"            # root. We also don\u0027t want to shard to a container that is"},{"line_number":1333,"context_line":"            # itself sharding or shrinking."},{"line_number":1334,"context_line":"            ancestor_names \u003d ["},{"line_number":1335,"context_line":"                sr.name for sr in own_shard_range.find_ancestors("},{"line_number":1336,"context_line":"                    shard_ranges_from_root)]"},{"line_number":1337,"context_line":"            # Containers never shard to an ancestor, including the"},{"line_number":1338,"context_line":"            # root; containers might ultimately *shrink* to root,"},{"line_number":1339,"context_line":"            # but that is handled elsewhere."},{"line_number":1340,"context_line":"            # Always update deleted shard ranges."},{"line_number":1341,"context_line":"            # containers should not shard to a container that is"},{"line_number":1342,"context_line":"            # itself sharding or shrinking"},{"line_number":1343,"context_line":"            other_shard_ranges_from_root \u003d ["},{"line_number":1344,"context_line":"                sr for sr in other_shard_ranges_from_root"},{"line_number":1345,"context_line":"                if (sr.name not in ancestor_names"}],"source_content_type":"text/x-python","patch_set":4,"id":"a180b1a2_89f4bea7","line":1342,"range":{"start_line":1315,"start_character":13,"end_line":1342,"end_character":42},"updated":"2022-09-14 17:14:43.000000000","message":"need to clean up all the commentary here!","commit_id":"1d39734d9170bb7fa49b67bdecf666a988508aee"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":false,"context_lines":[{"line_number":1312,"context_line":"        elif (other_shard_ranges_from_root"},{"line_number":1313,"context_line":"              and own_shard_range.state in ShardRange.SHARDING_STATES"},{"line_number":1314,"context_line":"                and not broker.is_sharded()):"},{"line_number":1315,"context_line":"            # If the up-to-date own_shard_range state indicates that"},{"line_number":1316,"context_line":"            # sharding is in progress then this shard DB would normally"},{"line_number":1317,"context_line":"            # have a complete set of shard ranges, either directly set or"},{"line_number":1318,"context_line":"            # eventually replicated from its peers. However, if the current"},{"line_number":1319,"context_line":"            # shard ranges are incomplete then sharding won\u0027t be able to"},{"line_number":1320,"context_line":"            # complete, so consider merging any useful shard ranges"},{"line_number":1321,"context_line":"            # returned from the root. This is only necessary until this DB"},{"line_number":1322,"context_line":"            # is fully cleaved and reaches SHARDED state, after which it is"},{"line_number":1323,"context_line":"            # normal for sub-shards to also shard and be deleted in the"},{"line_number":1324,"context_line":"            # root\u0027s shard range table, resulting in further unnecessary"},{"line_number":1325,"context_line":"            # updates to this shard\u0027s sub-shards."},{"line_number":1326,"context_line":"            # TODO: for debugging it seems useful for sharded shards to be"},{"line_number":1327,"context_line":"            #   frozen with the set of children that they sharded, but we"},{"line_number":1328,"context_line":"            #   could choose to have them continue to be updated from root"},{"line_number":1329,"context_line":"            #   with the current set of *active* descendant shard ranges"},{"line_number":1330,"context_line":"            # Normally a shard will shard to its own children. We never"},{"line_number":1331,"context_line":"            # expect a shard DB to shard to any ancestor including the"},{"line_number":1332,"context_line":"            # root. We also don\u0027t want to shard to a container that is"},{"line_number":1333,"context_line":"            # itself sharding or shrinking."},{"line_number":1334,"context_line":"            ancestor_names \u003d ["},{"line_number":1335,"context_line":"                sr.name for sr in own_shard_range.find_ancestors("},{"line_number":1336,"context_line":"                    shard_ranges_from_root)]"},{"line_number":1337,"context_line":"            # Containers never shard to an ancestor, including the"},{"line_number":1338,"context_line":"            # root; containers might ultimately *shrink* to root,"},{"line_number":1339,"context_line":"            # but that is handled elsewhere."},{"line_number":1340,"context_line":"            # Always update deleted shard ranges."},{"line_number":1341,"context_line":"            # containers should not shard to a container that is"},{"line_number":1342,"context_line":"            # itself sharding or shrinking"},{"line_number":1343,"context_line":"            other_shard_ranges_from_root \u003d ["},{"line_number":1344,"context_line":"                sr for sr in other_shard_ranges_from_root"},{"line_number":1345,"context_line":"                if (sr.name not in ancestor_names"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f49eda6_1a64dd28","line":1342,"range":{"start_line":1315,"start_character":13,"end_line":1342,"end_character":42},"in_reply_to":"a180b1a2_89f4bea7","updated":"2022-09-21 15:59:39.000000000","message":"Done","commit_id":"1d39734d9170bb7fa49b67bdecf666a988508aee"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6df1745fbad8ddeac436677b360d4b8f2d4da18e","unresolved":true,"context_lines":[{"line_number":1284,"context_line":"            # sub-shards. For example, a shard range repair may cause a child"},{"line_number":1285,"context_line":"            # sub-shard to be deleted and its namespace covered by another"},{"line_number":1286,"context_line":"            # \u0027acceptor\u0027 shard. Therefore, if the up-to-date own_shard_range"},{"line_number":1287,"context_line":"            # state indicates that sharding is in progress, then other shard"},{"line_number":1288,"context_line":"            # ranges will be merged, with the following caveats: we never"},{"line_number":1289,"context_line":"            # expect a shard to shard to any ancestor shard range including the"},{"line_number":1290,"context_line":"            # root (containers might ultimately *shrink* to root, but that is"},{"line_number":1291,"context_line":"            # handled elsewhere); we never want to shard to a container that is"},{"line_number":1292,"context_line":"            # itself sharding or shrinking; the merged shard ranges should not"},{"line_number":1293,"context_line":"            # result in gaps or overlaps in the namespace of this shard. This"},{"line_number":1294,"context_line":"            # is only necessary until this DB is fully cleaved and reaches"},{"line_number":1295,"context_line":"            # SHARDED state, after which it is useful for debugging to freeze"},{"line_number":1296,"context_line":"            # the set of sub-shards to which a shards has sharded."},{"line_number":1297,"context_line":"            ancestor_names \u003d ["},{"line_number":1298,"context_line":"                sr.name for sr in own_shard_range.find_ancestors(shard_ranges)]"},{"line_number":1299,"context_line":"            other_shard_ranges \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"411ac544_1f18e79b","line":1296,"range":{"start_line":1287,"start_character":65,"end_line":1296,"end_character":66},"updated":"2022-09-21 15:59:39.000000000","message":"I think the same statements could also be applied to the shrinking case, except we do sometimes what to shrink to the root, so I\u0027m wondering if we can generalise the two cases (sharding and shrinking).","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7ba9359f0c109bee2bc21f78666747472b24b1f2","unresolved":false,"context_lines":[{"line_number":1284,"context_line":"            # sub-shards. For example, a shard range repair may cause a child"},{"line_number":1285,"context_line":"            # sub-shard to be deleted and its namespace covered by another"},{"line_number":1286,"context_line":"            # \u0027acceptor\u0027 shard. Therefore, if the up-to-date own_shard_range"},{"line_number":1287,"context_line":"            # state indicates that sharding is in progress, then other shard"},{"line_number":1288,"context_line":"            # ranges will be merged, with the following caveats: we never"},{"line_number":1289,"context_line":"            # expect a shard to shard to any ancestor shard range including the"},{"line_number":1290,"context_line":"            # root (containers might ultimately *shrink* to root, but that is"},{"line_number":1291,"context_line":"            # handled elsewhere); we never want to shard to a container that is"},{"line_number":1292,"context_line":"            # itself sharding or shrinking; the merged shard ranges should not"},{"line_number":1293,"context_line":"            # result in gaps or overlaps in the namespace of this shard. This"},{"line_number":1294,"context_line":"            # is only necessary until this DB is fully cleaved and reaches"},{"line_number":1295,"context_line":"            # SHARDED state, after which it is useful for debugging to freeze"},{"line_number":1296,"context_line":"            # the set of sub-shards to which a shards has sharded."},{"line_number":1297,"context_line":"            ancestor_names \u003d ["},{"line_number":1298,"context_line":"                sr.name for sr in own_shard_range.find_ancestors(shard_ranges)]"},{"line_number":1299,"context_line":"            other_shard_ranges \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"37e24f11_3c345dd3","line":1296,"range":{"start_line":1287,"start_character":65,"end_line":1296,"end_character":66},"in_reply_to":"411ac544_1f18e79b","updated":"2022-09-29 20:36:34.000000000","message":"Done","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49e2fb2bc48accc2dae510e0251b26ee347f19ef","unresolved":true,"context_lines":[{"line_number":1301,"context_line":"                if (sr.name not in ancestor_names"},{"line_number":1302,"context_line":"                    and (sr.state not in ShardRange.CLEAVING_STATES"},{"line_number":1303,"context_line":"                         or sr.deleted))"},{"line_number":1304,"context_line":"            ]"},{"line_number":1305,"context_line":"            existing_shard_ranges \u003d broker.get_shard_ranges()"},{"line_number":1306,"context_line":"            combined_shard_ranges \u003d combine_shard_ranges("},{"line_number":1307,"context_line":"                other_shard_ranges, existing_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":9,"id":"52113ea3_7a94f44c","line":1304,"updated":"2022-09-21 18:29:42.000000000","message":"this says we won\u0027t shrink back into the parent I guess - that\u0027s probably reasonably defensive.","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"957cc623cd02b60802756065b1034c0c6123eff6","unresolved":false,"context_lines":[{"line_number":1301,"context_line":"                if (sr.name not in ancestor_names"},{"line_number":1302,"context_line":"                    and (sr.state not in ShardRange.CLEAVING_STATES"},{"line_number":1303,"context_line":"                         or sr.deleted))"},{"line_number":1304,"context_line":"            ]"},{"line_number":1305,"context_line":"            existing_shard_ranges \u003d broker.get_shard_ranges()"},{"line_number":1306,"context_line":"            combined_shard_ranges \u003d combine_shard_ranges("},{"line_number":1307,"context_line":"                other_shard_ranges, existing_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":9,"id":"b4a64160_cac76493","line":1304,"in_reply_to":"2fb2daab_ea5eccaf","updated":"2022-11-07 16:10:04.000000000","message":"Done","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"815380731e88783ec2d5164d01887747b2fcfe06","unresolved":true,"context_lines":[{"line_number":1301,"context_line":"                if (sr.name not in ancestor_names"},{"line_number":1302,"context_line":"                    and (sr.state not in ShardRange.CLEAVING_STATES"},{"line_number":1303,"context_line":"                         or sr.deleted))"},{"line_number":1304,"context_line":"            ]"},{"line_number":1305,"context_line":"            existing_shard_ranges \u003d broker.get_shard_ranges()"},{"line_number":1306,"context_line":"            combined_shard_ranges \u003d combine_shard_ranges("},{"line_number":1307,"context_line":"                other_shard_ranges, existing_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":9,"id":"2fb2daab_ea5eccaf","line":1304,"in_reply_to":"52113ea3_7a94f44c","updated":"2022-09-23 10:26:15.000000000","message":"no, we won\u0027t *shard* back to a parent - we do allow *shrink* back to parent in the special case that parent is root","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49e2fb2bc48accc2dae510e0251b26ee347f19ef","unresolved":true,"context_lines":[{"line_number":1309,"context_line":"            combined_paths_with_gaps \u003d find_paths_with_gaps("},{"line_number":1310,"context_line":"                combined_shard_ranges, own_shard_range)"},{"line_number":1311,"context_line":"            if not (overlaps or combined_paths_with_gaps):"},{"line_number":1312,"context_line":"                # only merge if shard ranges appear to be *good*"},{"line_number":1313,"context_line":"                self.logger.debug("},{"line_number":1314,"context_line":"                    \u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1315,"context_line":"                    len(other_shard_ranges))"}],"source_content_type":"text/x-python","patch_set":9,"id":"8ad00ad4_2ff0df53","line":1312,"updated":"2022-09-21 18:29:42.000000000","message":"exciting, i think with this generalization of \"good\" (and relevant) we could probably get away with \"always merge a good set of ranges from the root\"","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7ba9359f0c109bee2bc21f78666747472b24b1f2","unresolved":false,"context_lines":[{"line_number":1309,"context_line":"            combined_paths_with_gaps \u003d find_paths_with_gaps("},{"line_number":1310,"context_line":"                combined_shard_ranges, own_shard_range)"},{"line_number":1311,"context_line":"            if not (overlaps or combined_paths_with_gaps):"},{"line_number":1312,"context_line":"                # only merge if shard ranges appear to be *good*"},{"line_number":1313,"context_line":"                self.logger.debug("},{"line_number":1314,"context_line":"                    \u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1315,"context_line":"                    len(other_shard_ranges))"}],"source_content_type":"text/x-python","patch_set":9,"id":"be5debf7_db1d103c","line":1312,"in_reply_to":"46c1efe1_0507b1a0","updated":"2022-09-29 20:36:34.000000000","message":"Done","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"815380731e88783ec2d5164d01887747b2fcfe06","unresolved":true,"context_lines":[{"line_number":1309,"context_line":"            combined_paths_with_gaps \u003d find_paths_with_gaps("},{"line_number":1310,"context_line":"                combined_shard_ranges, own_shard_range)"},{"line_number":1311,"context_line":"            if not (overlaps or combined_paths_with_gaps):"},{"line_number":1312,"context_line":"                # only merge if shard ranges appear to be *good*"},{"line_number":1313,"context_line":"                self.logger.debug("},{"line_number":1314,"context_line":"                    \u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1315,"context_line":"                    len(other_shard_ranges))"}],"source_content_type":"text/x-python","patch_set":9,"id":"46c1efe1_0507b1a0","line":1312,"in_reply_to":"8ad00ad4_2ff0df53","updated":"2022-09-23 10:26:15.000000000","message":"yep, see my comment line 1296...I\u0027d like to explore generalising to \"always merge from root as long as it\u0027s not crazy broken state\"","commit_id":"a489717cb71029ebc137d9fc7143a393e8adc55e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7ba9359f0c109bee2bc21f78666747472b24b1f2","unresolved":true,"context_lines":[{"line_number":1252,"context_line":"            else:"},{"line_number":1253,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1254,"context_line":""},{"line_number":1255,"context_line":"        if children_shard_ranges and not broker.is_sharded():"},{"line_number":1256,"context_line":"            self.logger.debug(\u0027Updating %d children shard ranges from root\u0027,"},{"line_number":1257,"context_line":"                              len(children_shard_ranges))"},{"line_number":1258,"context_line":"            broker.merge_shard_ranges(children_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":12,"id":"8b7fe800_c62aad43","line":1255,"range":{"start_line":1255,"start_character":37,"end_line":1255,"end_character":60},"updated":"2022-09-29 20:36:34.000000000","message":"\u0027not broker.is_sharded()\u0027 needs unit test coverage","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a53260db7828339b269d23512eee265a7e2df12","unresolved":true,"context_lines":[{"line_number":1252,"context_line":"            else:"},{"line_number":1253,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1254,"context_line":""},{"line_number":1255,"context_line":"        if children_shard_ranges and not broker.is_sharded():"},{"line_number":1256,"context_line":"            self.logger.debug(\u0027Updating %d children shard ranges from root\u0027,"},{"line_number":1257,"context_line":"                              len(children_shard_ranges))"},{"line_number":1258,"context_line":"            broker.merge_shard_ranges(children_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9b47752b_fc8a9388","line":1255,"range":{"start_line":1255,"start_character":37,"end_line":1255,"end_character":60},"in_reply_to":"8b7fe800_c62aad43","updated":"2022-09-29 20:37:21.000000000","message":"also, this should be part of the parent patch","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743f2d16a6287bc99e19a9e3b27301360ee41776","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            else:"},{"line_number":1253,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1254,"context_line":""},{"line_number":1255,"context_line":"        if children_shard_ranges and not broker.is_sharded():"},{"line_number":1256,"context_line":"            self.logger.debug(\u0027Updating %d children shard ranges from root\u0027,"},{"line_number":1257,"context_line":"                              len(children_shard_ranges))"},{"line_number":1258,"context_line":"            broker.merge_shard_ranges(children_shard_ranges)"}],"source_content_type":"text/x-python","patch_set":12,"id":"cb244f93_fd440c89","line":1255,"range":{"start_line":1255,"start_character":37,"end_line":1255,"end_character":60},"in_reply_to":"9b47752b_fc8a9388","updated":"2022-09-30 10:21:30.000000000","message":"Done","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7ba9359f0c109bee2bc21f78666747472b24b1f2","unresolved":true,"context_lines":[{"line_number":1289,"context_line":"            # the merged shard ranges should not result in gaps or overlaps in"},{"line_number":1290,"context_line":"            # the namespace of this shard."},{"line_number":1291,"context_line":"            #"},{"line_number":1292,"context_line":"            # Merging shard ranges from the root is only necessary until this"},{"line_number":1293,"context_line":"            # DB is fully cleaved and reaches SHARDED DB state, after which it"},{"line_number":1294,"context_line":"            # is useful for debugging for the set of sub-shards to which a"},{"line_number":1295,"context_line":"            # shards has sharded to be frozen."},{"line_number":1296,"context_line":"            ancestor_names \u003d ["},{"line_number":1297,"context_line":"                sr.name for sr in own_shard_range.find_ancestors(shard_ranges)]"},{"line_number":1298,"context_line":"            filtered_other_shard_ranges \u003d ["}],"source_content_type":"text/x-python","patch_set":12,"id":"dbdf4170_9520066f","line":1295,"range":{"start_line":1292,"start_character":14,"end_line":1295,"end_character":46},"updated":"2022-09-29 20:36:34.000000000","message":"might be worthy of debate??","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"957cc623cd02b60802756065b1034c0c6123eff6","unresolved":false,"context_lines":[{"line_number":1289,"context_line":"            # the merged shard ranges should not result in gaps or overlaps in"},{"line_number":1290,"context_line":"            # the namespace of this shard."},{"line_number":1291,"context_line":"            #"},{"line_number":1292,"context_line":"            # Merging shard ranges from the root is only necessary until this"},{"line_number":1293,"context_line":"            # DB is fully cleaved and reaches SHARDED DB state, after which it"},{"line_number":1294,"context_line":"            # is useful for debugging for the set of sub-shards to which a"},{"line_number":1295,"context_line":"            # shards has sharded to be frozen."},{"line_number":1296,"context_line":"            ancestor_names \u003d ["},{"line_number":1297,"context_line":"                sr.name for sr in own_shard_range.find_ancestors(shard_ranges)]"},{"line_number":1298,"context_line":"            filtered_other_shard_ranges \u003d ["}],"source_content_type":"text/x-python","patch_set":12,"id":"b53497fa_62b38225","line":1295,"range":{"start_line":1292,"start_character":14,"end_line":1295,"end_character":46},"in_reply_to":"dbdf4170_9520066f","updated":"2022-11-07 16:10:04.000000000","message":"Ack","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"19da9d2235ba34217541050ed3e5b4b4bb84b2ac","unresolved":true,"context_lines":[{"line_number":520,"context_line":"              if existing[\u0027name\u0027] not in to_delete]"},{"line_number":521,"context_line":"    result.extend([ShardRange.from_dict(sr) for sr in to_add])"},{"line_number":522,"context_line":"    return sorted([sr for sr in result if not sr.deleted],"},{"line_number":523,"context_line":"                  key\u003dShardRange.sort_key)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"class CleavingContext(object):"}],"source_content_type":"text/x-python","patch_set":14,"id":"70cc63e4_6ee70de9","line":523,"updated":"2022-10-31 02:47:55.000000000","message":"Because we\u0027re not included deleted shard ranges, could this somehow lead to a gap. I guess there should be something that covers a deleted shards range... so should be ok.. hmm.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2e73a4d2842748e7181aa2638627b5cfeef5b338","unresolved":false,"context_lines":[{"line_number":520,"context_line":"              if existing[\u0027name\u0027] not in to_delete]"},{"line_number":521,"context_line":"    result.extend([ShardRange.from_dict(sr) for sr in to_add])"},{"line_number":522,"context_line":"    return sorted([sr for sr in result if not sr.deleted],"},{"line_number":523,"context_line":"                  key\u003dShardRange.sort_key)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"class CleavingContext(object):"}],"source_content_type":"text/x-python","patch_set":14,"id":"b08d865f_1e533ae9","line":523,"in_reply_to":"6c521f8d_7565a1cc","updated":"2022-11-14 01:40:14.000000000","message":"Ack","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"957cc623cd02b60802756065b1034c0c6123eff6","unresolved":true,"context_lines":[{"line_number":520,"context_line":"              if existing[\u0027name\u0027] not in to_delete]"},{"line_number":521,"context_line":"    result.extend([ShardRange.from_dict(sr) for sr in to_add])"},{"line_number":522,"context_line":"    return sorted([sr for sr in result if not sr.deleted],"},{"line_number":523,"context_line":"                  key\u003dShardRange.sort_key)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"class CleavingContext(object):"}],"source_content_type":"text/x-python","patch_set":14,"id":"6c521f8d_7565a1cc","line":523,"in_reply_to":"70cc63e4_6ee70de9","updated":"2022-11-07 16:10:04.000000000","message":"gaps should not be filled by deleted shards, right? we\u0027d want to see that gap if the only thing covering it is a deleted shard","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"59199b84ce3d0cdcb1fde49339084811b24ca363","unresolved":true,"context_lines":[{"line_number":1302,"context_line":"            # own_shard_range state to shrunk while another replica may still"},{"line_number":1303,"context_line":"            # be in the process of shrinking."},{"line_number":1304,"context_line":"            #"},{"line_number":1305,"context_line":"            # Sharding states: Normally a shard will shard to its own children."},{"line_number":1306,"context_line":"            # However, in some circumstances a shard may need to shard to other"},{"line_number":1307,"context_line":"            # non-children sub-shards. For example, a shard range repair may"},{"line_number":1308,"context_line":"            # cause a child sub-shard to be deleted and its namespace covered"}],"source_content_type":"text/x-python","patch_set":14,"id":"68aef520_4e00d6de","line":1305,"updated":"2022-10-28 05:57:30.000000000","message":"From implementation, it seems this broker will merge any shard ranges from root other than it\u0027s ancestors? for the problem listed in the comment, can we figure out ways to prevent them from happening totally?","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"957cc623cd02b60802756065b1034c0c6123eff6","unresolved":true,"context_lines":[{"line_number":1302,"context_line":"            # own_shard_range state to shrunk while another replica may still"},{"line_number":1303,"context_line":"            # be in the process of shrinking."},{"line_number":1304,"context_line":"            #"},{"line_number":1305,"context_line":"            # Sharding states: Normally a shard will shard to its own children."},{"line_number":1306,"context_line":"            # However, in some circumstances a shard may need to shard to other"},{"line_number":1307,"context_line":"            # non-children sub-shards. For example, a shard range repair may"},{"line_number":1308,"context_line":"            # cause a child sub-shard to be deleted and its namespace covered"}],"source_content_type":"text/x-python","patch_set":14,"id":"cd5f1d0d_fec96e1d","line":1305,"in_reply_to":"68aef520_4e00d6de","updated":"2022-11-07 16:10:04.000000000","message":"Is the \u0027problem\u0027 you\u0027re referring to the \u0027Note\u0027 at line 1320?\n\nI haven\u0027t yet thought of any way to detect that X is a grandparent of Y given only X and Y.\n\nThinking more...IF we forced the parent shard\u0027s epoch to be the same as the timestamp that is embedded in its children\u0027s names, then we might be able to predict the names of child with index N, so we could search to fill a single generation gap:\n\n  given grandparent with epoch T0 and a child:\n    for each index N\u003d0,1,...max-N:\n      calculate parent name based on hash(grandparent)-epoch-N\n      use child.is_child_of(parent_\n\nwhat should max_N be? in practice we tend to generate 2 or 3 shards from a sharding DB, so choosing max_N ~ 10 would give us good success rate - we\u0027d at least eliminate more cases.\n\n...yes, we CAN do better https://review.opendev.org/c/openstack/swift/+/863894 - but maybe that can be a follow-up patch?","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"807ff165b3c7bc7776a69b44299395396525af59","unresolved":false,"context_lines":[{"line_number":1302,"context_line":"            # own_shard_range state to shrunk while another replica may still"},{"line_number":1303,"context_line":"            # be in the process of shrinking."},{"line_number":1304,"context_line":"            #"},{"line_number":1305,"context_line":"            # Sharding states: Normally a shard will shard to its own children."},{"line_number":1306,"context_line":"            # However, in some circumstances a shard may need to shard to other"},{"line_number":1307,"context_line":"            # non-children sub-shards. For example, a shard range repair may"},{"line_number":1308,"context_line":"            # cause a child sub-shard to be deleted and its namespace covered"}],"source_content_type":"text/x-python","patch_set":14,"id":"7873ebee_9d7a51f8","line":1305,"in_reply_to":"b018527c_40159938","updated":"2022-11-18 11:48:09.000000000","message":"Having worked more on https://review.opendev.org/c/openstack/swift/+/863894/ I realised that in order to prove a garndchild relationship, the grandparent shard range must have an epoch value, which implies that the shard range will also have state in (SHARDING, SHARDED), which means that we already exclude it from other shard ranges.\n\nIdeally, we would be able to identify grandparent etc that are stale versions i.e. e.g. state ACTIVE, but https://review.opendev.org/c/openstack/swift/+/863894/ does not help with that.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"058cfaf0bbde4f5c386ad6ad83d3a6533edbe1b3","unresolved":false,"context_lines":[{"line_number":1302,"context_line":"            # own_shard_range state to shrunk while another replica may still"},{"line_number":1303,"context_line":"            # be in the process of shrinking."},{"line_number":1304,"context_line":"            #"},{"line_number":1305,"context_line":"            # Sharding states: Normally a shard will shard to its own children."},{"line_number":1306,"context_line":"            # However, in some circumstances a shard may need to shard to other"},{"line_number":1307,"context_line":"            # non-children sub-shards. For example, a shard range repair may"},{"line_number":1308,"context_line":"            # cause a child sub-shard to be deleted and its namespace covered"}],"source_content_type":"text/x-python","patch_set":14,"id":"b018527c_40159938","line":1305,"in_reply_to":"cd5f1d0d_fec96e1d","updated":"2022-11-16 06:10:48.000000000","message":"Ack","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2e73a4d2842748e7181aa2638627b5cfeef5b338","unresolved":true,"context_lines":[{"line_number":1321,"context_line":"            # and root *if they are present*, but if any ancestor is missing"},{"line_number":1322,"context_line":"            # then there is a chance that older generations in the"},{"line_number":1323,"context_line":"            # other_shard_ranges will not be filtered and could be merged. That"},{"line_number":1324,"context_line":"            # is only a problem if they are somehow still in ACTIVE state, and"},{"line_number":1325,"context_line":"            # no overlap is detected, so the ancestor is merged."},{"line_number":1326,"context_line":"            ancestor_names \u003d ["},{"line_number":1327,"context_line":"                sr.name for sr in own_shard_range.find_ancestors(shard_ranges)]"}],"source_content_type":"text/x-python","patch_set":14,"id":"3b9de736_58bd2cbd","line":1324,"range":{"start_line":1324,"start_character":61,"end_line":1324,"end_character":73},"updated":"2022-11-14 01:40:14.000000000","message":"And as you have rightly pointed out, any older ansestor shouldn\u0027t be in the ACTIVE state, when sharding it should\u0027ve moved to SHARDED. There would\u0027ve had to be a slow replica or split brain for a ancestor to be ACTIVE still, and the split brain would\u0027ve had to last longer then reclaim_age. So pretty safe.\n\nI guess there is old handoff nodes that where turned off.. but again, if a node is off for \u003e reclaim_age what can we expect. The only way I can see this happening is:\n\n1. split brain while a granparent shards, so one replica remains in ACTIVE state or an handoff is in a split brain.\n2. The new shard (parent) shards again and marked deleted as the ACTIVE grandparent node is is taken offline or still in a netsplit.\n3. The grandparent node needs to have been offline (due to netsplit or maintenence) longer then the parent is deleted _and_ reclaimed.\n\nBut the granparent would have to have been \"offline\" for at least 2x reclaim_ages, because we mark a shardrange OSR as deleted. We then wait a reclaim_age in the shard container audit and finally mark the shard db as deleted (broker.delete_db) which when officially marks the database as deleted.. thus starting a db_replicator recliam_age on delete timestamp before it is deleted.\n\nThis seems _really_ unlikely. Esp if we can also use Al\u0027s grandparant detection.\n\nIf this does happen though.. Ie we have a grandparent that is ACTIVE because of a long offline mode for it\u0027s replicas to be gone. But it\u0027s SHARDED child is still around, then this code will make sure it isn\u0027t added (awesome!). But what happens next? When the parent is reclaimed, there is a legitimate gap, and it could get merged. Although I guess we are probably already detecting an overlap way before we get to that point.\nI do wonder if we can use some of the parent/grandparent logic to make audit decisions. Like if we do detect a granparent in ACTIVE state, repair it?","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6a74f44bbc82de8abc63c284580b8db60bdde836","unresolved":true,"context_lines":[{"line_number":1321,"context_line":"            # and root *if they are present*, but if any ancestor is missing"},{"line_number":1322,"context_line":"            # then there is a chance that older generations in the"},{"line_number":1323,"context_line":"            # other_shard_ranges will not be filtered and could be merged. That"},{"line_number":1324,"context_line":"            # is only a problem if they are somehow still in ACTIVE state, and"},{"line_number":1325,"context_line":"            # no overlap is detected, so the ancestor is merged."},{"line_number":1326,"context_line":"            ancestor_names \u003d ["},{"line_number":1327,"context_line":"                sr.name for sr in own_shard_range.find_ancestors(shard_ranges)]"}],"source_content_type":"text/x-python","patch_set":14,"id":"5dacee1b_853b35a8","line":1324,"range":{"start_line":1324,"start_character":61,"end_line":1324,"end_character":73},"in_reply_to":"3b9de736_58bd2cbd","updated":"2022-11-16 16:20:56.000000000","message":"I\u0027m not sure I follow the final paragraph, but here\u0027s what I think you may be uncovering:\n\nIf we assume the root has: \n* stale active grandparent that has re-appeared after the grandparent range was reclaimed from root.\n* sharded deleted parent that has been reclaimed (so actually the root does not have this)\n* active child shards\n\nand now assume a child shard is moved to sharding: when it updates from root it would be unable to detect the grandparent relationship (because the parent is reclaimed) so it might merge in the grandparent if it has none of its own children (the great-granchildren). If the child has its children then the grandparent will be detected as an overlap and not merged.\n\nIf the grandparent is merged, the child will try to shard to it - that is not good 😞 but what happens?\n\nI think that (a) because the grandparent is active, the sharding child will not create the grandparent container. But of course, at least one stale replica of it does exist, and if that is on a current primary then the sharding child will succeed in cleaving to it i.e. replicate_object will succeed. That\u0027s bad.\n\nIf another replica of the child succeeds in cleaving to the great-grandchildren (the happy path) then eventually the root will report an overlap of grandparent with great-grandchildren. At least we\u0027ll get a warning in that case.\n\nBut because we allow unconditional shard range replication between peers, once one child has the grandparent then all children will get it, and because we sort shard ranges by upper I think the children might then choose to cleave to the grandparent in preference to their own children.\n\nIf all of the children replicas actually cleave to the grandparent then do we end up with some unused great-grandchildren shard ranges in found or created state? These are never reported as overlaps because their state is different to the grandparent. That\u0027s bad.\n\nMeanwhile we have a re-incarnated grandparent that one day might need to shard...\n\nThe one good thing is that the grandparent probably overlaps with other parent and grandchildren descendants that are not sharding and will therefore be overlapping in ACTIVE state, so we\u0027d get a warning.\n\nBut the bottom line is that we really really do not want to cleave to an ancestor.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"19da9d2235ba34217541050ed3e5b4b4bb84b2ac","unresolved":true,"context_lines":[{"line_number":1329,"context_line":"                sr for sr in other_shard_ranges"},{"line_number":1330,"context_line":"                if (sr.name not in ancestor_names"},{"line_number":1331,"context_line":"                    and (sr.state not in ShardRange.CLEAVING_STATES"},{"line_number":1332,"context_line":"                         or sr.deleted))"},{"line_number":1333,"context_line":"            ]"},{"line_number":1334,"context_line":"            if own_shard_range.state in ShardRange.SHRINKING_STATES:"},{"line_number":1335,"context_line":"                root_shard_range \u003d own_shard_range.find_root("}],"source_content_type":"text/x-python","patch_set":14,"id":"49639614_1d1bf8b2","line":1332,"updated":"2022-10-31 02:47:55.000000000","message":"Oh we\u0027re filtering out deleted here too, and checking for gaps and overlaps below. Cool.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"957cc623cd02b60802756065b1034c0c6123eff6","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"                sr for sr in other_shard_ranges"},{"line_number":1330,"context_line":"                if (sr.name not in ancestor_names"},{"line_number":1331,"context_line":"                    and (sr.state not in ShardRange.CLEAVING_STATES"},{"line_number":1332,"context_line":"                         or sr.deleted))"},{"line_number":1333,"context_line":"            ]"},{"line_number":1334,"context_line":"            if own_shard_range.state in ShardRange.SHRINKING_STATES:"},{"line_number":1335,"context_line":"                root_shard_range \u003d own_shard_range.find_root("}],"source_content_type":"text/x-python","patch_set":14,"id":"314dd5cc_afdbce83","line":1332,"in_reply_to":"49639614_1d1bf8b2","updated":"2022-11-07 16:10:04.000000000","message":"Ack","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"19da9d2235ba34217541050ed3e5b4b4bb84b2ac","unresolved":true,"context_lines":[{"line_number":1348,"context_line":"                self.logger.debug("},{"line_number":1349,"context_line":"                    \u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1350,"context_line":"                    len(filtered_other_shard_ranges))"},{"line_number":1351,"context_line":"                broker.merge_shard_ranges(filtered_other_shard_ranges)"},{"line_number":1352,"context_line":""},{"line_number":1353,"context_line":"        return own_shard_range, own_shard_range_from_root"},{"line_number":1354,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"88609d78_1f280861","line":1351,"updated":"2022-10-31 02:47:55.000000000","message":"So what happens with repair, when we might legit have an overlap of ranges, because we want to \"shrink\" one path into another? \nI guess, we\u0027re only checking for overlaps based on what the shard has and what was pooled from root, and that isn\u0027t including the shard\u0027s OSR. And if we were merging one overlap into another, the one that is shrinking would be shrinking itself into 1 or more others that aren\u0027t in shrinking and so wouldn\u0027t overlap.\n\nWhat happens if we repair when something has also just decided to cleave/shard. We\u0027d get an overlap then.. but in that case, not merging the overlap, I still think is the best way forward. Let the cleave finish then maybe deal with the repair?\n\nAnother random question, is is possible a proper repair would be to cleave/shrink back into an ancestor?","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"957cc623cd02b60802756065b1034c0c6123eff6","unresolved":true,"context_lines":[{"line_number":1348,"context_line":"                self.logger.debug("},{"line_number":1349,"context_line":"                    \u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1350,"context_line":"                    len(filtered_other_shard_ranges))"},{"line_number":1351,"context_line":"                broker.merge_shard_ranges(filtered_other_shard_ranges)"},{"line_number":1352,"context_line":""},{"line_number":1353,"context_line":"        return own_shard_range, own_shard_range_from_root"},{"line_number":1354,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"a7940c35_fac43df3","line":1351,"in_reply_to":"88609d78_1f280861","updated":"2022-11-07 16:10:04.000000000","message":"Re overlaps:\n1. own_sr is not included in the existing_shard_ranges (excludes self) or filtered_other_shard_ranges (excludes self, excludes children, excludes cleaving states), so the overlap check is ok for sub-ranges that overlap with own_sr.\n\n2. otherwise, the assertion is that we never want to have sub-shards with overlaps in a DB that is not yet sharded.\n\nSo if one replica of P, P1, is sharding to children A, B and split brain means that another replica P2 is sharding to children C, D: the root may get the overlapping A, B, C, D but P1 and P2 won\u0027t merge all four ACTIVE children from root.\n\nIf the overlap is fixed then 2 or A, B, C, D will be moved to SHRINKING and then will be excluded from filtered_other_shard_ranges. So P1 and P2 will then merge the remaining 2 ACTIVE sub-shards.\n\nThere is this anomaly: meanwhile, peer to peer replication will merge unconditionally so P1 and P2 can end up with A, B, C, D all ACTIVE and I think will just cleave to whichever subset is yielded in order as we iterate shard ranges in the cleave method. Which is OK.\n\nMaybe we could merge overlaps from root? But whatever we do in this patch we\u0027re merging more than we did before (apart from when own_sr is shrinking), so NOT merging overlaps is not a regression.\n\n\u003e Another random question, is is possible a proper repair would be to\n\u003e cleave/shrink back into an ancestor?\n\nAncestors (apart from root) are by definition always either sharding or sharded. We never want to shrink to them. That was what caused stalled DBs!\n\nRoot can become ACTIVE again for the purpose of shrinking back to root, but that is not fo repair.","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2e73a4d2842748e7181aa2638627b5cfeef5b338","unresolved":false,"context_lines":[{"line_number":1348,"context_line":"                self.logger.debug("},{"line_number":1349,"context_line":"                    \u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1350,"context_line":"                    len(filtered_other_shard_ranges))"},{"line_number":1351,"context_line":"                broker.merge_shard_ranges(filtered_other_shard_ranges)"},{"line_number":1352,"context_line":""},{"line_number":1353,"context_line":"        return own_shard_range, own_shard_range_from_root"},{"line_number":1354,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"0bd843f7_6e85c379","line":1351,"in_reply_to":"a7940c35_fac43df3","updated":"2022-11-14 01:40:14.000000000","message":"Ack","commit_id":"329fb9ec7a4714b93ac8119b6fce642e790e4e2e"}],"test/unit/container/test_backend.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7371089b6c87057fa5e2006eb83b99cac6a057f4","unresolved":true,"context_lines":[{"line_number":6492,"context_line":"        this \u003d ShardRange(\u0027a/o\u0027, next(ts_iter).internal)"},{"line_number":6493,"context_line":"        that \u003d ShardRange(\u0027a/o\u0027, next(ts_iter).internal)"},{"line_number":6494,"context_line":"        actual \u003d combine_shard_ranges([dict(this)], [dict(that)])"},{"line_number":6495,"context_line":"        self.assertEqual(actual, [dict(that)])"},{"line_number":6496,"context_line":"        actual \u003d combine_shard_ranges([dict(that)], [dict(this)])"},{"line_number":6497,"context_line":"        self.assertEqual(actual, [dict(that)])"},{"line_number":6498,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7066dcdd_e5dc8077","line":6495,"range":{"start_line":6495,"start_character":25,"end_line":6495,"end_character":31},"updated":"2022-08-12 06:59:12.000000000","message":"I think this is returned as a ShardRange not a dict.","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5bc8e857284749a1cf6cab922f3c54ca59e1c4d5","unresolved":false,"context_lines":[{"line_number":6492,"context_line":"        this \u003d ShardRange(\u0027a/o\u0027, next(ts_iter).internal)"},{"line_number":6493,"context_line":"        that \u003d ShardRange(\u0027a/o\u0027, next(ts_iter).internal)"},{"line_number":6494,"context_line":"        actual \u003d combine_shard_ranges([dict(this)], [dict(that)])"},{"line_number":6495,"context_line":"        self.assertEqual(actual, [dict(that)])"},{"line_number":6496,"context_line":"        actual \u003d combine_shard_ranges([dict(that)], [dict(this)])"},{"line_number":6497,"context_line":"        self.assertEqual(actual, [dict(that)])"},{"line_number":6498,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"a516ba7c_ae4f53c2","line":6495,"range":{"start_line":6495,"start_character":25,"end_line":6495,"end_character":31},"in_reply_to":"7066dcdd_e5dc8077","updated":"2022-08-12 17:45:47.000000000","message":"Done","commit_id":"25d6da53e0cae84705a4fea756256bdfd5d7e58d"}],"test/unit/container/test_sharder.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7ba9359f0c109bee2bc21f78666747472b24b1f2","unresolved":true,"context_lines":[{"line_number":6161,"context_line":"        # Verify that sharding shard with no existing shard ranges will merge"},{"line_number":6162,"context_line":"        # other ranges, but not root range."},{"line_number":6163,"context_line":"        root_own_sr \u003d ShardRange(\u0027a/c\u0027, next(self.ts_iter))"},{"line_number":6164,"context_line":"        acceptor \u003d ShardRange("},{"line_number":6165,"context_line":"            str(ShardRange.make_path("},{"line_number":6166,"context_line":"                \u0027.shards_a\u0027, \u0027c\u0027, \u0027c\u0027, next(self.ts_iter), 1)),"},{"line_number":6167,"context_line":"            next(self.ts_iter), \u0027a\u0027, \u0027c\u0027, state\u003dShardRange.ACTIVE)"}],"source_content_type":"text/x-python","patch_set":12,"id":"54ad17aa_9ae6c0d9","line":6164,"range":{"start_line":6164,"start_character":8,"end_line":6164,"end_character":16},"updated":"2022-09-29 20:36:34.000000000","message":"the acceptor var name doesn\u0027t really make sense in context of sharding","commit_id":"ebc821b8e8d2c33ad9e72aa65b34b363c48df709"}]}
