)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"34ba14a912173eed302cb0f6ab78ab7481d015f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7ac8f898_3785efa9","updated":"2024-05-17 05:22:35.000000000","message":"I think the shift to ``task_container_iteration_strategy`` is to support more iteration strategies in future. looks good to me.","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a6a03c61521f63fca5b80315201df9590ba96ef9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ab566072_8f8fbe5f","updated":"2024-05-15 02:26:11.000000000","message":"i\u0027d like to squash this into the carrying patch - i don\u0027t expect SRE to have to change their configs, but adding the \"strategy\" idea for task_iteration seems better aligned with the current state of \"experiment in prod and see what works\"\n\nright now \"randomized\" is working \"ok\" - but a new idea for \"parallel\" is in the works:\n\n918366: Parallel distirbuted task container iteration | https://review.opendev.org/c/openstack/swift/+/918366","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"75eae24718ea198d839f1868ad500591648eefcd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ea5b5f7d_aa468ad6","updated":"2024-05-17 12:18:19.000000000","message":"just a query that the old vs new order of precedence is as you intended","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"aa9c04a95e609d71b24e19608edf24a738faf634","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8d36b5a3_578c85c4","updated":"2024-05-17 15:12:49.000000000","message":"squashed 914713: DNM expirer: new options to control task iteration | https://review.opendev.org/c/openstack/swift/+/914713","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"}],"etc/object-expirer.conf-sample":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"75eae24718ea198d839f1868ad500591648eefcd","unresolved":true,"context_lines":[{"line_number":101,"context_line":"# expirer supports a task_container_iteration_strategy \"randomized\" which seems"},{"line_number":102,"context_line":"# to help avoid longer start-up delays if using large values w/ delay_reaping"},{"line_number":103,"context_line":"# and create less hot spots listing task_containers from many processes."},{"line_number":104,"context_line":"# task_container_iteration_strategy \u003d legacy"},{"line_number":105,"context_line":"#"},{"line_number":106,"context_line":"# Number of tasks objects to cache before processing.  With many nodes it may"},{"line_number":107,"context_line":"# take some time to fill a larger cache_size but may also have a better chance"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"3d1f707e_4f233c5b","line":104,"updated":"2024-05-17 12:18:19.000000000","message":"+1 this is more flexible","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"aa9c04a95e609d71b24e19608edf24a738faf634","unresolved":false,"context_lines":[{"line_number":101,"context_line":"# expirer supports a task_container_iteration_strategy \"randomized\" which seems"},{"line_number":102,"context_line":"# to help avoid longer start-up delays if using large values w/ delay_reaping"},{"line_number":103,"context_line":"# and create less hot spots listing task_containers from many processes."},{"line_number":104,"context_line":"# task_container_iteration_strategy \u003d legacy"},{"line_number":105,"context_line":"#"},{"line_number":106,"context_line":"# Number of tasks objects to cache before processing.  With many nodes it may"},{"line_number":107,"context_line":"# take some time to fill a larger cache_size but may also have a better chance"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"11e5463a_36a76d8e","line":104,"in_reply_to":"3d1f707e_4f233c5b","updated":"2024-05-17 15:12:49.000000000","message":"Acknowledged","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"}],"swift/obj/expirer.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"34ba14a912173eed302cb0f6ab78ab7481d015f6","unresolved":true,"context_lines":[{"line_number":165,"context_line":"            \u0027task_container_iteration_strategy\u0027, \u0027legacy\u0027)"},{"line_number":166,"context_line":"        # XXX temporary shim to support the un-released configuration option"},{"line_number":167,"context_line":"        if config_true_value(conf.get("},{"line_number":168,"context_line":"                \u0027randomized_task_container_iteration\u0027, \u0027false\u0027)):"},{"line_number":169,"context_line":"            self.task_container_iteration_strategy \u003d \"randomized\""},{"line_number":170,"context_line":"        # randomized task container iteration can be useful if there\u0027s lots of"},{"line_number":171,"context_line":"        # tasks in the queue under a configured delay_reaping"}],"source_content_type":"text/x-python","patch_set":1,"id":"1d9036ea_a9ba625d","line":168,"updated":"2024-05-17 05:22:35.000000000","message":"okay, this code will be removed eventually before or after upstream merge.","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"aa9c04a95e609d71b24e19608edf24a738faf634","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            \u0027task_container_iteration_strategy\u0027, \u0027legacy\u0027)"},{"line_number":166,"context_line":"        # XXX temporary shim to support the un-released configuration option"},{"line_number":167,"context_line":"        if config_true_value(conf.get("},{"line_number":168,"context_line":"                \u0027randomized_task_container_iteration\u0027, \u0027false\u0027)):"},{"line_number":169,"context_line":"            self.task_container_iteration_strategy \u003d \"randomized\""},{"line_number":170,"context_line":"        # randomized task container iteration can be useful if there\u0027s lots of"},{"line_number":171,"context_line":"        # tasks in the queue under a configured delay_reaping"}],"source_content_type":"text/x-python","patch_set":1,"id":"202535b6_0b9385e5","line":168,"in_reply_to":"1d9036ea_a9ba625d","updated":"2024-05-17 15:12:49.000000000","message":"definately!  I\u0027ll mark it as DNM when i squash.","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"75eae24718ea198d839f1868ad500591648eefcd","unresolved":true,"context_lines":[{"line_number":166,"context_line":"        # XXX temporary shim to support the un-released configuration option"},{"line_number":167,"context_line":"        if config_true_value(conf.get("},{"line_number":168,"context_line":"                \u0027randomized_task_container_iteration\u0027, \u0027false\u0027)):"},{"line_number":169,"context_line":"            self.task_container_iteration_strategy \u003d \"randomized\""},{"line_number":170,"context_line":"        # randomized task container iteration can be useful if there\u0027s lots of"},{"line_number":171,"context_line":"        # tasks in the queue under a configured delay_reaping"},{"line_number":172,"context_line":"        if self.task_container_iteration_strategy not in \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"c6cc9fae_57850ec0","line":169,"updated":"2024-05-17 12:18:19.000000000","message":"the intention is for the old option overrides the newer option, correct?\n\n```\ndiff --git a/test/unit/obj/test_expirer.py b/test/unit/obj/test_expirer.py\nindex 9759943ce..35b0cc429 100644\n--- a/test/unit/obj/test_expirer.py\n+++ b/test/unit/obj/test_expirer.py\n@@ -212,6 +212,23 @@ class TestObjectExpirer(TestCase):\n         self.assertEqual(\"randomized\", x.task_container_iteration_strategy)\n         self.assertEqual(x.round_robin_task_cache_size, 3000)\n\n+    def test_xxx_randomized_task_container_iteration_old_vs_new(self):\n+        conf \u003d {\n+            \u0027randomized_task_container_iteration\u0027: \u0027false\u0027,\n+            \u0027task_container_iteration_strategy\u0027: \u0027randomized\u0027,\n+        }\n+        x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger,\n+                                  swift\u003dself.fake_swift)\n+        self.assertEqual(\"randomized\", x.task_container_iteration_strategy)\n+\n+        conf \u003d {\n+            \u0027randomized_task_container_iteration\u0027: \u0027yes\u0027,\n+            \u0027task_container_iteration_strategy\u0027: \u0027legacy\u0027,\n+        }\n+        x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger,\n+                                  swift\u003dself.fake_swift)\n+        self.assertEqual(\"randomized\", x.task_container_iteration_strategy)\n+\n     def test_init_medium_round_robin_cache(self):\n         conf \u003d {\n             \u0027round_robin_task_cache_size\u0027: \u00273000\u0027,\n```","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"aa9c04a95e609d71b24e19608edf24a738faf634","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        # XXX temporary shim to support the un-released configuration option"},{"line_number":167,"context_line":"        if config_true_value(conf.get("},{"line_number":168,"context_line":"                \u0027randomized_task_container_iteration\u0027, \u0027false\u0027)):"},{"line_number":169,"context_line":"            self.task_container_iteration_strategy \u003d \"randomized\""},{"line_number":170,"context_line":"        # randomized task container iteration can be useful if there\u0027s lots of"},{"line_number":171,"context_line":"        # tasks in the queue under a configured delay_reaping"},{"line_number":172,"context_line":"        if self.task_container_iteration_strategy not in \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"147a3438_fea35ac2","line":169,"in_reply_to":"c6cc9fae_57850ec0","updated":"2024-05-17 15:12:49.000000000","message":"I didn\u0027t really consider the order of precedence; the \"intention\" is to keep prod working as is until we have SRE change the configs:\n\n    - randomized_task_container_iteration \u003d true\n    + task_container_iteration_strategy \u003d parallel\n    \nbut now that you mention it, yeah i guess i\u0027d rather *not* \"accidently* get legacy if the config explicitly said randomized yes.  So I\u0027m glad I accidently wrote it that way even without the test..","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"75eae24718ea198d839f1868ad500591648eefcd","unresolved":true,"context_lines":[{"line_number":176,"context_line":"                \"\u0027task_container_iteration_strategy \u003d %s\u0027 \""},{"line_number":177,"context_line":"                \"(using legacy)\","},{"line_number":178,"context_line":"                self.task_container_iteration_strategy)"},{"line_number":179,"context_line":"            self.task_container_iteration_strategy \u003d \"legacy\""},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        # with lots of nodes and lots of tasks a large cache size can"},{"line_number":182,"context_line":"        # significantly delay processing; and caching tasks to round_robin"}],"source_content_type":"text/x-python","patch_set":1,"id":"9969c6b2_22c70ac8","line":179,"updated":"2024-05-17 12:18:19.000000000","message":"this pattern could be a nice addition to the helpers in common.utils.config e.g. config_choice_value(value, \u003cset of choices\u003e, default)","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"aa9c04a95e609d71b24e19608edf24a738faf634","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \"\u0027task_container_iteration_strategy \u003d %s\u0027 \""},{"line_number":177,"context_line":"                \"(using legacy)\","},{"line_number":178,"context_line":"                self.task_container_iteration_strategy)"},{"line_number":179,"context_line":"            self.task_container_iteration_strategy \u003d \"legacy\""},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        # with lots of nodes and lots of tasks a large cache size can"},{"line_number":182,"context_line":"        # significantly delay processing; and caching tasks to round_robin"}],"source_content_type":"text/x-python","patch_set":1,"id":"c8d18ee3_27988c1b","line":179,"in_reply_to":"9969c6b2_22c70ac8","updated":"2024-05-17 15:12:49.000000000","message":"that is a really good idea!  If this schema makes it\u0027s way onto master we should either remember to look out for a second use-case (rsync|ssync?) or even consider enshrining the pattern in utils.config as a kind of \"preemtive technical investment\"\n\nAt this stage porting to utils and making the existing tests generic feels like it might be YAGNI - because I don\u0027t even know what strategies are going to work well enough to want to polish up and merge!  Once we have an idea we like, we might not want to maintain this configurability.","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"34ba14a912173eed302cb0f6ab78ab7481d015f6","unresolved":true,"context_lines":[{"line_number":181,"context_line":"        # with lots of nodes and lots of tasks a large cache size can"},{"line_number":182,"context_line":"        # significantly delay processing; and caching tasks to round_robin"},{"line_number":183,"context_line":"        # target container order may be less necessary if there\u0027s only a few"},{"line_number":184,"context_line":"        # target containers or with randomized task container iteration"},{"line_number":185,"context_line":"        self.round_robin_task_cache_size \u003d int("},{"line_number":186,"context_line":"            conf.get(\u0027round_robin_task_cache_size\u0027, MAX_OBJECTS_TO_CACHE))"},{"line_number":187,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f1635553_170b818d","line":184,"updated":"2024-05-17 05:22:35.000000000","message":"even with ``only a few target containers or with randomized task container iteration``, I feel the motivation for ``caching tasks to round_robin target container order`` is still valid: avoid to hotspot a specific target container.","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"aa9c04a95e609d71b24e19608edf24a738faf634","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        # with lots of nodes and lots of tasks a large cache size can"},{"line_number":182,"context_line":"        # significantly delay processing; and caching tasks to round_robin"},{"line_number":183,"context_line":"        # target container order may be less necessary if there\u0027s only a few"},{"line_number":184,"context_line":"        # target containers or with randomized task container iteration"},{"line_number":185,"context_line":"        self.round_robin_task_cache_size \u003d int("},{"line_number":186,"context_line":"            conf.get(\u0027round_robin_task_cache_size\u0027, MAX_OBJECTS_TO_CACHE))"},{"line_number":187,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"2cb367b5_b866030e","line":184,"in_reply_to":"f1635553_170b818d","updated":"2024-05-17 15:12:49.000000000","message":"If there\u0027s only 1 target container under grace (nvetl) and 100M tasks become due - it doesn\u0027t matter if we cache 1K or 100K before we start doing DELETEs they\u0027re all going to want to udpate that one target container.","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"}],"test/unit/obj/test_expirer.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"75eae24718ea198d839f1868ad500591648eefcd","unresolved":true,"context_lines":[{"line_number":261,"context_line":"        self.assertEqual(["},{"line_number":262,"context_line":"            \"Unrecognized config value: \u0027task_container_iteration_strategy \""},{"line_number":263,"context_line":"            \"\u003d next-great-thing\u0027 (using legacy)\""},{"line_number":264,"context_line":"        ], self.logger.get_lines_for_level(\u0027warning\u0027))"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    def test_init_internal_client_log_name(self):"},{"line_number":267,"context_line":"        def _do_test_init_ic_log_name(conf, exp_internal_client_log_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"59aedc66_388f1aed","line":264,"updated":"2024-05-17 12:18:19.000000000","message":"nice, covers all the bases","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"aa9c04a95e609d71b24e19608edf24a738faf634","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        self.assertEqual(["},{"line_number":262,"context_line":"            \"Unrecognized config value: \u0027task_container_iteration_strategy \""},{"line_number":263,"context_line":"            \"\u003d next-great-thing\u0027 (using legacy)\""},{"line_number":264,"context_line":"        ], self.logger.get_lines_for_level(\u0027warning\u0027))"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    def test_init_internal_client_log_name(self):"},{"line_number":267,"context_line":"        def _do_test_init_ic_log_name(conf, exp_internal_client_log_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"79637b29_ea809c41","line":264,"in_reply_to":"59aedc66_388f1aed","updated":"2024-05-17 15:12:49.000000000","message":"Acknowledged","commit_id":"014058097b89fe6bb8e89b63e10ccc4517b4e9a7"}]}
