)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"bdf8a40b2f13054c0b79125d20e7f3118a1c234a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2004d92b_5c92ebdf","updated":"2026-07-23 09:25:08.000000000","message":"Code LGTM, one question inline - not a blocker.\n\nThe idea is to remove operational friction? Today one could run a manual additional expirer for a failed process, but this needs manual work on a node. With this patch it\u0027s just a config change and a service restart - which should be covered by deployment tools. Right?","commit_id":"bd0b04e4ab1f5a205f0c1f14dd8c7a325e38007e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8cfbaf1c30c8244d06a16b13731fd1453dd9b46c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e70848e2_aa7aca5e","in_reply_to":"2004d92b_5c92ebdf","updated":"2026-08-27 00:57:05.000000000","message":"yeah that\u0027s the skinny; a strech goal might be just always deploy all nodes to cover for some others using some modulo math?\n\nThe process_ids are sort of roughly handled in priority order based on how their presented; with `processes \u003d 2` you could have:\n\nNode A: `process \u003d 0, 1`\nNode B: `process \u003d 1, 0`\n\nIn this case Node A should ~ hanndle new expirer work that comes do modulo `process \u003d 0` while Node B is also scannning for new work and discovers DELETEs in the section modulo `process \u003d 0` - because it\u0027s never incorrect to \"re-progress\" the queue concurrently and the handling is idempotent - it\u0027s \"only\" an efficiency concern.\n\nAt scale with 100\u0027s of nodes working on 1/100th or 3/100ths of the namespace (esp if 2/100ths are just \"sanity check\" noops) you should have headroom to have inherent redundancy w/o constantly reconfiguring your expirer configs.  TBD\n\nIn practice we had \"while is this expired object not reaped; oh that node was down for 2wks\" - we could have fixed it with a foreground job, but:\n\nyes, having sre just change a config and restart service \n\n... would have been easier than setting up a targeted non-conflicting foreground job that runs a dedicated expirer somewhere.","commit_id":"bd0b04e4ab1f5a205f0c1f14dd8c7a325e38007e"}],"swift/obj/expirer.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"bdf8a40b2f13054c0b79125d20e7f3118a1c234a","unresolved":true,"context_lines":[{"line_number":416,"context_line":"        indexes, existing_divisor \u003d \\"},{"line_number":417,"context_line":"            self._process_context_by_task_container.setdefault("},{"line_number":418,"context_line":"                task_container, (set(), divisor))"},{"line_number":419,"context_line":"        if existing_divisor !\u003d divisor:"},{"line_number":420,"context_line":"            raise ValueError("},{"line_number":421,"context_line":"                \u0027process values must share a task container divisor\u0027)"},{"line_number":422,"context_line":"        indexes.add(process_index)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bbaf6070_4f9fd7f5","line":419,"updated":"2026-07-23 09:25:08.000000000","message":"When can this ever happen? Same container should be same divisor, or not? Should only happen if another/new expirer is started with a different config, or not?","commit_id":"bd0b04e4ab1f5a205f0c1f14dd8c7a325e38007e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8cfbaf1c30c8244d06a16b13731fd1453dd9b46c","unresolved":true,"context_lines":[{"line_number":416,"context_line":"        indexes, existing_divisor \u003d \\"},{"line_number":417,"context_line":"            self._process_context_by_task_container.setdefault("},{"line_number":418,"context_line":"                task_container, (set(), divisor))"},{"line_number":419,"context_line":"        if existing_divisor !\u003d divisor:"},{"line_number":420,"context_line":"            raise ValueError("},{"line_number":421,"context_line":"                \u0027process values must share a task container divisor\u0027)"},{"line_number":422,"context_line":"        indexes.add(process_index)"}],"source_content_type":"text/x-python","patch_set":1,"id":"4258b9b3_4545e4a1","line":419,"in_reply_to":"bbaf6070_4f9fd7f5","updated":"2026-08-27 00:57:05.000000000","message":"exactly, if the caller tries to pass in NEW/different divisor for a container that\u0027s already been registered that would be a bug - and since the current code is *bug free* ;) - that never happens.\n\nThe idea is that you could configure `process \u003d 1, 4` and depending on your processes and container-per-day maybe those BOTH get the a slice of the task container `172799`\n\ne.g. in the calling loop\n\n```\nprocess_ids \u003d\u003d (1, 4)\n```\n\nFor `processes\u003d6`, `task_container_per_day\u003d3`:\n\n```\n  process_id   container_offset\n  0            0\n  1            1\n  2            2\n  3            0\n  4            1\n  5            2\n```\n\nBoth IDs select offset-1 containers, but they split task objects in those containers: index 0 handles one half, index 1 handles the other half. \n\n```\nprocess_id   container offset   process_index   divisor\n1            1                  0               2\n4            1                  1               2\n```\n\nThus a daemon covering process_ids\u003d(1,4) might call:\n\n```\n  _record_task_container_context(\u002786399\u0027, 0, 2)\n  _record_task_container_context(\u0027172799\u0027, 0, 2)\n  _record_task_container_context(\u002786399\u0027, 1, 2)\n  _record_task_container_context(\u0027172799\u0027, 1, 2)\n```\n\nIMHO it\u0027s not so super obvious that `_record_task_container_context(\u0027172799\u0027, 2, 3)` at that point would be a terrible terrible bug; and since we\u0027d already previously been told the task_assignement process context that covered the mod index map for `172799` must map with-in a divisor of n-2 ... we really \u0027ought shouldn\u0027t let a subsequent call *change* the meaning of previous calls?!\n\nOTOH, there\u0027s probably lots of validation we could do for arguments, checking every call in the loop against `self.processes` and `.expirer_config` - but it\u0027s slightly more obvious (to me) that `_record_task_container_context(\u0027172799\u0027, 2, 2)` is a always a bug because `process_index \u003c 0 or process_index \u003e\u003d divisor` is an invarient that doesn\u0027t depend on the previous state; so I didn\u0027t add a ValueError for that.  YMMV\n\nIMHO, the guard should stay to help future refactors fail early - and also that if we\u0027re going to keep it deserves a test; I\u0027ve added `test_parallel_strategy_rejects_mismatched_container_divisor` 🙏","commit_id":"bd0b04e4ab1f5a205f0c1f14dd8c7a325e38007e"}]}