)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dc16996c5378c44e15eb4bf82bc55d1e14eb883c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ab2cbaf6_2756088a","updated":"2022-07-19 17:54:09.000000000","message":"(probe test failure looks unrelated, fwiw -- probably just a slow/overloaded VM)","commit_id":"a5c1444faa827dc2e3d85468d42622acea1d435e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"026ac57c484195439f66998d0cbc05d5e5e18e99","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"724921a2_f00dee20","updated":"2022-07-19 15:25:52.000000000","message":"Good catch! I\u0027m a little concerned about draining the response -- WDYT about just closing?","commit_id":"a5c1444faa827dc2e3d85468d42622acea1d435e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"79037aced312e560b32c23329d617abaa692b063","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"36846e1c_4cee9455","updated":"2022-07-22 22:28:32.000000000","message":"recheck\n\nTook a crack at limiting our draining in https://review.opendev.org/c/openstack/swift/+/850782 -- think you could try it out? You were right; the test changes got a little funky -- but might be worth it.","commit_id":"a5c1444faa827dc2e3d85468d42622acea1d435e"}],"swift/common/middleware/slo.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"026ac57c484195439f66998d0cbc05d5e5e18e99","unresolved":true,"context_lines":[{"line_number":1533,"context_line":"                    raise HTTPServerError(\u0027Unable to load SLO manifest\u0027)"},{"line_number":1534,"context_line":"            else:"},{"line_number":1535,"context_line":"                # Drain and close GET request (prevents socket leaks)"},{"line_number":1536,"context_line":"                drain_and_close(resp)"},{"line_number":1537,"context_line":"                raise HTTPBadRequest(\u0027Not an SLO manifest\u0027)"},{"line_number":1538,"context_line":"        elif resp.status_int \u003d\u003d HTTP_NOT_FOUND:"},{"line_number":1539,"context_line":"            raise HTTPNotFound(\u0027SLO manifest not found\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7d31a03d_1fa5aa12","line":1536,"updated":"2022-07-19 15:25:52.000000000","message":"What if the object was still fairly large (multiple gigs, say)? Maybe we could just do a\n\n close_if_possible(resp)\n\n? Are the 499s annoying?\n\nAlternatively, we could include a range header like bytes\u003d-1 and tell the object server to ignore the range header if it\u0027s an SLO manifest, similar to what we do in https://github.com/openstack/swift/commit/e8b654f318cfab485a772e19db33cb2fe4c3d858 -- though it gets a little annoying if we have to worry about rolling upgrades from pre-2.24.0 swift. General advice is to upgrade storage layer before proxies (in which case there isn\u0027t actually a concern), but that isn\u0027t always practical.","commit_id":"a5c1444faa827dc2e3d85468d42622acea1d435e"},{"author":{"_account_id":28499,"name":"Romain de Joux","email":"romain.de-joux@ovhcloud.com","username":"rdejoux"},"change_message_id":"6d85cf4159fb67d91559994aa025a56d8f4e5ae5","unresolved":true,"context_lines":[{"line_number":1533,"context_line":"                    raise HTTPServerError(\u0027Unable to load SLO manifest\u0027)"},{"line_number":1534,"context_line":"            else:"},{"line_number":1535,"context_line":"                # Drain and close GET request (prevents socket leaks)"},{"line_number":1536,"context_line":"                drain_and_close(resp)"},{"line_number":1537,"context_line":"                raise HTTPBadRequest(\u0027Not an SLO manifest\u0027)"},{"line_number":1538,"context_line":"        elif resp.status_int \u003d\u003d HTTP_NOT_FOUND:"},{"line_number":1539,"context_line":"            raise HTTPNotFound(\u0027SLO manifest not found\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"41359710_357ea161","line":1536,"in_reply_to":"7d31a03d_1fa5aa12","updated":"2022-07-22 07:56:19.000000000","message":"First, thanks for the review.\n\nI try to simply use `close_if_possible(resp)` but that don\u0027t resolved the memory leak, that\u0027s why I use `drain_and_close(resp)` instead.\n\nI try your suggestion to use `update_ignore_range_header` and that works fine even with `close_if_possible` (no more memory and socket leak on my preprod env) BUT this patch broke all other SLO delete tests and after an lot of try I don\u0027t find a way to fix that :-/\n\nWIP new patch:\n```\n-        resp \u003d Request.blank(\u0027\u0027, new_env).get_response(self.app)\n+        # Get only one byte if object is not a SLO manifest\n+        manifest_req \u003d Request.blank(\u0027\u0027, new_env, range\u003d\u0027bytes\u003d0-1\u0027)\n+        update_ignore_range_header(manifest_req, \u0027X-Static-Large-Object\u0027)\n+        resp \u003d manifest_req.get_response(self.app)\n \n         if resp.is_success:\n             if config_true_value(resp.headers.get(\u0027X-Static-Large-Object\u0027)):\n@@ -1532,8 +1535,8 @@ class StaticLargeObject(object):\n                 except ValueError:\n                     raise HTTPServerError(\u0027Unable to load SLO manifest\u0027)\n             else:\n+                # Close GET request (prevents socket leaks)\n+                close_if_possible(resp)\n                 raise HTTPBadRequest(\u0027Not an SLO manifest\u0027)\n```\n\nI try to emulate `update_ignore_range_header` in `FakeSwift.__call__` without success.","commit_id":"a5c1444faa827dc2e3d85468d42622acea1d435e"}]}