)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d192b1d1ca7ab76a8353eaa3dd76396dbf16b097","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c245ab9c_72b0d8c3","updated":"2026-01-12 17:37:37.000000000","message":"Makes sense to carry, at least for a while until our cleaving contexts have had a chance to turn over. Still thinking about the implications for merging upstream, though.","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89e6791d87c7d1c38a277098e6d916ed16d9e153","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"02748e11_ee347caf","updated":"2026-01-12 16:17:13.000000000","message":"The fix and comment are obvious, the test and commit message are well written.\n\nSo, this might be fine, but as the commit message painfully explains: we don\u0027t think this is a \"real\" problem for master and we (having gotten ourselves into this mess) could at least *consider* carrying a patch to remove (and write back) the problematic data (for a cycle?)\n\n... but maybe this is better and useful on master.","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e42e7e7c364ddaf822f51c6777bce5cb6f7f2348","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d7d0634c_cdd7a9d3","updated":"2026-01-12 16:46:23.000000000","message":"The only argument to maybe consider is if we want old sharders processing new contexts blindly or if we want them to wait and get upgraded.\n\nFor our mess we could carry something more targeted and remove it after a few releases:\n\n973116: DNM: remove CleavingContext.repliation_count | https://review.opendev.org/c/openstack/swift/+/973116","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"}],"swift/container/sharder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89e6791d87c7d1c38a277098e6d916ed16d9e153","unresolved":true,"context_lines":[{"line_number":630,"context_line":"    \"\"\""},{"line_number":631,"context_line":"    # note: the constructor accepts **kwargs just in case any of the documented"},{"line_number":632,"context_line":"    # kwargs are ever removed from the method signature but remain persisted in"},{"line_number":633,"context_line":"    # container metadata"},{"line_number":634,"context_line":"    def __init__(self, ref, cursor\u003d\u0027\u0027, max_row\u003dNone, cleave_to_row\u003dNone,"},{"line_number":635,"context_line":"                 last_cleave_to_row\u003dNone, cleaving_done\u003dFalse,"},{"line_number":636,"context_line":"                 misplaced_done\u003dFalse, ranges_done\u003d0, ranges_todo\u003d0,"}],"source_content_type":"text/x-python","patch_set":2,"id":"c9883e71_d06ea864","line":633,"updated":"2026-01-12 16:17:13.000000000","message":"as a side-effect: on upgrade new keys are silently *ignored* instead of the old-sharder refusing to process the broker with the newly replicated context until it is upgraded itself.","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d192b1d1ca7ab76a8353eaa3dd76396dbf16b097","unresolved":true,"context_lines":[{"line_number":630,"context_line":"    \"\"\""},{"line_number":631,"context_line":"    # note: the constructor accepts **kwargs just in case any of the documented"},{"line_number":632,"context_line":"    # kwargs are ever removed from the method signature but remain persisted in"},{"line_number":633,"context_line":"    # container metadata"},{"line_number":634,"context_line":"    def __init__(self, ref, cursor\u003d\u0027\u0027, max_row\u003dNone, cleave_to_row\u003dNone,"},{"line_number":635,"context_line":"                 last_cleave_to_row\u003dNone, cleaving_done\u003dFalse,"},{"line_number":636,"context_line":"                 misplaced_done\u003dFalse, ranges_done\u003d0, ranges_todo\u003d0,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ec8a9571_c9619183","line":633,"in_reply_to":"c9883e71_d06ea864","updated":"2026-01-12 17:37:37.000000000","message":"...which seems significant -- we\u0027re generally accepting of some tracebacks during rolling upgrades, and I\u0027m sure we\u0027ve used that failure mode before to ensure some background process doesn\u0027t process a work item until it\u0027s been upgraded.\n\nRequiring that any new keys be ignoreable -- I feel like we may find it constraining later. Or at least confusing -- ignoring data *that is written down* starts to smell like data loss...","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ff6cd05b16fddf47588434aaaf2dbcb0655e1085","unresolved":true,"context_lines":[{"line_number":630,"context_line":"    \"\"\""},{"line_number":631,"context_line":"    # note: the constructor accepts **kwargs just in case any of the documented"},{"line_number":632,"context_line":"    # kwargs are ever removed from the method signature but remain persisted in"},{"line_number":633,"context_line":"    # container metadata"},{"line_number":634,"context_line":"    def __init__(self, ref, cursor\u003d\u0027\u0027, max_row\u003dNone, cleave_to_row\u003dNone,"},{"line_number":635,"context_line":"                 last_cleave_to_row\u003dNone, cleaving_done\u003dFalse,"},{"line_number":636,"context_line":"                 misplaced_done\u003dFalse, ranges_done\u003d0, ranges_todo\u003d0,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bccb4f90_147e4b41","line":633,"in_reply_to":"ec8a9571_c9619183","updated":"2026-01-13 16:54:07.000000000","message":"I think I agree that a blanket \"ignore what you don\u0027t recognise\" may not be what we want in future upgrades","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89e6791d87c7d1c38a277098e6d916ed16d9e153","unresolved":true,"context_lines":[{"line_number":716,"context_line":"        data \u003d json.loads(data) if data else {}"},{"line_number":717,"context_line":"        data[\u0027ref\u0027] \u003d ref"},{"line_number":718,"context_line":"        data[\u0027max_row\u0027] \u003d brokers[0].get_max_row()"},{"line_number":719,"context_line":"        return cls(**data)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"    def store(self, broker):"},{"line_number":722,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"09f671b1_3e9abddb","line":719,"updated":"2026-01-12 16:17:13.000000000","message":"i suppose an alternative fix maybe have been something like:\n\n```\ndiff --git a/swift/container/sharder.py b/swift/container/sharder.py\nindex e878b83eb..23b42ebf7 100644\n--- a/swift/container/sharder.py\n+++ b/swift/container/sharder.py\n@@ -711,9 +711,9 @@ class CleavingContext(object):\n         ref \u003d cls._make_ref(brokers[0])\n         data \u003d brokers[-1].get_sharding_sysmeta(\u0027Context-\u0027 + ref)\n         data \u003d json.loads(data) if data else {}\n-        data[\u0027ref\u0027] \u003d ref\n-        data[\u0027max_row\u0027] \u003d brokers[0].get_max_row()\n-        return cls(**data)\n+        filtered_data \u003d {k: data.get(k, v) for k, v in cls(ref)}\n+        filtered_data[\u0027max_row\u0027] \u003d brokers[0].get_max_row()\n+        return cls(**filtered_data)\n \n     def store(self, broker):\n         \"\"\"\n```\n\nI\u0027m not sure it\u0027s particularly virtuous for this class to refuse unexpected kwargs - but FWIW in the related patch the new attribute was removed from `__iter__` at the same time.","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"3a206f1f90e7b5cca11990224ee9c1b9c9bccf3c","unresolved":true,"context_lines":[{"line_number":716,"context_line":"        data \u003d json.loads(data) if data else {}"},{"line_number":717,"context_line":"        data[\u0027ref\u0027] \u003d ref"},{"line_number":718,"context_line":"        data[\u0027max_row\u0027] \u003d brokers[0].get_max_row()"},{"line_number":719,"context_line":"        return cls(**data)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"    def store(self, broker):"},{"line_number":722,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"cebdd938_a6a6c6db","line":719,"in_reply_to":"09f671b1_3e9abddb","updated":"2026-01-12 21:18:25.000000000","message":"this is slightly better which guarantees to get what k/v pairs are defined. but also have the same side affect\n```\non upgrade new keys are silently ignored instead of the old-sharder refusing to process\nthe broker with the newly replicated context until it is upgraded itself.\n```","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0d5beb45f6836224f83e11132ea09eb9a2969b7d","unresolved":true,"context_lines":[{"line_number":716,"context_line":"        data \u003d json.loads(data) if data else {}"},{"line_number":717,"context_line":"        data[\u0027ref\u0027] \u003d ref"},{"line_number":718,"context_line":"        data[\u0027max_row\u0027] \u003d brokers[0].get_max_row()"},{"line_number":719,"context_line":"        return cls(**data)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"    def store(self, broker):"},{"line_number":722,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a66e893_b44bb11b","line":719,"in_reply_to":"cebdd938_a6a6c6db","updated":"2026-01-13 06:00:18.000000000","message":"should we warn and continue on any new \"extra\" keys? So it\u0027s at least not silently ignored but still not traced backed?\n\nSomething like:\n```\ndiff --git i/swift/container/sharder.py w/swift/container/sharder.py\nindex 4d070a9c1..a327bcc11 100644\n--- i/swift/container/sharder.py\n+++ w/swift/container/sharder.py\n@@ -702,7 +702,7 @@ class CleavingContext(object):\n         return contexts\n \n     @classmethod\n-    def load(cls, broker):\n+    def load(cls, broker, logger\u003dNone):\n         \"\"\"\n         Returns a CleavingContext tracking the cleaving progress of the given\n         broker\u0027s DB.\n@@ -716,6 +716,20 @@ class CleavingContext(object):\n         data \u003d json.loads(data) if data else {}\n         data[\u0027ref\u0027] \u003d ref\n         data[\u0027max_row\u0027] \u003d brokers[0].get_max_row()\n+        if logger:\n+            data_set \u003d set(data.keys())\n+            class_set \u003d set(dict(cls(ref).keys()))\n+            obsoleted_keys \u003d data_set.difference(class_set)\n+            if obsoleted_keys:\n+                logger.warning(\n+                    f\u0027CleaveContext obsoleted keys found for {ref}: \u0027\n+                    f\u0027{sorted(obsoleted_keys)}\u0027)\n+            new_keys \u003d class_set.difference(data_set)\n+            if obsoleted_keys:\n+                logger.warning(\n+                    f\u0027New CleaveContext class keys found for {ref}: \u0027\n+                    f\u0027{sorted(class_keys)}\u0027)\n+\n         return cls(**data)\n \n     def store(self, broker):\n```\n\nTho probably overkill","commit_id":"749bfbcc64c3f12e1b72c5b5774ebfc8e6d17ecf"}]}