)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"82d3bbd7e83cd0d8a61993b6e64905bcdfb2a096","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This has a few different benefits:"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"* When re-running the relinker, we won\u0027t try to rehash partitions"},{"line_number":12,"context_line":"  outside the expanded range."},{"line_number":13,"context_line":"* When running on a small, sparse cluster (like in dev or testing) we"},{"line_number":14,"context_line":"  won\u0027t try to rehash an empty new partition."},{"line_number":15,"context_line":"* If we find old files from an earlier part power increase and link them"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"220565d4_6cbe478f","line":12,"updated":"2021-03-29 19:06:18.000000000","message":"Also, when re-running the relinker, we\u0027ll only rehash in the expanded range *if we found ourselves needing to make new links*.","commit_id":"ff8caa0c8e5eb3c70c0ee0f0bbcf59fc8bcdeecf"}],"swift/cli/relinker.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6c9d6177dfe08a21bb6a5c650e7c4c44c981aa67","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        if not self.do_cleanup:"},{"line_number":207,"context_line":"            for partition in self.dirty_partitions:"},{"line_number":208,"context_line":"                if partition \u003e\u003d 2 ** self.states[\u0027part_power\u0027]:"},{"line_number":209,"context_line":"                    self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                        device, partition, [], self.policy)"},{"line_number":211,"context_line":"            self.dirty_partitions \u003d set()"}],"source_content_type":"text/x-python","patch_set":2,"id":"31201631_1c77dd7c","line":208,"updated":"2021-05-06 00:27:22.000000000","message":"Arem\u0027t you missing the -1 to get the last 1/2 of the parts:\n\n  if partition \u003e\u003d 2 ** (self.states[\u0027part_power\u0027] -1):\n\nAt the moment aren\u0027t we only checking the last partition?","commit_id":"baa10f11d0ef7195b91427e702d73605951aaebc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8126521f4008906c93f0be8e17600f3b1a9fd416","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        if not self.do_cleanup:"},{"line_number":207,"context_line":"            for partition in self.dirty_partitions:"},{"line_number":208,"context_line":"                if partition \u003e\u003d 2 ** self.states[\u0027part_power\u0027]:"},{"line_number":209,"context_line":"                    self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                        device, partition, [], self.policy)"},{"line_number":211,"context_line":"            self.dirty_partitions \u003d set()"}],"source_content_type":"text/x-python","patch_set":2,"id":"b1e7d0a7_28cf1aaa","line":208,"in_reply_to":"31201631_1c77dd7c","updated":"2021-05-06 18:41:42.000000000","message":"Nope, this is the way we want it. The old code needed the check so we could verify that when we (used to!) up-shift the partition with\n\n for new_part in (2 * partition, 2 * partition + 1):\n\nit\u0027d be in the range that wouldn\u0027t be touched by cleanup. Now, all the dirty partitions should already be up-shifted during relink. The old way ran into trouble if you re-ran the relinker: imagine you had\n\n* part power 7, so 128 possible partitions\n* parts to process: 123, 45\n\nso we run the relinker once and now have parts\n\n* 123\n* 246 (rehashed)\n* 247 (rehashed)\n* 45\n* 90 (invalidated but not rehashed)\n* 91 (invalidated but not rehashed)\n\nIf we run the relinker *again*, on master we\u0027d rehash 246, 247, 492, 493, 494, and 495, even though those last four don\u0027t even make sense with our new part power of 8. On the plus side, with https://review.opendev.org/c/openstack/swift/+/781908 we shouldn\u0027t go creating mostly-empty partition dirs anymore, but it\u0027d be even better if we never even *tried* to rehash. With *this*, nothing should be rehashed, assuming the initial relink was successful and so no new links were created.\n\nThe situation\u0027s even hairier if some of those part-123 diskfiles never got relinked and now we\u0027re doing the 8 -\u003e 9 part power increase. Replication would have moved them to some other node that doesn\u0027t have any 246 or 247 parts, then the relinker will link into 492, 493, 494, and/or 495. With this patch, those will be rehashed, while master will still be rehashing 246 and 247.","commit_id":"baa10f11d0ef7195b91427e702d73605951aaebc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8126521f4008906c93f0be8e17600f3b1a9fd416","unresolved":true,"context_lines":[{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        if rehash:"},{"line_number":418,"context_line":"            self.dirty_partitions.add(get_partition_from_path("},{"line_number":419,"context_line":"                self.conf[\u0027devices\u0027], hash_path))"},{"line_number":420,"context_line":"            try:"},{"line_number":421,"context_line":"                diskfile.invalidate_hash(os.path.dirname(hash_path))"},{"line_number":422,"context_line":"            except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b581a541_ac4fb020","line":419,"updated":"2021-05-06 18:41:42.000000000","message":"This is going to grow and never reset during cleanup currently. We either ought to drop it or reset self.dirty_partitions on cleanup, too.","commit_id":"baa10f11d0ef7195b91427e702d73605951aaebc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"312bb907de473dbae4ecd5e736915aab211584be","unresolved":true,"context_lines":[{"line_number":203,"context_line":"        # shift to the new partition space and rehash"},{"line_number":204,"context_line":"        #   |0                             2N|"},{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        for partition in self.dirty_partitions:"},{"line_number":207,"context_line":"            if self.do_cleanup or partition \u003e\u003d 2 ** ("},{"line_number":208,"context_line":"                    self.states[\u0027next_part_power\u0027] - 1):"},{"line_number":209,"context_line":"                self.diskfile_mgr.get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"ccf6834d_f2182272","line":206,"range":{"start_line":206,"start_character":12,"end_line":206,"end_character":21},"updated":"2021-05-07 17:19:09.000000000","message":"nit: I\u0027d find it a little easier to read if the partition that the hook is handling was named partition and the dirty partition was named dirty_partition or something else","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"312bb907de473dbae4ecd5e736915aab211584be","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        #   |0                             2N|"},{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        for partition in self.dirty_partitions:"},{"line_number":207,"context_line":"            if self.do_cleanup or partition \u003e\u003d 2 ** ("},{"line_number":208,"context_line":"                    self.states[\u0027next_part_power\u0027] - 1):"},{"line_number":209,"context_line":"                self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                    device, partition, [], self.policy)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf120ff1_f9e056ab","line":207,"range":{"start_line":207,"start_character":15,"end_line":207,"end_character":30},"updated":"2021-05-07 17:19:09.000000000","message":"there doesn\u0027t appear to be any test coverage for this part of the change regressing:\n\n  diff --git a/swift/cli/relinker.py b/swift/cli/relinker.py\n  index b59e3c154..8510160bd 100644\n  --- a/swift/cli/relinker.py\n  +++ b/swift/cli/relinker.py\n  @@ -204,7 +204,7 @@ class Relinker(object):\n           #   |0                             2N|\n           #   |                IIJJKKLLMMNNOOPP|\n           for partition in self.dirty_partitions:\n  -            if self.do_cleanup or partition \u003e\u003d 2 ** (\n  +            if partition \u003e\u003d 2 ** (\n                       self.states[\u0027next_part_power\u0027] - 1):\n                   self.diskfile_mgr.get_hashes(\n                       device, partition, [], self.policy)","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2e22ac180b55935f9aef3b3903f053f38779ad16","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        for partition in self.dirty_partitions:"},{"line_number":207,"context_line":"            if self.do_cleanup or partition \u003e\u003d 2 ** ("},{"line_number":208,"context_line":"                    self.states[\u0027next_part_power\u0027] - 1):"},{"line_number":209,"context_line":"                self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                    device, partition, [], self.policy)"},{"line_number":211,"context_line":"        self.dirty_partitions \u003d set()"}],"source_content_type":"text/x-python","patch_set":3,"id":"2a3f67a8_0471647d","line":208,"updated":"2021-05-07 07:04:13.000000000","message":"Nice, looks right. If the cleanup has to relink a missing link then make sure we rehash them if they happen to be in the upper region (even though cleanup doesn\u0027t look that far).","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"312bb907de473dbae4ecd5e736915aab211584be","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        for partition in self.dirty_partitions:"},{"line_number":207,"context_line":"            if self.do_cleanup or partition \u003e\u003d 2 ** ("},{"line_number":208,"context_line":"                    self.states[\u0027next_part_power\u0027] - 1):"},{"line_number":209,"context_line":"                self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                    device, partition, [], self.policy)"},{"line_number":211,"context_line":"        self.dirty_partitions \u003d set()"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f05fd5d_c0cc5793","line":208,"range":{"start_line":208,"start_character":32,"end_line":208,"end_character":49},"updated":"2021-05-07 17:19:09.000000000","message":"if we were ever to increase by more than 1 then I think this would need to be \n\n  self.states[\u0027part_power\u0027]\n\nworth changing now, just in case?","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a9270a53a7dae12a0487e544640440aa9f08d023","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        for partition in self.dirty_partitions:"},{"line_number":207,"context_line":"            if self.do_cleanup or partition \u003e\u003d 2 ** ("},{"line_number":208,"context_line":"                    self.states[\u0027next_part_power\u0027] - 1):"},{"line_number":209,"context_line":"                self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                    device, partition, [], self.policy)"},{"line_number":211,"context_line":"        self.dirty_partitions \u003d set()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5bcbef9a_e53b9366","line":208,"in_reply_to":"2a3f67a8_0471647d","updated":"2021-05-07 15:58:39.000000000","message":"Actually, cleanup does the rehash *regardless*, since we process partitions in descending order. The expectation is that cleanup should only rarely be doing relinking, though, so it shouldn\u0027t matter much.","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0770e66c010434d67cea3fc820419ae113d4bcdd","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        #   |                IIJJKKLLMMNNOOPP|"},{"line_number":206,"context_line":"        for partition in self.dirty_partitions:"},{"line_number":207,"context_line":"            if self.do_cleanup or partition \u003e\u003d 2 ** ("},{"line_number":208,"context_line":"                    self.states[\u0027next_part_power\u0027] - 1):"},{"line_number":209,"context_line":"                self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                    device, partition, [], self.policy)"},{"line_number":211,"context_line":"        self.dirty_partitions \u003d set()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5d54f908_373feac0","line":208,"range":{"start_line":208,"start_character":32,"end_line":208,"end_character":49},"in_reply_to":"9f05fd5d_c0cc5793","updated":"2021-05-08 00:02:45.000000000","message":"I had it that way before... 🤔 I *think* I was thinking about what would happen when part_power \u003d\u003d next_part_power and whether I\u0027d still be filtering properly -- but then I realized I needed to always do the rehash when cleaning up, anyway.\n\n#willfix","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"312bb907de473dbae4ecd5e736915aab211584be","unresolved":true,"context_lines":[{"line_number":208,"context_line":"                    self.states[\u0027next_part_power\u0027] - 1):"},{"line_number":209,"context_line":"                self.diskfile_mgr.get_hashes("},{"line_number":210,"context_line":"                    device, partition, [], self.policy)"},{"line_number":211,"context_line":"        self.dirty_partitions \u003d set()"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        if self.do_cleanup:"},{"line_number":214,"context_line":"            hashes \u003d self.diskfile_mgr.get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"b8808237_160eb5e9","line":211,"range":{"start_line":211,"start_character":8,"end_line":211,"end_character":37},"updated":"2021-05-07 17:19:09.000000000","message":"there\u0027s no way that post_partition_hook can be skipped (by exception handling) and we still proceed to another partition, correct?","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2e22ac180b55935f9aef3b3903f053f38779ad16","unresolved":true,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        if self.do_cleanup:"},{"line_number":214,"context_line":"            hashes \u003d self.diskfile_mgr.get_hashes("},{"line_number":215,"context_line":"                device, int(part), [], self.policy)"},{"line_number":216,"context_line":"            # In any reasonably-large cluster, we\u0027d expect all old"},{"line_number":217,"context_line":"            # partitions P to be empty after cleanup (i.e., it\u0027s unlikely"},{"line_number":218,"context_line":"            # that there\u0027s another partition Q :\u003d P//2 that also has data"}],"source_content_type":"text/x-python","patch_set":3,"id":"4285d21d_5fb3919f","line":215,"updated":"2021-05-07 07:04:13.000000000","message":"And if we happen to hit a part that has already been rehashed that should be basically no noop right because the cleanup could rehash a newly linked file in the prev_part_power range.\n\nI guess it might have to go check hashes, but shouldn\u0027t have to do any hashing.","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a9270a53a7dae12a0487e544640440aa9f08d023","unresolved":true,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        if self.do_cleanup:"},{"line_number":214,"context_line":"            hashes \u003d self.diskfile_mgr.get_hashes("},{"line_number":215,"context_line":"                device, int(part), [], self.policy)"},{"line_number":216,"context_line":"            # In any reasonably-large cluster, we\u0027d expect all old"},{"line_number":217,"context_line":"            # partitions P to be empty after cleanup (i.e., it\u0027s unlikely"},{"line_number":218,"context_line":"            # that there\u0027s another partition Q :\u003d P//2 that also has data"}],"source_content_type":"text/x-python","patch_set":3,"id":"77ebb472_b45075f4","line":215,"in_reply_to":"4285d21d_5fb3919f","updated":"2021-05-07 15:58:39.000000000","message":"This should always be doing useful rehashing, unless you were nervous about cleanup and wanted to do a second pass. But yeah, if it\u0027s been recently rehashed, the only IO that should be needed will be to load up hashes.pkl and check that hashed.invalid is empty (short of new writes coming in).","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a9270a53a7dae12a0487e544640440aa9f08d023","unresolved":true,"context_lines":[{"line_number":417,"context_line":""},{"line_number":418,"context_line":"        if rehash:"},{"line_number":419,"context_line":"            # Even though we\u0027re invalidating the suffix, don\u0027t update"},{"line_number":420,"context_line":"            # self.dirty_hashes -- we only care about them for relinking"},{"line_number":421,"context_line":"            # into the new part-power space"},{"line_number":422,"context_line":"            try:"},{"line_number":423,"context_line":"                diskfile.invalidate_hash(os.path.dirname(hash_path))"}],"source_content_type":"text/x-python","patch_set":3,"id":"ee145d99_cc29d4c8","line":420,"range":{"start_line":420,"start_character":19,"end_line":420,"end_character":31},"updated":"2021-05-07 15:58:39.000000000","message":"\"dirty_partitions\" -- though maybe I should rename that to something like \"linked_into_partitions\"....","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"312bb907de473dbae4ecd5e736915aab211584be","unresolved":true,"context_lines":[{"line_number":417,"context_line":""},{"line_number":418,"context_line":"        if rehash:"},{"line_number":419,"context_line":"            # Even though we\u0027re invalidating the suffix, don\u0027t update"},{"line_number":420,"context_line":"            # self.dirty_hashes -- we only care about them for relinking"},{"line_number":421,"context_line":"            # into the new part-power space"},{"line_number":422,"context_line":"            try:"},{"line_number":423,"context_line":"                diskfile.invalidate_hash(os.path.dirname(hash_path))"}],"source_content_type":"text/x-python","patch_set":3,"id":"1feb9e24_11a18d84","line":420,"range":{"start_line":420,"start_character":19,"end_line":420,"end_character":31},"in_reply_to":"ee145d99_cc29d4c8","updated":"2021-05-07 17:19:09.000000000","message":"+1 linked_into_partitions","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"8d06086c310573f46f5ed59ce8762c007c4cdbfd","unresolved":true,"context_lines":[{"line_number":118,"context_line":"        self.devices_data \u003d recursive_defaultdict()"},{"line_number":119,"context_line":"        self.policy_count \u003d 0"},{"line_number":120,"context_line":"        self.pid \u003d os.getpid()"},{"line_number":121,"context_line":"        self.linked_into_partitions \u003d set()"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def _aggregate_dev_policy_stats(self):"},{"line_number":124,"context_line":"        for dev_data in self.devices_data.values():"}],"source_content_type":"text/x-python","patch_set":5,"id":"78571c29_24680696","line":121,"updated":"2021-05-12 06:35:24.000000000","message":"NIT: becuase this re-initialised during the pre-partition hook this might be redundant. But I guess it\u0027s ok to initialise it in the contructor anyway.","commit_id":"1078223eb960ee7fa226fdae43050bc8758e1587"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"541c588540941271cf5f78d8f3b488550629c41a","unresolved":true,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def hook_pre_partition(self, partition_path):"},{"line_number":286,"context_line":"        self.pre_partition_errors \u003d self.total_errors"},{"line_number":287,"context_line":"        self.linked_into_partitions \u003d set()"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def hook_post_partition(self, partition_path):"},{"line_number":290,"context_line":"        datadir_path, partition \u003d os.path.split("}],"source_content_type":"text/x-python","patch_set":5,"id":"f97da30c_dd60fc5b","line":287,"updated":"2021-05-12 09:02:55.000000000","message":"+1","commit_id":"1078223eb960ee7fa226fdae43050bc8758e1587"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"541c588540941271cf5f78d8f3b488550629c41a","unresolved":true,"context_lines":[{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        if rehash:"},{"line_number":537,"context_line":"            # Even though we\u0027re invalidating the suffix, don\u0027t update"},{"line_number":538,"context_line":"            # self.dirty_hashes -- we only care about them for relinking"},{"line_number":539,"context_line":"            # into the new part-power space"},{"line_number":540,"context_line":"            try:"},{"line_number":541,"context_line":"                diskfile.invalidate_hash(os.path.dirname(hash_path))"}],"source_content_type":"text/x-python","patch_set":5,"id":"7b4f53c9_b7583ca6","line":538,"range":{"start_line":538,"start_character":14,"end_line":538,"end_character":32},"updated":"2021-05-12 09:02:55.000000000","message":"nit: linked_into_partitions :)","commit_id":"1078223eb960ee7fa226fdae43050bc8758e1587"}],"swift/common/utils.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2e22ac180b55935f9aef3b3903f053f38779ad16","unresolved":true,"context_lines":[{"line_number":5926,"context_line":"    else:"},{"line_number":5927,"context_line":"        raise ValueError(\u0027Path %r is not under device dir %r\u0027 % ("},{"line_number":5928,"context_line":"            path, devices))"},{"line_number":5929,"context_line":"    return int(path_components[offset + 2])"},{"line_number":5930,"context_line":""},{"line_number":5931,"context_line":""},{"line_number":5932,"context_line":"def replace_partition_in_path(devices, path, part_power):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3d565848_6f6f9083","line":5929,"updated":"2021-05-07 07:04:13.000000000","message":"We tend to grab the path and walk backwards. But I like that this also double checks that the path exists on the devices path. So kudos.","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"312bb907de473dbae4ecd5e736915aab211584be","unresolved":true,"context_lines":[{"line_number":5926,"context_line":"    else:"},{"line_number":5927,"context_line":"        raise ValueError(\u0027Path %r is not under device dir %r\u0027 % ("},{"line_number":5928,"context_line":"            path, devices))"},{"line_number":5929,"context_line":"    return int(path_components[offset + 2])"},{"line_number":5930,"context_line":""},{"line_number":5931,"context_line":""},{"line_number":5932,"context_line":"def replace_partition_in_path(devices, path, part_power):"}],"source_content_type":"text/x-python","patch_set":3,"id":"09c2575f_529c50ad","line":5929,"in_reply_to":"0c99043b_0dee93d8","updated":"2021-05-07 17:19:09.000000000","message":"nice. we could share some code if we had a\n\n  get_path_components(devices, path)","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a9270a53a7dae12a0487e544640440aa9f08d023","unresolved":true,"context_lines":[{"line_number":5926,"context_line":"    else:"},{"line_number":5927,"context_line":"        raise ValueError(\u0027Path %r is not under device dir %r\u0027 % ("},{"line_number":5928,"context_line":"            path, devices))"},{"line_number":5929,"context_line":"    return int(path_components[offset + 2])"},{"line_number":5930,"context_line":""},{"line_number":5931,"context_line":""},{"line_number":5932,"context_line":"def replace_partition_in_path(devices, path, part_power):"}],"source_content_type":"text/x-python","patch_set":3,"id":"0c99043b_0dee93d8","line":5929,"in_reply_to":"3d565848_6f6f9083","updated":"2021-05-07 15:58:39.000000000","message":"Yeah, I was mostly piggy-backing off replace_partition_in_path, below. The nice thing is that it can work with either data files *or* hashdirs.","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"}],"test/unit/cli/test_relinker.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"312bb907de473dbae4ecd5e736915aab211584be","unresolved":true,"context_lines":[{"line_number":3138,"context_line":"            expected \u003d [(\u0027invalidate\u0027, self.next_suffix_dir)]"},{"line_number":3139,"context_line":"            if self.part \u003e\u003d 2 ** (PART_POWER - 1):"},{"line_number":3140,"context_line":"                expected.append((\u0027get_hashes\u0027, self.existing_device,"},{"line_number":3141,"context_line":"                                 self.next_part, [], POLICIES[0]))"},{"line_number":3142,"context_line":""},{"line_number":3143,"context_line":"            self.assertEqual(calls, expected)"},{"line_number":3144,"context_line":"            # Depending on partition, there may or may not be a get_hashes here"}],"source_content_type":"text/x-python","patch_set":3,"id":"ed575e3e_52dc6ef1","line":3141,"updated":"2021-05-07 17:19:09.000000000","message":"ok, so this is the validation of the significant change i.e. only one object was relinked so only one partition is rehashed","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0770e66c010434d67cea3fc820419ae113d4bcdd","unresolved":true,"context_lines":[{"line_number":3138,"context_line":"            expected \u003d [(\u0027invalidate\u0027, self.next_suffix_dir)]"},{"line_number":3139,"context_line":"            if self.part \u003e\u003d 2 ** (PART_POWER - 1):"},{"line_number":3140,"context_line":"                expected.append((\u0027get_hashes\u0027, self.existing_device,"},{"line_number":3141,"context_line":"                                 self.next_part, [], POLICIES[0]))"},{"line_number":3142,"context_line":""},{"line_number":3143,"context_line":"            self.assertEqual(calls, expected)"},{"line_number":3144,"context_line":"            # Depending on partition, there may or may not be a get_hashes here"}],"source_content_type":"text/x-python","patch_set":3,"id":"3b0f4da9_b34e72c9","line":3141,"in_reply_to":"ed575e3e_52dc6ef1","updated":"2021-05-08 00:02:45.000000000","message":"Yup -- I should probably have some more tests to cover some of the other cases in the commit message...","commit_id":"f6a74c03b3cc34ef1319b6ce47a57e521744c7a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"541c588540941271cf5f78d8f3b488550629c41a","unresolved":true,"context_lines":[{"line_number":3212,"context_line":"        self.assertEqual([], self.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":3213,"context_line":""},{"line_number":3214,"context_line":"    def test_cleanup_not_yet_relinked(self):"},{"line_number":3215,"context_line":"        # force rehash of new partition to not happen during cleanup"},{"line_number":3216,"context_line":"        self._setup_object(lambda part: part \u003e\u003d 2 ** (PART_POWER - 1))"},{"line_number":3217,"context_line":"        self._common_test_cleanup(relink\u003dFalse)"},{"line_number":3218,"context_line":"        with self._mock_relinker():"}],"source_content_type":"text/x-python","patch_set":5,"id":"defb17da_06d6fbd3","line":3215,"range":{"start_line":3215,"start_character":8,"end_line":3215,"end_character":68},"updated":"2021-05-12 09:02:55.000000000","message":"this comment is a little stale - the test now validates that rehash *is* done, but it\u0027s a consequence of tracking dirty partitions rather than the blanket rehash during cleanup","commit_id":"1078223eb960ee7fa226fdae43050bc8758e1587"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"541c588540941271cf5f78d8f3b488550629c41a","unresolved":true,"context_lines":[{"line_number":3242,"context_line":"        self.assertEqual([], self.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"    def test_cleanup_not_yet_relinked_low(self):"},{"line_number":3245,"context_line":"        # force rehash of new partition to not happen during cleanup"},{"line_number":3246,"context_line":"        self._setup_object(lambda part: part \u003c 2 ** (PART_POWER - 1))"},{"line_number":3247,"context_line":"        self._common_test_cleanup(relink\u003dFalse)"},{"line_number":3248,"context_line":"        self.assertFalse(os.path.isfile(self.expected_file))"}],"source_content_type":"text/x-python","patch_set":5,"id":"624a8df2_6edf44c9","line":3245,"range":{"start_line":3245,"start_character":8,"end_line":3245,"end_character":68},"updated":"2021-05-12 09:02:55.000000000","message":"as above, the comment needs updating","commit_id":"1078223eb960ee7fa226fdae43050bc8758e1587"}]}