)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"75ac7dddf6a3c429bfe2963fcb76c56305b76431","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4c204594_72c0932c","updated":"2026-01-12 21:45:19.000000000","message":"I\u0027d also like to get those obsolete fields removed, who knows what other issue they would cause later on. This patch needs to include a rare case of ``load_all`` though.","commit_id":"250ad4c5e8f4e3f3a3e2dc74fb5db4f2269131eb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1a36f6e3585872015c45bf8c519aaf4dae7e3c7a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"55641c0d_86f40480","updated":"2026-01-12 16:48:02.000000000","message":"if we want to go to all the trouble to carry this and then remove it we might want to add some metrics so we can be sure we\u0027ve visited all the dbs and aren\u0027t seeing anymore replication_count\u0027s getting removed.\n\nWe might also have to ensure that shard audit will load all the contexts so we don\u0027t end up with any sleepers.","commit_id":"250ad4c5e8f4e3f3a3e2dc74fb5db4f2269131eb"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"88f31a3d61ff67b1fd29ff2af5b8e654a6b1c88d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f90c4b64_c98c969c","updated":"2026-01-12 21:41:58.000000000","message":"title has a typo: ``repliation_count-\u003ereplication_count``","commit_id":"250ad4c5e8f4e3f3a3e2dc74fb5db4f2269131eb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"710c8f1c651243aa1e4d63f4964c09cf8e3bdf70","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0d75d9d9_583bfcb2","updated":"2026-07-31 13:20:27.000000000","message":"I don\u0027t think it is safe to make modifications in load_all\n\nI left some suggestions here 999416: sq? fixups for replication_count removal | https://review.opendev.org/c/openstack/swift/+/999416","commit_id":"5e1cfded35733f32e6e55619eb3b9441c2923b6f"}],"swift/container/sharder.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"88f31a3d61ff67b1fd29ff2af5b8e654a6b1c88d","unresolved":true,"context_lines":[{"line_number":694,"context_line":"            if key.startswith(\"Context-\") and val:"},{"line_number":695,"context_line":"                try:"},{"line_number":696,"context_line":"                    contexts.append((cls(**json.loads(val)), timestamp))"},{"line_number":697,"context_line":"                except ValueError:"},{"line_number":698,"context_line":"                    continue"},{"line_number":699,"context_line":"        return contexts"},{"line_number":700,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"21b78120_2779709f","line":697,"updated":"2026-01-12 21:41:58.000000000","message":"if a sharded container has already finished cleaving, but got stuck at here (replication_count will cause TypeError exception) for metrics reporting; then the fix in ``load`` won\u0027t remove ``replication_count`` for this container.\n\nif we change this line to\n```\nexcept (ValueError, TypeError):\n```\n\nand then sharder will be unblocked and go through ``load()`` call again to remove ``replication_count``.","commit_id":"250ad4c5e8f4e3f3a3e2dc74fb5db4f2269131eb"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"a2f308c0563bdfdc59f2ee4c7824f0f3fbf2bf4f","unresolved":true,"context_lines":[{"line_number":694,"context_line":"            if key.startswith(\"Context-\") and val:"},{"line_number":695,"context_line":"                try:"},{"line_number":696,"context_line":"                    contexts.append((cls(**json.loads(val)), timestamp))"},{"line_number":697,"context_line":"                except ValueError:"},{"line_number":698,"context_line":"                    continue"},{"line_number":699,"context_line":"        return contexts"},{"line_number":700,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"597a688b_adc51b0b","line":697,"in_reply_to":"21b78120_2779709f","updated":"2026-07-30 18:11:35.000000000","message":"Added the typerror check so things pass through; but to be certain of the attribute\u0027s removal I copied some of the deletion logic from load over to load_all as well. The new test verifies no errs + deleted attr.","commit_id":"250ad4c5e8f4e3f3a3e2dc74fb5db4f2269131eb"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"89cba4c0c0fbfd5ff41eb92d9bbe03b313850d26","unresolved":true,"context_lines":[{"line_number":694,"context_line":"            if key.startswith(\"Context-\") and val:"},{"line_number":695,"context_line":"                try:"},{"line_number":696,"context_line":"                    contexts.append((cls(**json.loads(val)), timestamp))"},{"line_number":697,"context_line":"                except ValueError:"},{"line_number":698,"context_line":"                    continue"},{"line_number":699,"context_line":"        return contexts"},{"line_number":700,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"36a330e2_f86cc481","line":697,"in_reply_to":"597a688b_adc51b0b","updated":"2026-07-31 04:45:29.000000000","message":"turns out adding ``TypeError`` to ``except (ValueError):`` is not enough. ``load()`` is only used by active sharding path, but ``load_all()`` is called potentially by many old/completed contexts simply because audit or metrics inspected them.","commit_id":"250ad4c5e8f4e3f3a3e2dc74fb5db4f2269131eb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"710c8f1c651243aa1e4d63f4964c09cf8e3bdf70","unresolved":true,"context_lines":[{"line_number":699,"context_line":"                        data.pop(\u0027replication_count\u0027)"},{"line_number":700,"context_line":"                        do_repair_writeback \u003d True"},{"line_number":701,"context_line":"                    context \u003d cls(**data)"},{"line_number":702,"context_line":"                except (ValueError, TypeError):"},{"line_number":703,"context_line":"                    continue"},{"line_number":704,"context_line":"                if do_repair_writeback:"},{"line_number":705,"context_line":"                    context.store(broker)"}],"source_content_type":"text/x-python","patch_set":3,"id":"6b337603_b4f961db","line":702,"updated":"2026-07-31 13:20:27.000000000","message":"so here we do tolerate other unexpected keys","commit_id":"5e1cfded35733f32e6e55619eb3b9441c2923b6f"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"89cba4c0c0fbfd5ff41eb92d9bbe03b313850d26","unresolved":true,"context_lines":[{"line_number":702,"context_line":"                except (ValueError, TypeError):"},{"line_number":703,"context_line":"                    continue"},{"line_number":704,"context_line":"                if do_repair_writeback:"},{"line_number":705,"context_line":"                    context.store(broker)"},{"line_number":706,"context_line":"                    timestamp \u003d ("},{"line_number":707,"context_line":"                        broker.get_sharding_sysmeta_with_timestamps()[key][1])"},{"line_number":708,"context_line":"                contexts.append((context, timestamp))"}],"source_content_type":"text/x-python","patch_set":3,"id":"05c9022e_560ee058","line":705,"updated":"2026-07-31 04:45:29.000000000","message":"``get_sharding_sysmeta_with_timestamps`` takes a metadata snapshot, and here serializes that snapshot back through ``context.store()``, which assigns ``Timestamp.now()``.\n\nThere are two consequences:\n1. If another replicator updates that context after the snapshot but before this write, the repair’s newer timestamp can overwrite those newer context fields with the stale snapshot. this is a very small function, chances are low though.\n2. Even without concurrency, it changes the context’s logical last-modified time to “now.”\n\nThis probably is not an issue for load() since the active sharding will overwrite CleaveContext later anyway.","commit_id":"5e1cfded35733f32e6e55619eb3b9441c2923b6f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"710c8f1c651243aa1e4d63f4964c09cf8e3bdf70","unresolved":true,"context_lines":[{"line_number":702,"context_line":"                except (ValueError, TypeError):"},{"line_number":703,"context_line":"                    continue"},{"line_number":704,"context_line":"                if do_repair_writeback:"},{"line_number":705,"context_line":"                    context.store(broker)"},{"line_number":706,"context_line":"                    timestamp \u003d ("},{"line_number":707,"context_line":"                        broker.get_sharding_sysmeta_with_timestamps()[key][1])"},{"line_number":708,"context_line":"                contexts.append((context, timestamp))"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf585493_ac74e34d","line":705,"in_reply_to":"05c9022e_560ee058","updated":"2026-07-31 13:20:27.000000000","message":"\u003e chances are low though\n...which is a massive scale system means \"this will happen\"\n\nIMHO a specific sharder process that is processing a DB replica with id X should NOT be modifying contexts whose ref binds them to a different DB replica, e.g. with id Y. We can be reasonably confident that only one sharder process at a time will do read-modify-write of the context metadata with ref X in DB with id X. We have to allow for another sharder process to be concurrently doing a read-modify-write of the context metadata with ref Y in DB with id Y.\n\nWe only need ``load_all`` to successfully load for the purposes of progress reporting and auditing. We don\u0027t need it for fix the context; the fix can be left to when a sharder is handling the DB whose id is bound to the context and calls ``load``.\n\nOnce sharding has completed in a context, the audit may use the results from ``load_all` to delete the context, so stale polluted contexts should be eventually removed entirely, without being specifically modified to remove ``replication_count``.","commit_id":"5e1cfded35733f32e6e55619eb3b9441c2923b6f"}],"test/unit/container/test_sharder.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"710c8f1c651243aa1e4d63f4964c09cf8e3bdf70","unresolved":true,"context_lines":[{"line_number":8382,"context_line":"            ctx.delete(broker)"},{"line_number":8383,"context_line":"        self.assertEqual([], CleavingContext.load_all(broker))"},{"line_number":8384,"context_line":""},{"line_number":8385,"context_line":"    def test_store_modify_attributes_load_all(self):"},{"line_number":8386,"context_line":"        broker \u003d self._make_sharding_broker()"},{"line_number":8387,"context_line":"        db_id \u003d broker.get_brokers()[0].get_info()[\u0027id\u0027]"},{"line_number":8388,"context_line":"        ctx \u003d CleavingContext(db_id, \u0027curs\u0027, 12, 11, 2, True, True)"}],"source_content_type":"text/x-python","patch_set":3,"id":"6fc2880a_b9ba1683","line":8385,"updated":"2026-07-31 13:20:27.000000000","message":"this test would benefit from having more than one context to load, so that it verifies that the loading continues despite a polluted context being found","commit_id":"5e1cfded35733f32e6e55619eb3b9441c2923b6f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"710c8f1c651243aa1e4d63f4964c09cf8e3bdf70","unresolved":true,"context_lines":[{"line_number":8649,"context_line":"        self.assertFalse(new_ctx.misplaced_done)"},{"line_number":8650,"context_line":"        self.assertFalse(new_ctx.cleaving_done)"},{"line_number":8651,"context_line":""},{"line_number":8652,"context_line":"    def test_store_modify_attributes_load(self):"},{"line_number":8653,"context_line":"        # if the CleavingContext constructor interface ever changes we may have"},{"line_number":8654,"context_line":"        # obsolete attributes persisted in JSON metadata; verify that obsolete"},{"line_number":8655,"context_line":"        # attributes are ignored."}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0bf57c_e0cb19de","line":8652,"updated":"2026-07-31 13:20:27.000000000","message":"this is fine for a patch that we don\u0027t intend to merge upstream.\n\nIf we were merging upstream I\u0027d suggest a test that verifies that any other unexpected key will still raise an error.","commit_id":"5e1cfded35733f32e6e55619eb3b9441c2923b6f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"710c8f1c651243aa1e4d63f4964c09cf8e3bdf70","unresolved":true,"context_lines":[{"line_number":8677,"context_line":""},{"line_number":8678,"context_line":"        # and it\u0027s really gone"},{"line_number":8679,"context_line":"        data \u003d json.loads(broker.metadata[key][0])"},{"line_number":8680,"context_line":"        self.assertNotIn(\u0027replication_count\u0027, data)"},{"line_number":8681,"context_line":""},{"line_number":8682,"context_line":"    def test_load_modify_store_load(self):"},{"line_number":8683,"context_line":"        broker \u003d self._make_sharding_broker()"}],"source_content_type":"text/x-python","patch_set":3,"id":"4d0b6e64_8f95b785","line":8680,"updated":"2026-07-31 13:20:27.000000000","message":"better to assert the whole dict to guard against unexpected modifications","commit_id":"5e1cfded35733f32e6e55619eb3b9441c2923b6f"}]}
