)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97f2e5d17082936a8cf6d9cae243fbe8ddd63fa7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"12149981_b6d77f41","updated":"2022-09-14 13:58:46.000000000","message":"Al had to clean this up for me https://review.opendev.org/c/openstack/swift/+/857608","commit_id":"f53ba5b502bf33bd7bdcfcdd7518461470059280"}],"swift/container/backend.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c3fc171e39f52d5f156f6ee93d2b048685b585fa","unresolved":true,"context_lines":[{"line_number":822,"context_line":"                Timestamp(info[\u0027put_timestamp\u0027]))"},{"line_number":823,"context_line":""},{"line_number":824,"context_line":"    def is_reclaimable(self, now, reclaim_age):"},{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1b0fef51_58df9456","line":825,"updated":"2021-01-19 15:49:03.000000000","message":"I don\u0027t think the can be sharding_initiated, and have no shard ranges, but not be empty, but nevertheless belt-and-braces might justify here, first:\n\n  if not self.empty():\n      return False","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":822,"context_line":"                Timestamp(info[\u0027put_timestamp\u0027]))"},{"line_number":823,"context_line":""},{"line_number":824,"context_line":"    def is_reclaimable(self, now, reclaim_age):"},{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"}],"source_content_type":"text/x-python","patch_set":1,"id":"561a6b7f_27b50ca8","line":825,"in_reply_to":"1b0fef51_58df9456","updated":"2021-01-21 14:17:55.000000000","message":"Ack","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c3fc171e39f52d5f156f6ee93d2b048685b585fa","unresolved":true,"context_lines":[{"line_number":823,"context_line":""},{"line_number":824,"context_line":"    def is_reclaimable(self, now, reclaim_age):"},{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"83b806c1_c9a084ce","line":828,"range":{"start_line":826,"start_character":1,"end_line":828,"end_character":50},"updated":"2021-01-19 15:49:03.000000000","message":"(see comment in sharder.py too) maybe break this into another helper (is_empty_enough_to_reclaim() ???) so that it could be re-used in sharder.py when testing for the logger warning","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"997252b4317f655b8d610874bce70970fcab6044","unresolved":true,"context_lines":[{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"a44d8f8d_0abf0aa9","line":828,"updated":"2021-01-19 08:05:58.000000000","message":"Turns out a side effect of this change is that containers that are marked deleted will still replicate/sync.. which isn\u0027t necessarily a bad thing. it means we can do stuff in one of the deleted roots, say with manage-shard-ranges and changes to shards like I am playing with in a followup patch to explore this some more,  and have the changes replicate to the other primaries.","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":823,"context_line":""},{"line_number":824,"context_line":"    def is_reclaimable(self, now, reclaim_age):"},{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"8bd92b71_13d5c7e2","line":828,"range":{"start_line":826,"start_character":1,"end_line":828,"end_character":50},"in_reply_to":"83b806c1_c9a084ce","updated":"2021-01-21 14:17:55.000000000","message":"Ack","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"37902b72_1510e913","line":828,"in_reply_to":"a44d8f8d_0abf0aa9","updated":"2021-01-21 14:17:55.000000000","message":"Ack","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"054cb948a2b75f584675995858577a1ec76394dc","unresolved":true,"context_lines":[{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    def get_info_is_deleted(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"b84c3369_bdc6dbeb","line":830,"updated":"2021-01-17 22:28:11.000000000","message":"I did originally have something very similar to this in my other patch.. but removed it when I was going to effort of poisioning and removing anyway it seemed not needed 😊\n\nHowever as you say if we just want a bandand to stop the bleeding until we can get auto shrinking enabled _and_ we can just manually remove existing orphaned shards then yeah. This is cool.","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    def get_info_is_deleted(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"93b1b766_4908e524","line":830,"in_reply_to":"b84c3369_bdc6dbeb","updated":"2021-01-21 14:17:55.000000000","message":"Ack","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"77302f66efcb6edc73476c9f9721c2428653478a","unresolved":true,"context_lines":[{"line_number":823,"context_line":""},{"line_number":824,"context_line":"    def is_reclaimable(self, now, reclaim_age):"},{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"2c94520d_5a3d6355","line":826,"updated":"2021-01-20 04:54:51.000000000","message":"if we look at the code behind self.sharding_inititated():\n\n        own_shard_range \u003d self.get_own_shard_range()\n        if own_shard_range.state in (ShardRange.SHARDING,\n                                     ShardRange.SHRINKING,\n                                     ShardRange.SHARDED,\n                                     ShardRange.SHRUNK):\n            return bool(self.get_shard_ranges())\n        return False\n\nWe can see it\u0027s actual result is a self.get_shard_ranges(). This we then use a as a result to this function.\n\nWe could do something like:\n\n if self.is_old_enough_to_reclaim(now, reclaim_age):\n     if self.is_root_container():\n         return not self.get_shard_ranges()\n\nBut a non sharded container is also considered a root container. So I see why you\u0027ve needed to add self.sharding_initiated. Although on a non sharded root that should return is_reclaimable as true (as it doens\u0027t have shard ranges). But as it currently is consistent with .empty so I guess it\u0027s fine.\n\nNot sure how possible this is, but what happens with shards in the FOUND state, these won\u0027t have a shard elsewhere (yet?).. so maybe in the is_reclaimable we ignore these state? Gotta have a play with it I think :)","commit_id":"6e89d392bbfa2a349987e0ac859d888d38f4c9b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":823,"context_line":""},{"line_number":824,"context_line":"    def is_reclaimable(self, now, reclaim_age):"},{"line_number":825,"context_line":"        if self.is_old_enough_to_reclaim(now, reclaim_age):"},{"line_number":826,"context_line":"            if self.is_root_container() and self.sharding_initiated():"},{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b4eba1a6_e311a27d","line":826,"in_reply_to":"2c94520d_5a3d6355","updated":"2021-01-21 14:17:55.000000000","message":"Ack","commit_id":"6e89d392bbfa2a349987e0ac859d888d38f4c9b3"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"39bc9090b5746163e67641de8ed81463e3b206ae","unresolved":true,"context_lines":[{"line_number":821,"context_line":"            Timestamp(info[\u0027delete_timestamp\u0027]) \u003e"},{"line_number":822,"context_line":"                Timestamp(info[\u0027put_timestamp\u0027])):"},{"line_number":823,"context_line":"            return self.empty()"},{"line_number":824,"context_line":"        return False"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"    def get_info_is_deleted(self):"},{"line_number":827,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"9db2e0ff_3c0e29ed","side":"PARENT","line":824,"updated":"2021-01-21 01:02:20.000000000","message":"I wonder if we can simplify this whole method down to:\n\n      def is_reclaimable(self, now, reclaim_age):\n        if self.is_old_enough_to_reclaim(now, reclaim_age):\n            return (self.empty() and \n                    not self.get_shard_ranges() and\n                    self.get_db_state() in (UNSHARDED, SHARDED, COLLAPSED))\n        return False\n\nBecause by deinition _all_ unsharded root container (those that aren\u0027t shards) are considered root containers. Also even unsharded containers can return no shard ranges and unsharded containers will come back in the UNSHARDED State.\n\nThe caveat however is that in this version a shard could be deemed un-reclaimable if it has shards because it\u0027s shrinking or cleaving... but this is ok I think, we want them to finish what they\u0027re doing.","commit_id":"47b4d4dc1c8c306064c718e0bee20b558ff2c063"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":821,"context_line":"            Timestamp(info[\u0027delete_timestamp\u0027]) \u003e"},{"line_number":822,"context_line":"                Timestamp(info[\u0027put_timestamp\u0027])):"},{"line_number":823,"context_line":"            return self.empty()"},{"line_number":824,"context_line":"        return False"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"    def get_info_is_deleted(self):"},{"line_number":827,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"cee76bcb_a2990541","side":"PARENT","line":824,"in_reply_to":"2f3e6d8f_a4148ede","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"47b4d4dc1c8c306064c718e0bee20b558ff2c063"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bc07f08727dbbe9501c971f401bc05673d1f6f40","unresolved":true,"context_lines":[{"line_number":821,"context_line":"            Timestamp(info[\u0027delete_timestamp\u0027]) \u003e"},{"line_number":822,"context_line":"                Timestamp(info[\u0027put_timestamp\u0027])):"},{"line_number":823,"context_line":"            return self.empty()"},{"line_number":824,"context_line":"        return False"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"    def get_info_is_deleted(self):"},{"line_number":827,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"a39b4f9e_790e50d9","side":"PARENT","line":824,"in_reply_to":"9db2e0ff_3c0e29ed","updated":"2021-01-21 05:54:56.000000000","message":"Just had a play using Al\u0027s compact on the probe test, and in watching what happens, a shrinking shard is marked as deleted, but it\u0027s shardrange table (in the shard) still has the acctor in it as active, so we\u0027d need to add self.is_root_container somewhere otherwise shards will probably fail to be cleaned up with the above suggested change.\n\n      def is_reclaimable(self, now, reclaim_age):\n        if self.is_old_enough_to_reclaim(now, reclaim_age):\n            return (self.empty() and \n                    (not self.get_shard_ranges() or not self.is_root_container()) and\n                    self.get_db_state() in (UNSHARDED, SHARDED, COLLAPSED))\n        return False\n\nI think that might work, but maybe my boolean-fu is failing me at 5pm :P Maybe breaking it into ifs would be easier to read.","commit_id":"47b4d4dc1c8c306064c718e0bee20b558ff2c063"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"09bdead41cbeb6fd6173d79d77d81af6b0551ea7","unresolved":true,"context_lines":[{"line_number":821,"context_line":"            Timestamp(info[\u0027delete_timestamp\u0027]) \u003e"},{"line_number":822,"context_line":"                Timestamp(info[\u0027put_timestamp\u0027])):"},{"line_number":823,"context_line":"            return self.empty()"},{"line_number":824,"context_line":"        return False"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"    def get_info_is_deleted(self):"},{"line_number":827,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"2f3e6d8f_a4148ede","side":"PARENT","line":824,"in_reply_to":"a39b4f9e_790e50d9","updated":"2021-01-21 11:02:19.000000000","message":"+1 see my comment in sharder.py, the whole \u0027don\u0027t reclaim until all shards are deleted\u0027 thing must only apply to root containers.","commit_id":"47b4d4dc1c8c306064c718e0bee20b558ff2c063"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"09bdead41cbeb6fd6173d79d77d81af6b0551ea7","unresolved":true,"context_lines":[{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    def get_info_is_deleted(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"665c0b0c_ecddf7a5","line":830,"updated":"2021-01-21 11:02:19.000000000","message":"whatever condition we have here that is additional to is_old_enough_to_reclaim() should be the same as used in sharder.py to gate the warning. So breaking it out to another helper may be useful:\n\n(caveat: I\u0027ve not run this code, and I\u0027m not sure I like the method name)\n\n  def is_empty_enough_to_reclaim():\n      if self.is_root_container():\n        if (self.get_shard_ranges() or\n                self.get_db_state() \u003d\u003d SHARDING):\n            return False\n      return self.empty():\n\nthen is_reclaimable() becomes:\n\n  return (self.is_old_enough_to_reclaim() and\n      self.is_empty_enough_to_reclaim())\n\n\nand over in sharder we can use\n\n  if (is_old_enough_to_reclaim() and not\n      self.is_empty_enough_to_reclaim()):\n     LOG WARNING","commit_id":"5ac1a38c710edb29d5e79512e5e0cb94c16b87c3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    def get_info_is_deleted(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"cdcac6fa_547c5774","line":830,"in_reply_to":"665c0b0c_ecddf7a5","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"5ac1a38c710edb29d5e79512e5e0cb94c16b87c3"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4c37592ab4068d4352ae1f0fdde3f6892e3125bd","unresolved":false,"context_lines":[{"line_number":827,"context_line":"                # reclaim when all shard ranges have been shrunk and deleted"},{"line_number":828,"context_line":"                return not self.get_shard_ranges()"},{"line_number":829,"context_line":"            else:"},{"line_number":830,"context_line":"                return self.empty()"},{"line_number":831,"context_line":"        return False"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    def get_info_is_deleted(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"90dd81a2_9644319a","line":830,"in_reply_to":"cdcac6fa_547c5774","updated":"2021-01-21 22:02:32.000000000","message":"The reason I combined the not get_shard_ranges and the not is root check in my example is because I wanted shards to stop being reclaimed if they too were in the sharding state, as I thought we\u0027d want them to finsish sharding before anything else happens. This just ignores the shards state and will reclaim if they\u0027re empty. Now thinking with a fresh brain, I empty check is enough here, because if sharding then it wont be emtpy :) So nice!","commit_id":"5ac1a38c710edb29d5e79512e5e0cb94c16b87c3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c75c53d218a98fe71af008e99a3354fe6aedcb82","unresolved":true,"context_lines":[{"line_number":821,"context_line":"                Timestamp(info[\u0027delete_timestamp\u0027]) \u003e"},{"line_number":822,"context_line":"                Timestamp(info[\u0027put_timestamp\u0027]))"},{"line_number":823,"context_line":""},{"line_number":824,"context_line":"    def is_empty_enough_to_reclaim(self):"},{"line_number":825,"context_line":"        if self.is_root_container() and (self.get_shard_ranges() or"},{"line_number":826,"context_line":"                                         self.get_db_state() \u003d\u003d SHARDING):"},{"line_number":827,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"977f8d1d_dfc16301","line":824,"updated":"2021-01-22 17:00:48.000000000","message":"might be worth a comment here justifying the self.get_db_state() \u003d\u003d SHARDING condition i.e. \u0027playing it safe, future us may conclude it is not necessary\u0027","commit_id":"7814021016b620a30b0501082f3df37d561a9be4"}],"swift/container/sharder.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"054cb948a2b75f584675995858577a1ec76394dc","unresolved":true,"context_lines":[{"line_number":836,"context_line":"        if broker.is_deleted():"},{"line_number":837,"context_line":"            if broker.is_old_enough_to_reclaim("},{"line_number":838,"context_line":"                    time.time(), self.reclaim_age) \\"},{"line_number":839,"context_line":"                    and broker.get_shard_ranges():"},{"line_number":840,"context_line":"                self.logger.warning("},{"line_number":841,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking: %s (%s)\u0027,"},{"line_number":842,"context_line":"                    broker.db_file, quote(broker.path))"}],"source_content_type":"text/x-python","patch_set":1,"id":"aac93c42_b776d583","line":839,"updated":"2021-01-17 22:28:11.000000000","message":"NOt sure how this could happen so only a very slim edge case. There is a chance say if there is a shink on a shard (or cleave) that it\u0027ll have shardranges, so if somehow it got old and deleted then a shard should start emitting this warning... tho I\u0027m really not sure that\u0027s even possible.\n\nWe could add a `is_root_container()` but that might be over  kill. As we\u0027d see the .sharded_\u003caccount\u003e in the path in the warning which would tell us what\u0027s happening anyway.","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c3fc171e39f52d5f156f6ee93d2b048685b585fa","unresolved":true,"context_lines":[{"line_number":836,"context_line":"        if broker.is_deleted():"},{"line_number":837,"context_line":"            if broker.is_old_enough_to_reclaim("},{"line_number":838,"context_line":"                    time.time(), self.reclaim_age) \\"},{"line_number":839,"context_line":"                    and broker.get_shard_ranges():"},{"line_number":840,"context_line":"                self.logger.warning("},{"line_number":841,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking: %s (%s)\u0027,"},{"line_number":842,"context_line":"                    broker.db_file, quote(broker.path))"}],"source_content_type":"text/x-python","patch_set":1,"id":"e30d8af8_5b684722","line":839,"in_reply_to":"aac93c42_b776d583","updated":"2021-01-19 15:49:03.000000000","message":"I think we would expect this to eventually be true for any shard that has shrunk, so we should have is_root_container() as a condition. (The shard with shard ranges will be reclaimed, whereas a root will not).\n\nCould use:\n  \n  if broker.is_old_enough_to_reclaim(\n                    time.time(), self.reclaim_age) and\n      not broker.is_reclaimable():\n          self.logger.....\n\nalthough there is a risk that the reason for the warning deviates over time from the implementation of is_reclaimable()","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":836,"context_line":"        if broker.is_deleted():"},{"line_number":837,"context_line":"            if broker.is_old_enough_to_reclaim("},{"line_number":838,"context_line":"                    time.time(), self.reclaim_age) \\"},{"line_number":839,"context_line":"                    and broker.get_shard_ranges():"},{"line_number":840,"context_line":"                self.logger.warning("},{"line_number":841,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking: %s (%s)\u0027,"},{"line_number":842,"context_line":"                    broker.db_file, quote(broker.path))"}],"source_content_type":"text/x-python","patch_set":1,"id":"4754250d_25855b98","line":839,"in_reply_to":"e30d8af8_5b684722","updated":"2021-01-21 14:17:55.000000000","message":"Ack","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"054cb948a2b75f584675995858577a1ec76394dc","unresolved":true,"context_lines":[{"line_number":839,"context_line":"                    and broker.get_shard_ranges():"},{"line_number":840,"context_line":"                self.logger.warning("},{"line_number":841,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking: %s (%s)\u0027,"},{"line_number":842,"context_line":"                    broker.db_file, quote(broker.path))"},{"line_number":843,"context_line":"            # if the container has been marked as deleted, all metadata will"},{"line_number":844,"context_line":"            # have been erased so no point auditing. But we want it to pass, in"},{"line_number":845,"context_line":"            # case any objects exist inside it."}],"source_content_type":"text/x-python","patch_set":1,"id":"21851b05_eb99ac52","line":842,"updated":"2021-01-17 22:28:11.000000000","message":"nice! I guess as we get more and more of these messages it might be a great kick in the pants to get auto sharding done.","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":839,"context_line":"                    and broker.get_shard_ranges():"},{"line_number":840,"context_line":"                self.logger.warning("},{"line_number":841,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking: %s (%s)\u0027,"},{"line_number":842,"context_line":"                    broker.db_file, quote(broker.path))"},{"line_number":843,"context_line":"            # if the container has been marked as deleted, all metadata will"},{"line_number":844,"context_line":"            # have been erased so no point auditing. But we want it to pass, in"},{"line_number":845,"context_line":"            # case any objects exist inside it."}],"source_content_type":"text/x-python","patch_set":1,"id":"488567cf_e54f9a75","line":842,"in_reply_to":"21851b05_eb99ac52","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"09bdead41cbeb6fd6173d79d77d81af6b0551ea7","unresolved":true,"context_lines":[{"line_number":836,"context_line":"        if broker.is_deleted():"},{"line_number":837,"context_line":"            if broker.is_old_enough_to_reclaim("},{"line_number":838,"context_line":"                    time.time(), self.reclaim_age) \\"},{"line_number":839,"context_line":"                    and broker.get_shard_ranges():"},{"line_number":840,"context_line":"                self.logger.warning("},{"line_number":841,"context_line":"                    \u0027Reclaimable db stuck waiting for shrinking: %s (%s)\u0027,"},{"line_number":842,"context_line":"                    broker.db_file, quote(broker.path))"}],"source_content_type":"text/x-python","patch_set":3,"id":"669b35af_2c76c823","line":839,"updated":"2021-01-21 11:02:19.000000000","message":"IIRC a shrunk, deleted shard may still have undeleted shard ranges (its acceptor and/or possibly other overlapping shards). Deleted shards should always be reclaimable after reclaim age, and we shouldn\u0027t emit this warning for shards. So I think we need to add\n\n  and broker.is_root_container()\n\nBut furthermore, the condition to emit the warning should be exactly the same condition that prevents reclaim (correct?), so a single helper method to evaluate the condition might make sense. I think on patchset 1 I suggested something like is_empty_enough_to_reclaim() as a companion for is_old_enough_to_reclaim() but there\u0027s probably a better name.","commit_id":"5ac1a38c710edb29d5e79512e5e0cb94c16b87c3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":836,"context_line":"        if broker.is_deleted():"},{"line_number":837,"context_line":"            if broker.is_old_enough_to_reclaim("},{"line_number":838,"context_line":"                    time.time(), self.reclaim_age) \\"},{"line_number":839,"context_line":"                    and broker.get_shard_ranges():"},{"line_number":840,"context_line":"                self.logger.warning("},{"line_number":841,"context_line":"                    \u0027Reclaimable db stuck waiting for shrinking: %s (%s)\u0027,"},{"line_number":842,"context_line":"                    broker.db_file, quote(broker.path))"}],"source_content_type":"text/x-python","patch_set":3,"id":"c8c45b5a_192f7352","line":839,"in_reply_to":"669b35af_2c76c823","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"5ac1a38c710edb29d5e79512e5e0cb94c16b87c3"}],"test/probe/test_sharder.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"054cb948a2b75f584675995858577a1ec76394dc","unresolved":true,"context_lines":[{"line_number":1790,"context_line":"        for conf_file in self.configs[\u0027container-replicator\u0027].values():"},{"line_number":1791,"context_line":"            conf \u003d utils.readconf(conf_file, \u0027container-replicator\u0027)"},{"line_number":1792,"context_line":"            conf[\u0027reclaim_age\u0027] \u003d 0"},{"line_number":1793,"context_line":"            ContainerReplicator(conf).run_once()"},{"line_number":1794,"context_line":""},{"line_number":1795,"context_line":"        logger \u003d debug_logger(\u0027probe\u0027)"},{"line_number":1796,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e3244482_556a39b5","line":1793,"updated":"2021-01-17 22:28:11.000000000","message":"Love this, very clever!","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c3fc171e39f52d5f156f6ee93d2b048685b585fa","unresolved":true,"context_lines":[{"line_number":1799,"context_line":"        sharder_conf_files \u003d []"},{"line_number":1800,"context_line":"        for server in Manager([\u0027container-sharder\u0027]):"},{"line_number":1801,"context_line":"            sharder_conf_files.extend(server.conf_files())"},{"line_number":1802,"context_line":"        # we don\u0027t expect warnings from shard audits"},{"line_number":1803,"context_line":"        for conf_file in sharder_conf_files:"},{"line_number":1804,"context_line":"            conf \u003d utils.readconf(conf_file, \u0027container-sharder\u0027)"},{"line_number":1805,"context_line":"            ContainerSharder(conf, logger\u003dlogger).run_once()"}],"source_content_type":"text/x-python","patch_set":1,"id":"07e42917_5a4ab226","line":1802,"updated":"2021-01-19 15:49:03.000000000","message":"confusing comment - we don\u0027t expect warning from *sharder* root audits","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":1799,"context_line":"        sharder_conf_files \u003d []"},{"line_number":1800,"context_line":"        for server in Manager([\u0027container-sharder\u0027]):"},{"line_number":1801,"context_line":"            sharder_conf_files.extend(server.conf_files())"},{"line_number":1802,"context_line":"        # we don\u0027t expect warnings from shard audits"},{"line_number":1803,"context_line":"        for conf_file in sharder_conf_files:"},{"line_number":1804,"context_line":"            conf \u003d utils.readconf(conf_file, \u0027container-sharder\u0027)"},{"line_number":1805,"context_line":"            ContainerSharder(conf, logger\u003dlogger).run_once()"}],"source_content_type":"text/x-python","patch_set":1,"id":"8a503793_3b32879a","line":1802,"in_reply_to":"07e42917_5a4ab226","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c3fc171e39f52d5f156f6ee93d2b048685b585fa","unresolved":true,"context_lines":[{"line_number":1805,"context_line":"            ContainerSharder(conf, logger\u003dlogger).run_once()"},{"line_number":1806,"context_line":"            self.assertEqual([], logger.get_lines_for_level(\u0027warning\u0027))"},{"line_number":1807,"context_line":""},{"line_number":1808,"context_line":"        # until they want to start reclaiming but we haven\u0027t shrunk yet!"},{"line_number":1809,"context_line":"        found_warning \u003d False"},{"line_number":1810,"context_line":"        for conf_file in sharder_conf_files:"},{"line_number":1811,"context_line":"            conf \u003d utils.readconf(conf_file, \u0027container-sharder\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3b8002a2_445f05d5","line":1808,"updated":"2021-01-19 15:49:03.000000000","message":"what is \u0027they\u0027 here - there\u0027s just the one root container that wants to reclaim - the shards are not yet deleted","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":1805,"context_line":"            ContainerSharder(conf, logger\u003dlogger).run_once()"},{"line_number":1806,"context_line":"            self.assertEqual([], logger.get_lines_for_level(\u0027warning\u0027))"},{"line_number":1807,"context_line":""},{"line_number":1808,"context_line":"        # until they want to start reclaiming but we haven\u0027t shrunk yet!"},{"line_number":1809,"context_line":"        found_warning \u003d False"},{"line_number":1810,"context_line":"        for conf_file in sharder_conf_files:"},{"line_number":1811,"context_line":"            conf \u003d utils.readconf(conf_file, \u0027container-sharder\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"902ca6e1_6dae2e8b","line":1808,"in_reply_to":"3b8002a2_445f05d5","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c3fc171e39f52d5f156f6ee93d2b048685b585fa","unresolved":true,"context_lines":[{"line_number":1820,"context_line":"                found_warning \u003d True"},{"line_number":1821,"context_line":"        self.assertTrue(found_warning)"},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"        # TODO: shrink empty shards and assert the root reclaims"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    def _setup_replication_scenario(self, num_shards, extra_objs\u003d(\u0027alpha\u0027,)):"},{"line_number":1826,"context_line":"        # Get cluster to state where 2 replicas are sharding or sharded but 3rd"}],"source_content_type":"text/x-python","patch_set":1,"id":"52ace290_dc936b2a","line":1823,"updated":"2021-01-19 15:49:03.000000000","message":"..and that the shards would now reclaim too (shrinking shard makes it deleted and therefore should be reclaimable)","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":1820,"context_line":"                found_warning \u003d True"},{"line_number":1821,"context_line":"        self.assertTrue(found_warning)"},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"        # TODO: shrink empty shards and assert the root reclaims"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    def _setup_replication_scenario(self, num_shards, extra_objs\u003d(\u0027alpha\u0027,)):"},{"line_number":1826,"context_line":"        # Get cluster to state where 2 replicas are sharding or sharded but 3rd"}],"source_content_type":"text/x-python","patch_set":1,"id":"76e9022e_5021588c","line":1823,"in_reply_to":"52ace290_dc936b2a","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"5cd2f0b2402709bbba9bdaf43947be42d72a971d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"77302f66efcb6edc73476c9f9721c2428653478a","unresolved":true,"context_lines":[{"line_number":1815,"context_line":"            warnings \u003d logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":1816,"context_line":"            if warnings:"},{"line_number":1817,"context_line":"                self.assertTrue(warnings[0].startswith("},{"line_number":1818,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking\u0027))"},{"line_number":1819,"context_line":"                self.assertEqual(1, len(warnings))"},{"line_number":1820,"context_line":"                found_warning \u003d True"},{"line_number":1821,"context_line":"        self.assertTrue(found_warning)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e6523451_96d34361","line":1818,"range":{"start_line":1818,"start_character":33,"end_line":1818,"end_character":37},"updated":"2021-01-20 04:54:51.000000000","message":"This should be \u0027db\u0027:\n\n  \u0027Reclaimable db stuck waiting for shrinking\u0027","commit_id":"6e89d392bbfa2a349987e0ac859d888d38f4c9b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":false,"context_lines":[{"line_number":1815,"context_line":"            warnings \u003d logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":1816,"context_line":"            if warnings:"},{"line_number":1817,"context_line":"                self.assertTrue(warnings[0].startswith("},{"line_number":1818,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking\u0027))"},{"line_number":1819,"context_line":"                self.assertEqual(1, len(warnings))"},{"line_number":1820,"context_line":"                found_warning \u003d True"},{"line_number":1821,"context_line":"        self.assertTrue(found_warning)"}],"source_content_type":"text/x-python","patch_set":2,"id":"86191b0b_38f6e9c5","line":1818,"range":{"start_line":1818,"start_character":33,"end_line":1818,"end_character":37},"in_reply_to":"e6523451_96d34361","updated":"2021-01-21 14:17:55.000000000","message":"Done","commit_id":"6e89d392bbfa2a349987e0ac859d888d38f4c9b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e6a82e85397103564ffa1b6093dc332499f93ed4","unresolved":true,"context_lines":[{"line_number":1815,"context_line":"            warnings \u003d logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":1816,"context_line":"            if warnings:"},{"line_number":1817,"context_line":"                self.assertTrue(warnings[0].startswith("},{"line_number":1818,"context_line":"                    \u0027Reclaimable root stuck waiting for shrinking\u0027))"},{"line_number":1819,"context_line":"                self.assertEqual(1, len(warnings))"},{"line_number":1820,"context_line":"                found_warning \u003d True"},{"line_number":1821,"context_line":"        self.assertTrue(found_warning)"}],"source_content_type":"text/x-python","patch_set":3,"id":"49eb04ae_caa7fdff","line":1818,"updated":"2021-01-21 14:17:55.000000000","message":"s/root/db/g","commit_id":"5ac1a38c710edb29d5e79512e5e0cb94c16b87c3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c75c53d218a98fe71af008e99a3354fe6aedcb82","unresolved":true,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"# repair a side effect of importing unittests\u0027 debug_logger"},{"line_number":46,"context_line":"utils.HASH_PATH_SUFFIX \u003d utils.HASH_PATH_PREFIX \u003d b\u0027\u0027"},{"line_number":47,"context_line":"utils.validate_hash_conf()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"MIN_SHARD_CONTAINER_THRESHOLD \u003d 4"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f069f27_d2c772d3","line":47,"updated":"2021-01-22 17:00:48.000000000","message":"this may help https://review.opendev.org/c/openstack/swift/+/771969","commit_id":"7814021016b620a30b0501082f3df37d561a9be4"}],"test/unit/container/test_sharder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97f2e5d17082936a8cf6d9cae243fbe8ddd63fa7","unresolved":true,"context_lines":[{"line_number":4722,"context_line":"                self._mock_sharder() as sharder:"},{"line_number":4723,"context_line":"            fake_time.return_value \u003d 6048000 + float(delete_ts)"},{"line_number":4724,"context_line":"            sharder._audit_container(broker)"},{"line_number":4725,"context_line":"        self.assertEqual([], self.logger.get_lines_for_level(\u0027warning\u0027))"},{"line_number":4726,"context_line":""},{"line_number":4727,"context_line":"    def test_audit_old_style_shard_container(self):"},{"line_number":4728,"context_line":"        self._do_test_audit_shard_container(\u0027Root\u0027, \u0027a/c\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"677c7502_9b212818","line":4725,"updated":"2022-09-14 13:58:46.000000000","message":"This method would look better up above all these _do_test helpers that are used in the tests below.\n\nI sort of imagine I copied test_deleted_root_container, then searched down for the next \"def test\" to paste it just above.  Next time I\u0027ll duplicate the test above itself to preserve the original and then hack in place.","commit_id":"f53ba5b502bf33bd7bdcfcdd7518461470059280"}]}
