)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"8563cf59e4f343da0d39e7ba502754a5e2e2faa0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7b21038d_7c897fad","updated":"2026-07-08 06:39:43.000000000","message":"recheck\n\nTimeout with mirror.gra1.ovh.opendev.org","commit_id":"260fec51ade0e14c6151a2aeac59925648216e78"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy","status":"I\u0027m a Software Engineer at Red Hat and I love Open Source and connect with people! Feel free to DM through IRC, I\u0027ll be delighted to chat"},"change_message_id":"b658716741e5710a9b7adf0966a8e1e52c21ce68","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e2b4ad26_ceb9eb08","updated":"2026-07-14 13:24:01.000000000","message":"This is looking good already. The two hardest code paths (chunked parser, drain detection) are good. The one issue that I found I pointed in a comment about `load_config()` crashing the service on a config typo during SIGHUP reload. But after that should be a +1","commit_id":"48dd6d94e7061f06dd2963955f50035a651daa30"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"177eeb66cb977c5570b25139901c24802733f7a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e83302b3_b47f5115","updated":"2026-09-14 09:48:24.000000000","message":"Self-approving on feature branch after discussing within the core reviewers team.","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9e5194521a7b2f4c7c67f49f48ddfebac6d9cc69","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b43ab23d_f8b82f07","updated":"2026-09-14 09:47:00.000000000","message":"Thanks @maallen@nvidia.com for the review, I think all comments should be now adressed in the follow up patches.","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"}],"swift/common/wsgi_gunicorn.py":[{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy","status":"I\u0027m a Software Engineer at Red Hat and I love Open Source and connect with people! Feel free to DM through IRC, I\u0027ll be delighted to chat"},"change_message_id":"b658716741e5710a9b7adf0966a8e1e52c21ce68","unresolved":true,"context_lines":[{"line_number":599,"context_line":"        # load_default_config() first, resetting bind to its default"},{"line_number":600,"context_line":"        # 127.0.0.1:8000; a no-op here would lose our real bind/workers and"},{"line_number":601,"context_line":"        # reloaded masters would collide trying to rebind 8000."},{"line_number":602,"context_line":"        self.cfg \u003d self.build_cfg()"},{"line_number":603,"context_line":""},{"line_number":604,"context_line":"    def load(self):"},{"line_number":605,"context_line":"        return self.load_app()"}],"source_content_type":"text/x-python","patch_set":5,"id":"c83fbe83_6abb16a0","line":602,"updated":"2026-07-14 13:24:01.000000000","message":"This call here needs to be inside a try/except block to survive a bad SIGHUP reload.\n\n`build_cfg()` calls `appconfig()`, `reload_constraints()`, `reload_storage_policies()` and `int()` conversions. Any of these can rainse. On SIGHUP, gunicorn\u0027s Arbiter.reload() calls `load_config()` unguarded, the exception propagates to the arbiter\u0027s main loop, where the generic except-Exception handler calls `self.stop(False)` then `sys.exit(-1)`. A single config typo during a graceful reload kills the master and all workers.\n\nI reproduced in a SAIO machine:\n\n```bash\n# Start proxy under gunicorn\nsudo env USE_EVENTLET\u003dfalse swift-proxy-server /etc/swift/proxy-server/proxy-server.conf.d \u0026\n\n# Inject bad config into [DEFAULT]\nsudo sed -i \u0027/^\\[DEFAULT\\]/a threads \u003d abc\u0027 /etc/swift/proxy-server/proxy-server.conf.d/00_base.conf\n\n# Send SIGHUP to master\nsudo kill -HUP \u003cmaster_pid\u003e\n```\n\nFrom the log:\n\n```\n2026-07-14T13:02:39.760591+00:00 saio proxy-server: STDERR: Error: invalid literal for int() with base 10: \u0027abc\u0027\n2026-07-14T13:02:40.254834+00:00 saio proxy-server: STDERR: [2026-07-14 13:02:40 +0000] [6630] [INFO] Parent changed, shutting down: \u003cWorker 6630\u003e\n2026-07-14T13:02:40.256194+00:00 saio proxy-server: STDERR: [2026-07-14 13:02:40 +0000] [6630] [INFO] Worker exiting (pid: 6630)\n```\n\nThe master died from the unhandled ValueError; the worker detected \"Parent changed\" (master PID gone) and self-terminated. Service went fully down, healthcheck returns 000 (connection refused).","commit_id":"48dd6d94e7061f06dd2963955f50035a651daa30"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy","status":"I\u0027m a Software Engineer at Red Hat and I love Open Source and connect with people! Feel free to DM through IRC, I\u0027ll be delighted to chat"},"change_message_id":"7e5e4d9ebd5ab8e85ad4976cb3ac1dc9c18d5df8","unresolved":true,"context_lines":[{"line_number":599,"context_line":"        # load_default_config() first, resetting bind to its default"},{"line_number":600,"context_line":"        # 127.0.0.1:8000; a no-op here would lose our real bind/workers and"},{"line_number":601,"context_line":"        # reloaded masters would collide trying to rebind 8000."},{"line_number":602,"context_line":"        self.cfg \u003d self.build_cfg()"},{"line_number":603,"context_line":""},{"line_number":604,"context_line":"    def load(self):"},{"line_number":605,"context_line":"        return self.load_app()"}],"source_content_type":"text/x-python","patch_set":5,"id":"a8ea79fe_91f96321","line":602,"in_reply_to":"46ed288c_c3e58efb","updated":"2026-07-27 10:30:29.000000000","message":"Yeah I believe overriding `reload()` instead of wrapping `load_config()` is the right call. I just have a question: when `build_cfg()` fails halfway through, it has already changed some global state — `reload_constraints()`, `reload_storage_policies()`, and `_CLIENT_TIMEOUT` all run before the config object is fully built. So after a failed reload, the service would be running with the old gunicorn config (bind, workers, threads) but new constraints, storage policies, and client timeout. Is that the intended behaviour?","commit_id":"48dd6d94e7061f06dd2963955f50035a651daa30"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"09d5f562c25cad896782d7d1c1b48610aa0d4c9d","unresolved":false,"context_lines":[{"line_number":599,"context_line":"        # load_default_config() first, resetting bind to its default"},{"line_number":600,"context_line":"        # 127.0.0.1:8000; a no-op here would lose our real bind/workers and"},{"line_number":601,"context_line":"        # reloaded masters would collide trying to rebind 8000."},{"line_number":602,"context_line":"        self.cfg \u003d self.build_cfg()"},{"line_number":603,"context_line":""},{"line_number":604,"context_line":"    def load(self):"},{"line_number":605,"context_line":"        return self.load_app()"}],"source_content_type":"text/x-python","patch_set":5,"id":"22c206ca_51e2db96","line":602,"in_reply_to":"a8ea79fe_91f96321","updated":"2026-08-21 07:46:44.000000000","message":"Right! With Matt\u0027s follow up this should be solved now.","commit_id":"48dd6d94e7061f06dd2963955f50035a651daa30"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"02f847da8150d0170e36ba8010bb0d411a4da019","unresolved":true,"context_lines":[{"line_number":599,"context_line":"        # load_default_config() first, resetting bind to its default"},{"line_number":600,"context_line":"        # 127.0.0.1:8000; a no-op here would lose our real bind/workers and"},{"line_number":601,"context_line":"        # reloaded masters would collide trying to rebind 8000."},{"line_number":602,"context_line":"        self.cfg \u003d self.build_cfg()"},{"line_number":603,"context_line":""},{"line_number":604,"context_line":"    def load(self):"},{"line_number":605,"context_line":"        return self.load_app()"}],"source_content_type":"text/x-python","patch_set":5,"id":"46ed288c_c3e58efb","line":602,"in_reply_to":"c83fbe83_6abb16a0","updated":"2026-07-27 09:43:26.000000000","message":"Maybe somethihng like: https://review.opendev.org/c/openstack/swift/+/998747 ?","commit_id":"48dd6d94e7061f06dd2963955f50035a651daa30"},{"author":{"_account_id":39164,"name":"Matthew Allen","display_name":"Matthew Allen","email":"maallen@nvidia.com","username":"matthewallen"},"change_message_id":"53a391e6c7e15154b97096aa3f27d38c8c1bc2e9","unresolved":true,"context_lines":[{"line_number":783,"context_line":"    cfg.set(\u0027header_map\u0027, \u0027dangerous\u0027)"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    # Defaults to 30 seconds, should be less than common.manager.KILL_WAIT"},{"line_number":786,"context_line":"    cfg.set(\u0027graceful_timeout\u0027, 5)"},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"    cfg.set(\u0027limit_request_fields\u0027, int(constraints.MAX_HEADER_COUNT * 1.6))"},{"line_number":789,"context_line":"    # eventlet rejected a header line \u003e\u003d MAX_HEADER_SIZE (400) and a request"}],"source_content_type":"text/x-python","patch_set":7,"id":"0e0a5100_c0e3f6b7","line":786,"updated":"2026-08-22 02:39:15.000000000","message":"Agentic review flagged a concern that this may significantly shorten the drain window for in-flight requests vs. mainline. I am placing a note here to follow up on this concern next week--I\u0027m not convinced this is a big deal since quorum writes and repair should be able to deal with most cases. Is this bounded by the KILL_WAIT constraint above? What do we think a reasonable drain window would be?","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9e5194521a7b2f4c7c67f49f48ddfebac6d9cc69","unresolved":true,"context_lines":[{"line_number":783,"context_line":"    cfg.set(\u0027header_map\u0027, \u0027dangerous\u0027)"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    # Defaults to 30 seconds, should be less than common.manager.KILL_WAIT"},{"line_number":786,"context_line":"    cfg.set(\u0027graceful_timeout\u0027, 5)"},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"    cfg.set(\u0027limit_request_fields\u0027, int(constraints.MAX_HEADER_COUNT * 1.6))"},{"line_number":789,"context_line":"    # eventlet rejected a header line \u003e\u003d MAX_HEADER_SIZE (400) and a request"}],"source_content_type":"text/x-python","patch_set":7,"id":"8717f096_863e3a77","line":786,"in_reply_to":"0e0a5100_c0e3f6b7","updated":"2026-09-14 09:47:00.000000000","message":"Yes, the drain window is bounded. After SIGTERM a gthread worker stops accepting and drains in-flight requests for at most graceful_timeout, then exits\n\nOn stop, the arbiter sends SIGKILL after the same window. KILL_WAIT (15s) is the outer bound: swift-init sends SIGKILL to the group after kill_wait, so graceful_timeout must stay below it. That is why the value is 5.\n\n\nI proposed a new patch to restore the eventlet drain window: https://review.opendev.org/c/openstack/swift/+/1005516.","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":39164,"name":"Matthew Allen","display_name":"Matthew Allen","email":"maallen@nvidia.com","username":"matthewallen"},"change_message_id":"53a391e6c7e15154b97096aa3f27d38c8c1bc2e9","unresolved":true,"context_lines":[{"line_number":910,"context_line":"    # serves all bound sockets, so this restores listeners on every port but"},{"line_number":911,"context_line":"    # does NOT give the per-port process isolation of the eventlet"},{"line_number":912,"context_line":"    # ServersPerPortStrategy, and changed ring ports take effect on a reload"},{"line_number":913,"context_line":"    # (SIGHUP) rather than the eventlet ring_check_interval poll."},{"line_number":914,"context_line":"    ip \u003d conf.get(\u0027bind_ip\u0027, \u00270.0.0.0\u0027)"},{"line_number":915,"context_line":"    spp \u003d (app_section \u003d\u003d \u0027object-server\u0027"},{"line_number":916,"context_line":"           and int(conf.get(\u0027servers_per_port\u0027, \u00270\u0027) or 0))"}],"source_content_type":"text/x-python","patch_set":7,"id":"55d8f7d4_29b513d7","line":913,"updated":"2026-08-22 02:39:15.000000000","message":"Have we contemplated the operational story for the above? Previously rings were picked up automatically via the ring_check_interval poll. Does this mean SIGHUP on every object server node would need to be a part of rebalance or capacity addition operations?","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9e5194521a7b2f4c7c67f49f48ddfebac6d9cc69","unresolved":true,"context_lines":[{"line_number":910,"context_line":"    # serves all bound sockets, so this restores listeners on every port but"},{"line_number":911,"context_line":"    # does NOT give the per-port process isolation of the eventlet"},{"line_number":912,"context_line":"    # ServersPerPortStrategy, and changed ring ports take effect on a reload"},{"line_number":913,"context_line":"    # (SIGHUP) rather than the eventlet ring_check_interval poll."},{"line_number":914,"context_line":"    ip \u003d conf.get(\u0027bind_ip\u0027, \u00270.0.0.0\u0027)"},{"line_number":915,"context_line":"    spp \u003d (app_section \u003d\u003d \u0027object-server\u0027"},{"line_number":916,"context_line":"           and int(conf.get(\u0027servers_per_port\u0027, \u00270\u0027) or 0))"}],"source_content_type":"text/x-python","patch_set":7,"id":"a38bf4de_cf55c234","line":913,"in_reply_to":"55d8f7d4_29b513d7","updated":"2026-09-14 09:47:00.000000000","message":"Good point. I proposed a new patch to fix this: 1005517: gunicorn: poll the ring in the per-port supervisor | https://review.opendev.org/c/openstack/swift/+/1005517","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":39164,"name":"Matthew Allen","display_name":"Matthew Allen","email":"maallen@nvidia.com","username":"matthewallen"},"change_message_id":"53a391e6c7e15154b97096aa3f27d38c8c1bc2e9","unresolved":true,"context_lines":[{"line_number":917,"context_line":"    if spp:"},{"line_number":918,"context_line":"        ports \u003d sorted(BindPortsCache("},{"line_number":919,"context_line":"            conf.get(\u0027swift_dir\u0027, \u0027/etc/swift\u0027),"},{"line_number":920,"context_line":"            conf.get(\u0027ring_ip\u0027, ip)).all_bind_ports_for_node())"},{"line_number":921,"context_line":"        return ([_bind_str(ip, p) for p in ports],"},{"line_number":922,"context_line":"                spp * max(1, len(ports)))"},{"line_number":923,"context_line":"    return (_bind_str(ip, int(conf[\u0027bind_port\u0027])),"}],"source_content_type":"text/x-python","patch_set":7,"id":"3ad6f7cb_1bce7082","line":920,"updated":"2026-08-22 02:39:15.000000000","message":"Do ports orphaned by disk removals from the ring hang around until the\nnext config reload?\n\nServersPerPortStrategy.new_worker_socks() handles this inline today: any\n(port, idx) pair that\u0027s no longer in the ring gets shutdown_safe(sock) +\nsock.close(). The shutdown() propagates across the fork, so the child\nstops accepting, drains in-flight requests via pool.waitall(), and\nexits with no operator action.","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9e5194521a7b2f4c7c67f49f48ddfebac6d9cc69","unresolved":true,"context_lines":[{"line_number":917,"context_line":"    if spp:"},{"line_number":918,"context_line":"        ports \u003d sorted(BindPortsCache("},{"line_number":919,"context_line":"            conf.get(\u0027swift_dir\u0027, \u0027/etc/swift\u0027),"},{"line_number":920,"context_line":"            conf.get(\u0027ring_ip\u0027, ip)).all_bind_ports_for_node())"},{"line_number":921,"context_line":"        return ([_bind_str(ip, p) for p in ports],"},{"line_number":922,"context_line":"                spp * max(1, len(ports)))"},{"line_number":923,"context_line":"    return (_bind_str(ip, int(conf[\u0027bind_port\u0027])),"}],"source_content_type":"text/x-python","patch_set":7,"id":"7b800b2a_6e81ef06","line":920,"in_reply_to":"3ad6f7cb_1bce7082","updated":"2026-09-14 09:47:00.000000000","message":"Yes, and it is now fixed with 1005517: gunicorn: poll the ring in the per-port supervisor | https://review.opendev.org/c/openstack/swift/+/1005517 as well.","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":39164,"name":"Matthew Allen","display_name":"Matthew Allen","email":"maallen@nvidia.com","username":"matthewallen"},"change_message_id":"53a391e6c7e15154b97096aa3f27d38c8c1bc2e9","unresolved":true,"context_lines":[{"line_number":919,"context_line":"            conf.get(\u0027swift_dir\u0027, \u0027/etc/swift\u0027),"},{"line_number":920,"context_line":"            conf.get(\u0027ring_ip\u0027, ip)).all_bind_ports_for_node())"},{"line_number":921,"context_line":"        return ([_bind_str(ip, p) for p in ports],"},{"line_number":922,"context_line":"                spp * max(1, len(ports)))"},{"line_number":923,"context_line":"    return (_bind_str(ip, int(conf[\u0027bind_port\u0027])),"},{"line_number":924,"context_line":"            _resolve_worker_count(conf, logger))"},{"line_number":925,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"8c724171_80229cb2","line":922,"updated":"2026-08-22 02:39:15.000000000","message":"Capturing the concern that was discussed with @cschwede@nvidia.com today with a more thoroughly articulated explanation: \nAs noted in the comment above, we are binding every local ring port to a single arbiter and returning spp * max(1, len(ports)) generic workers. The worker count matches eventlet, but gunicorn\u0027s Arbiter.spawn_worker() passes self.LISTENERS to every worker (unless I\u0027m missing something), so each one accepts on every port (gunicorn supports multiple binds).\n\nThe deployment guide gives the motivation for servers_per_port as (thanks Claude):\n\"Because any object-server worker can service a request for any disk, and a slow I/O request blocks the eventlet hub, a single slow disk can impair an entire storage node.\" Real threads alleviate the hub-blocking part to some degree, but without isolation between disks we\u0027re back to any worker servicing any disk, so the entire object-server instance may perform poorly in the presence of a bad disk or disks.\n\nI started exploring options for restoring servers-per-port behavior, and it seems like assigning a port in gunicorn\u0027s pre_fork hook and pruning worker.sockets to it in post_fork might be the least invasive way to maintain consistency with eventlet behavior: it keeps one arbiter and one pid per conf file, so the swift-init/swift reload work is unaffected. (There are some details to work through with config reload.) I\u0027d be happy to draft a follow-on change if that is helpful, but I understand if this is something you want to address.","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9e5194521a7b2f4c7c67f49f48ddfebac6d9cc69","unresolved":true,"context_lines":[{"line_number":919,"context_line":"            conf.get(\u0027swift_dir\u0027, \u0027/etc/swift\u0027),"},{"line_number":920,"context_line":"            conf.get(\u0027ring_ip\u0027, ip)).all_bind_ports_for_node())"},{"line_number":921,"context_line":"        return ([_bind_str(ip, p) for p in ports],"},{"line_number":922,"context_line":"                spp * max(1, len(ports)))"},{"line_number":923,"context_line":"    return (_bind_str(ip, int(conf[\u0027bind_port\u0027])),"},{"line_number":924,"context_line":"            _resolve_worker_count(conf, logger))"},{"line_number":925,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ec39d13c_3c2b9927","line":922,"in_reply_to":"8c724171_80229cb2","updated":"2026-09-14 09:47:00.000000000","message":"Yes, and this should be now fixed in 1005517: gunicorn: poll the ring in the per-port supervisor | https://review.opendev.org/c/openstack/swift/+/1005517.","commit_id":"6ac0be9fe3124d3d02bb6c12aaec9e30140b93a8"}]}