)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"371075c7d4b2cabdf87d3fd6f562a023d7cb7a13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"df6e8a5c_f10fb3c1","updated":"2026-08-13 05:40:34.000000000","message":"Just pep8 fixes.","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9439799f13f0e184d940449da183a7326ad91d22","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f8104e63_bc742fd9","updated":"2026-08-13 07:04:11.000000000","message":"Please see my inline comments.\n\nIdeas how to fix this and tests in 1000802: sq? reconstructor: invalidate hashes after an rsync revert | https://review.opendev.org/c/openstack/swift/+/1000802","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"17111fac8a7e4aedec1c65f7c1715287a7908858","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"548e87b5_5ad4e833","updated":"2026-08-13 14:59:45.000000000","message":"Seems like a solid plan. I feel like there\u0027s a good opportunity to have the replicator and reconstructor both use some common rsync-invoking code, which would address a lot of Christian\u0027s concerns about the current differences.\n\nUsing `job[\u0027primary_frag_index\u0027] is None` to determine a \"pure\" handoff was sneaky/subtle -- I thought for sure we\u0027d want/need the ring history table to give us a stronger signal that this used to be a primary.","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"}],"etc/object-server.conf-sample":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9439799f13f0e184d940449da183a7326ad91d22","unresolved":true,"context_lines":[{"line_number":455,"context_line":"# subsequently handled by normal reconstruction there. Reverts for partitions"},{"line_number":456,"context_line":"# with a local primary fragment use SSYNC instead. max_objects_per_revert does"},{"line_number":457,"context_line":"# not limit rsync reverts; it applies only to SSYNC reverts."},{"line_number":458,"context_line":"# prefer_rsync_reverts \u003d no"},{"line_number":459,"context_line":"#"},{"line_number":460,"context_line":"# You can set scheduling priority of processes. Niceness values range from -20"},{"line_number":461,"context_line":"# (most favorable to the process) to 19 (least favorable to the process)."}],"source_content_type":"application/octet-stream","patch_set":2,"id":"dbbfc566_a98591be","line":458,"updated":"2026-08-13 07:04:11.000000000","message":"So this enables rsync in the reconstructor, but an operator needs to look up available rsync config settings (`rsync_timeout`, `rsync_io_timeout`, `rsync_bwlimit`, `rsync_compress`, `rsync_module`) in the replicator section and copy them into this one. I think they need to be documented here as well?","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"}],"swift/obj/reconstructor.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9439799f13f0e184d940449da183a7326ad91d22","unresolved":true,"context_lines":[{"line_number":254,"context_line":"        self.rsync_compress \u003d config_true_value("},{"line_number":255,"context_line":"            conf.get(\u0027rsync_compress\u0027, \u0027no\u0027))"},{"line_number":256,"context_line":"        self.rsync_module \u003d conf.get(\u0027rsync_module\u0027, \u0027\u0027).rstrip(\u0027/\u0027) or \\"},{"line_number":257,"context_line":"            \u0027{replication_ip}::object\u0027"},{"line_number":258,"context_line":"        # When upgrading from liberasurecode\u003c\u003d1.5.0, you may want to continue"},{"line_number":259,"context_line":"        # writing legacy CRCs until all nodes are upgraded and capabale of"},{"line_number":260,"context_line":"        # reading fragments with zlib CRCs."}],"source_content_type":"text/x-python","patch_set":2,"id":"177c5c3f_3ab392f6","line":257,"updated":"2026-08-13 07:04:11.000000000","message":"Do we want `log_rsync_transfers` as well?","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9439799f13f0e184d940449da183a7326ad91d22","unresolved":true,"context_lines":[{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"    def _delete_reverted_suffixes(self, job, suffixes):"},{"line_number":1043,"context_line":"        for suffix in suffixes:"},{"line_number":1044,"context_line":"            shutil.rmtree(join(job[\u0027path\u0027], suffix), ignore_errors\u003dTrue)"},{"line_number":1045,"context_line":""},{"line_number":1046,"context_line":"    def process_job(self, job):"},{"line_number":1047,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"6d132aaa_820b926d","line":1044,"updated":"2026-08-13 07:04:11.000000000","message":"ssync deletes objects via `df.purge()`, which calls `invalidate_hash()` - but that\u0027s not the case here, so the cache is stale after removing the tree.\n\nHow about this?\n\n```\ndef _delete_reverted_suffixes(self, job, suffixes):\n    df_mgr \u003d self._df_router[job[\u0027policy\u0027]]\n    for suffix in suffixes:\n        suffix_dir \u003d join(job[\u0027path\u0027], suffix)\n        shutil.rmtree(suffix_dir, ignore_errors\u003dTrue)\n        df_mgr.invalidate_hash(suffix_dir)\n```","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9439799f13f0e184d940449da183a7326ad91d22","unresolved":true,"context_lines":[{"line_number":1188,"context_line":"            self.logger.error(\u0027Bad rsync return code: %d \u003c- %s\u0027,"},{"line_number":1189,"context_line":"                              return_code, args)"},{"line_number":1190,"context_line":"            return False"},{"line_number":1191,"context_line":"        for line in output.decode(\u0027utf8\u0027).splitlines():"},{"line_number":1192,"context_line":"            self.logger.debug(line)"},{"line_number":1193,"context_line":"        return True"},{"line_number":1194,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"b755569a_bf59c4af","line":1191,"updated":"2026-08-13 07:04:11.000000000","message":"This emits no log at all b/c `--itemize-changes` is missing in the rsync args (see next comment). And I think we want the same as in https://github.com/openstack/swift/blob/master/swift/obj/replicator.py#L410-L420?","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9439799f13f0e184d940449da183a7326ad91d22","unresolved":true,"context_lines":[{"line_number":1199,"context_line":"            \u0027--xattrs\u0027, \u0027--ignore-existing\u0027,"},{"line_number":1200,"context_line":"            \u0027--timeout\u003d%s\u0027 % self.rsync_io_timeout,"},{"line_number":1201,"context_line":"            \u0027--contimeout\u003d%s\u0027 % self.rsync_io_timeout,"},{"line_number":1202,"context_line":"            \u0027--bwlimit\u003d%s\u0027 % self.rsync_bwlimit,"},{"line_number":1203,"context_line":"        ]"},{"line_number":1204,"context_line":"        if self.rsync_compress and \\"},{"line_number":1205,"context_line":"                job[\u0027local_dev\u0027][\u0027region\u0027] !\u003d node[\u0027region\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"09732f9f_9e5c75ac","line":1202,"updated":"2026-08-13 07:04:11.000000000","message":"In the replicator we use `\u0027--exclude\u003d.*.%s\u0027 % \u0027\u0027.join(\u0027[0-9a-zA-Z]\u0027 for i in range(6))` to exclude rsyncs own temp data (https://github.com/openstack/swift/blob/master/swift/obj/replicator.py#L441-L452). I think this should be used here as well?\n\nAlso missing: `--itemize-changes`.","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"17111fac8a7e4aedec1c65f7c1715287a7908858","unresolved":true,"context_lines":[{"line_number":1225,"context_line":"                    try:"},{"line_number":1226,"context_line":"                        conn.getresponse().read()"},{"line_number":1227,"context_line":"                    finally:"},{"line_number":1228,"context_line":"                        conn.close()"},{"line_number":1229,"context_line":"            except (Exception, Timeout):"},{"line_number":1230,"context_line":"                self.logger.exception("},{"line_number":1231,"context_line":"                    \u0027Unable to notify %s after rsync\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"280f277c_c98eb652","line":1228,"updated":"2026-08-13 14:59:45.000000000","message":"This should be more like\n```\n                with Timeout(self.conn_timeout):\n                    conn \u003d http_connect(...)\n                with Timeout(self.http_timeout):\n                    try:\n                        conn.getresponse().read()\n                    finally:\n                        conn.close()\n```\n(ie, separate connect and http timeouts)","commit_id":"32d72ff0c254b91dc7758fe0656684b41903d4c2"}],"test/unit/obj/test_reconstructor.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"2e992d2d058f4ba44fc2a9bc3d60eb96a52eaaa7","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"        reconstructor \u003d object_reconstructor.ObjectReconstructor("},{"line_number":1628,"context_line":"            dict(self.conf, prefer_rsync_reverts\u003d\u0027yes\u0027,"},{"line_number":1629,"context_line":"                 max_objects_per_revert\u003d1), logger\u003dself.logger)"},{"line_number":1630,"context_line":"        sync_method, cleanup_method \u003d reconstructor._dispatch_revert(revert_job)"},{"line_number":1631,"context_line":""},{"line_number":1632,"context_line":"        self.assertEqual(\u0027_rsync_revert\u0027, sync_method.__name__)"},{"line_number":1633,"context_line":"        self.assertEqual(\u0027_delete_reverted_suffixes\u0027, cleanup_method.__name__)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5debf968_f4ffe10f","line":1630,"updated":"2026-08-03 20:04:48.000000000","message":"pep8: E501 line too long (80 \u003e 79 characters)","commit_id":"71c023ad93cbaf1c67e63d085befe0935df32e3d"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"2e992d2d058f4ba44fc2a9bc3d60eb96a52eaaa7","unresolved":false,"context_lines":[{"line_number":4854,"context_line":"        self.assertEqual(self.reconstructor.handoffs_remaining, 0)"},{"line_number":4855,"context_line":""},{"line_number":4856,"context_line":"    def _do_test_process_job_rsync_revert(self, return_codes,"},{"line_number":4857,"context_line":"                                           replicate_responses\u003dNone):"},{"line_number":4858,"context_line":"        partition \u003d 0"},{"line_number":4859,"context_line":"        frag_index \u003d 2"},{"line_number":4860,"context_line":"        part_nodes \u003d self.policy.object_ring.get_part_nodes(partition)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ca1dad86_dc5f9cfc","line":4857,"updated":"2026-08-03 20:04:48.000000000","message":"pep8: E127 continuation line over-indented for visual indent","commit_id":"71c023ad93cbaf1c67e63d085befe0935df32e3d"}]}
