)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49a7be5aaf3e3750178922322d5e96633f150aa4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7974e6d7_6a095b4f","updated":"2023-02-10 15:34:23.000000000","message":"FWIW the only one of these that seemed to help us drop idle sockets faster was wsgi_keepalive \u003d False (which effectively disables http 1.1 connection: keep-alive and pipeline requests) so every socket is closed after a single response.","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0a616373e09ab152142490277b54a0ed8a6e972e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2d14793e_6d4f2d92","updated":"2023-02-13 01:33:14.000000000","message":"I do like plumbing these through, just some thoughts inline. We\u0027re missing the keepalive interval option, which defaults (in linux) to 75 seconds, which we\u0027d also want configurable.\n\nI also tested this on a known recreated issue, which deals with stale/idle connections from a bad client. By it self the job takes 15 minutes, but:\n\nclient_timeout \u003d 1.0\n\nreal    0m26.439s\nuser    0m1.088s\nsys     0m0.517s\n\n\nwsgi_keepalive \u003d false\n\nreal    0m10.810s\nuser    0m1.084s\nsys     0m0.626s\n\nSo I think it\u0027s nice to have these options.","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bd2f7f698cfe4ff4adeafdabbf51db8ad7cec4dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"31548162_ef521f94","updated":"2023-02-12 22:13:59.000000000","message":"It\u0027s nice having these exposed. Spending the end of last week debugging and tracking down issues that could have been \"resolved\" or at least used an option here as a work around just make it more flexible!\n\nI was planning on whipping something like this up today, but nice to see it already done as I come in on Monday morning, thanks Clay!\n\nI\u0027m going to give this a whirl in my test env where I was experiencing an issue that this would solve, if all goes well will give it my +2.","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e8abd141e1ce9da2b758328675a6a3700e7f165a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fcfe301a_d589dbd2","updated":"2023-02-13 16:39:00.000000000","message":"NP, I can respin.\n\nYou noted that it would be useful (with the problem we\u0027re having in prod, right now) if ops had the option to configure wsgi_keepalive\u003dFalse (which currently, ops can not).  I think I saw you also concluded that there may be value in configuring the tcp keepalive settings (which currently, ops can not) - do I have that right?\n\nMoreover I think you\u0027re saying this patch is DEFICIENT (-1).  i.e. it is not complete, it doesn\u0027t make swift *only* better.  And I think the reason you gave is because it\u0027s missing 1/3 values needed to FULLY configure tcp keepalive (which currently swift does not support, but half-assed support is worse than none).  So you\u0027d like me to EXPAND the patch?  I want to confirm, you think the BEST way to change THIS patch would be to add a NEW configuration (N.B. this patch makes 3 hard-coded settings configurable already).\n\nWhat would you think about pulling out the tcp keepalive configuration from this patch - just leave them hard-coded - maybe tcp keepalive is unrelated/useless?  Maybe it\u0027d be easier for us to agree on a diff that is complete and makes swift better if the change only did *ONE* thing: add support to configure wsgi_keepalive\u003dFalse\n\nThen if we have a reason to support tcp keepalive configuration in the future we could add that in too!  What do you think?","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2c925457b19201dea34234bf49dbb6b7180958bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"88b4b576_779395b8","updated":"2023-02-10 15:40:25.000000000","message":"i think working on the eventlet/wsgi guts to identify idle sockets between requests, so that we can limit the # of idle connections, how long we keep an idle connection between requests, and what to do with idle connections when the accept queue has a new connection but the handler pool is full .. . would all be useful things to pursue as well.","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da74ac74909f1f1aefbfde56feb2b427df249886","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3472597c_49ad2156","in_reply_to":"b395633b_31a2b713","updated":"2023-02-15 15:39:32.000000000","message":"Yeah I\u0027m trying to be more honest - I appreciate your feedback.\n\nThanks for checking my thinking before I spun another patch - let me know if you think we need to make this new slimmer version better still, or if perhaps it\u0027s just not useful to give ops this knob?\n\nIf we want to work on tcp keepalive tuning in the future we can pick up https://review.opendev.org/c/openstack/swift/+/873942","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"45fdf131b5cd4d153486184253b68b5bf07da3d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b395633b_31a2b713","in_reply_to":"fcfe301a_d589dbd2","updated":"2023-02-13 21:43:22.000000000","message":"OK, that is an interesting response :)\n\nYeah, so the patch as it stands is making swift better, but also allowing ops to control the tcp keep alive settings, both I think are great, except the tcp keep alive settings are only 2/3 complete, so the patch makes swift better, but also incomplete, as it can\u0027t really configure tcp keep alive (thus the -1).\n\nIf you want want to split into 2, cool, or yes add a new keepalive_intvl config option so we can fully configure the proxies keepalive settings.\n\nMaybe we\u0027re not using the keepalive features too much today, but I do wonder if they might be useful, so I for one wouldn\u0027t mind playing around with them. If we have them easily configurable that would make it easier.\n\nTL;DR: respin as either 2 patches (and we\u0027ll land the first now) or as one and add an interval. I don\u0027t mind which. Until there is in a release for us downstream we\u0027re not going to make use the to wsgi_keepalive\u003dfalse. So either way.\n\n\u003ckeepalive tangent\u003e\nThe keepalive defaults might not be super useful for us, but if we tune them down, we could have some extra connection monitoring on our connections that could disconnect before we hard fail on a client_idle (which ends up basically being a blocking timeout). So we can have a hardline idle timeout of a connection AND potentially have keep alive do a ligher check for connectivity probe. And at the code of an empty packet or a few getting more socket connection garbage collection, might be worth it.\n\u003c/tangent\u003e","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"}],"etc/proxy-server.conf-sample":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2c925457b19201dea34234bf49dbb6b7180958bc","unresolved":true,"context_lines":[{"line_number":84,"context_line":"# CORS documentation)."},{"line_number":85,"context_line":"# cors_expose_headers \u003d"},{"line_number":86,"context_line":"#"},{"line_number":87,"context_line":"# client_timeout \u003d 60.0"},{"line_number":88,"context_line":"# wsgi_keepalive \u003d true  # false means handle only ONE request per connection"},{"line_number":89,"context_line":"# tcp_keepalive \u003d true  # set SO_KEEPALIVE"},{"line_number":90,"context_line":"# keep_idle \u003d 600 # set TCP_KEEPIDLE"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"ac75f4a5_8969f8b7","line":87,"updated":"2023-02-10 15:40:25.000000000","message":"turning this down to 1.0 also has the effect of kicking idle connections off the eventlet.wsgi pool - but effects all operations on the socket in general, not just between requests.","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":22042,"name":"Chris Smart","email":"distroguy@gmail.com","username":"csmart"},"change_message_id":"2802251fdaa9afbba4ed187eb1702c36b264f6f6","unresolved":true,"context_lines":[{"line_number":87,"context_line":"# client_timeout \u003d 60.0"},{"line_number":88,"context_line":"# wsgi_keepalive \u003d true  # false means handle only ONE request per connection"},{"line_number":89,"context_line":"# tcp_keepalive \u003d true  # set SO_KEEPALIVE"},{"line_number":90,"context_line":"# keep_idle \u003d 600 # set TCP_KEEPIDLE"},{"line_number":91,"context_line":"# keep_cnt \u003d 8  # set TCP_KEEPCNT"},{"line_number":92,"context_line":"#"},{"line_number":93,"context_line":"# Note: enabling evenlet_debug might reveal sensitive information, for example"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"21cac48a_6c7b4af4","line":90,"updated":"2023-02-13 01:03:16.000000000","message":"Should we add a little bit extra in the comment to say something like, \"max time in seconds before requiring a keepalive packet\" or do we just expect people will lookup what TCP_KEEPIDLE means?","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bd2f7f698cfe4ff4adeafdabbf51db8ad7cec4dc","unresolved":true,"context_lines":[{"line_number":88,"context_line":"# wsgi_keepalive \u003d true  # false means handle only ONE request per connection"},{"line_number":89,"context_line":"# tcp_keepalive \u003d true  # set SO_KEEPALIVE"},{"line_number":90,"context_line":"# keep_idle \u003d 600 # set TCP_KEEPIDLE"},{"line_number":91,"context_line":"# keep_cnt \u003d 8  # set TCP_KEEPCNT"},{"line_number":92,"context_line":"#"},{"line_number":93,"context_line":"# Note: enabling evenlet_debug might reveal sensitive information, for example"},{"line_number":94,"context_line":"# signatures for temp urls"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"c4aa974a_3a006f75","line":91,"updated":"2023-02-12 22:13:59.000000000","message":"setting max_http_version to \u0027HTTP/1.0\u0027 also solved the problem in the wsgi server object, basically does the same thing as keepalive \u003d false. So keepalive \u003d false is better. But changing it to max_http_version to 1.0 is the first thing I did last week in my test enviornment which then \"solved\" the issue (or rather was going to be a possible workaround) until the eventlet patch merged. But yeah, then I saw the keepalive option, which did the same thing, basically.","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0a616373e09ab152142490277b54a0ed8a6e972e","unresolved":true,"context_lines":[{"line_number":88,"context_line":"# wsgi_keepalive \u003d true  # false means handle only ONE request per connection"},{"line_number":89,"context_line":"# tcp_keepalive \u003d true  # set SO_KEEPALIVE"},{"line_number":90,"context_line":"# keep_idle \u003d 600 # set TCP_KEEPIDLE"},{"line_number":91,"context_line":"# keep_cnt \u003d 8  # set TCP_KEEPCNT"},{"line_number":92,"context_line":"#"},{"line_number":93,"context_line":"# Note: enabling evenlet_debug might reveal sensitive information, for example"},{"line_number":94,"context_line":"# signatures for temp urls"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"bf1dc3c3_00941bcf","line":91,"in_reply_to":"c4aa974a_3a006f75","updated":"2023-02-13 01:33:14.000000000","message":"If we\u0027re going to support keep alive configuration, we\u0027re missing a major option.\n\nKeepalive has 3 options:\n\n  - socket.TCP_KEEPIDLE (send first keep alive after being idle this long, we have this one)\n  - socket.TCP_KEEPINTVL (after the first keep alive is fired how ofter to keep sending them, we don\u0027t have this one)\n  - socket.TCP_KEEPCNT (how many keep alives need to fail before we drop the connection, we have this one too).\n \nSo we\u0027re missing the keepalive interval (keepintvl). The default under linux for the interval is 75, so we may want to change that.\n\nThe client_timeout seems to set the established socket timeout. So a timeout on blocking connections. So not sure a keepalive probe response would be considered data, so not sure client_timeout and tcp_keepalive will work well together, unless the keepalives are tiny.\n\nI wonder if we\u0027d rely on keep alives more, say client_timeout \u003d 0 (turn off) and keep_idle \u003d 10, keep_interval \u003d 10, keep_cnt \u003d 6. Would do something similar. But I it wouldn\u0027t do a hard drop.\n\nMaybe client_timeout \u003d 60, keep_idle \u003d 2, keep_interval \u003d 2, keep_cnt \u003d 5\n\nwhich would give us a hard timeout, but also detect a stale connection faster.\n\nMight have to have a play with a bunch of settings.","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"},{"author":{"_account_id":22042,"name":"Chris Smart","email":"distroguy@gmail.com","username":"csmart"},"change_message_id":"2802251fdaa9afbba4ed187eb1702c36b264f6f6","unresolved":true,"context_lines":[{"line_number":88,"context_line":"# wsgi_keepalive \u003d true  # false means handle only ONE request per connection"},{"line_number":89,"context_line":"# tcp_keepalive \u003d true  # set SO_KEEPALIVE"},{"line_number":90,"context_line":"# keep_idle \u003d 600 # set TCP_KEEPIDLE"},{"line_number":91,"context_line":"# keep_cnt \u003d 8  # set TCP_KEEPCNT"},{"line_number":92,"context_line":"#"},{"line_number":93,"context_line":"# Note: enabling evenlet_debug might reveal sensitive information, for example"},{"line_number":94,"context_line":"# signatures for temp urls"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"455b8a42_a10378b0","line":91,"in_reply_to":"c4aa974a_3a006f75","updated":"2023-02-13 01:03:16.000000000","message":"Should we also a bit more in the comment here saying something like, \"number of unacked keepalive probes before dropping the connection\" ?","commit_id":"744d9636adc6061e729cace6a8bb870a25b86d65"}],"swift/common/wsgi.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8f6066df88e515778a97f16e8938c51f6685b0a8","unresolved":true,"context_lines":[{"line_number":425,"context_line":"        \u0027custom_pool\u0027: pool,"},{"line_number":426,"context_line":"        \u0027protocol\u0027: protocol_class,"},{"line_number":427,"context_line":"        \u0027socket_timeout\u0027: float(conf.get(\u0027client_timeout\u0027) or 60),"},{"line_number":428,"context_line":"        \u0027keepalive\u0027: config_true_value(conf.get(\u0027wsgi_keepalive\u0027, \u0027true\u0027)),"},{"line_number":429,"context_line":"        # Disable capitalizing headers in Eventlet. This is necessary for"},{"line_number":430,"context_line":"        # the AWS SDK to work with s3api middleware (it needs an \"ETag\""},{"line_number":431,"context_line":"        # header; \"Etag\" just won\u0027t do)."}],"source_content_type":"text/x-python","patch_set":2,"id":"c066c8ad_eb170963","line":428,"updated":"2023-02-15 17:21:45.000000000","message":"I don\u0027t love it -- keepalive True/False seems like such a blunt instrument -- I think we can use the truthiness test in existing eventlet to smuggle in another timeout: https://review.opendev.org/c/openstack/swift/+/873744","commit_id":"5d67ec27ec05c68818e27002691001fe9fff6ecf"}]}