eiri commented on pull request #3712: URL: https://github.com/apache/couchdb/pull/3712#issuecomment-906411618
@iilyak They are not the same. Consider following (important parts in bold): With my patch: <pre> $ curl -q -s -v -u *:* http://127.0.0.1:15984/koi/doc/att?rev=2-a85de97b9bf75058156d7872a4bc3b89 -X DELETE -H 'Content-Type: application/json' -d '{"oh", "hi"}' --next -s -v http://127.0.0.1:15984 | jq '.' * Trying 127.0.0.1... * TCP_NODELAY set * Connected to 127.0.0.1 (127.0.0.1) port 15984 (#0) * Server auth using Basic with user *** > DELETE /koi/doc/att?rev=2-a85de97b9bf75058156d7872a4bc3b89 HTTP/1.1 > Host: 127.0.0.1:15984 > Authorization: *** > User-Agent: curl/7.64.1 > Accept: */* > Content-Type: application/json > Content-Length: 12 > } [12 bytes data] * upload completely sent off: 12 out of 12 bytes < HTTP/1.1 200 OK < Cache-Control: must-revalidate < Content-Length: 66 < Content-Type: application/json < Date: Thu, 26 Aug 2021 12:47:00 GMT < Server: CouchDB/3.1.1-dirty (Erlang OTP/22) < X-Couch-Request-ID: edbe14f280 < X-CouchDB-Body-Time: 0 < { [66 bytes data] <strong>* Connection #0 to host 127.0.0.1 left intact</strong> *{ Found bundle for host 127.0.0.1: 0x7fe3f03056a0 [can pipeline] * "ok": true, Could pipeline, but not asked to! "id": "doc", "rev": "3-bb43bd1ff8645e9544c8d0700b474337" } <strong> * Re-using existing connection! (#0) with host 127.0.0.1</strong> * Connected to 127.0.0.1 (127.0.0.1) port 15984 (#0) > GET / HTTP/1.1 > Host: 127.0.0.1:15984 > User-Agent: curl/7.64.1 > Accept: */* > < HTTP/1.1 200 OK ... </pre> With this patch: <pre> curl -q -s -v -u *:* http://127.0.0.1:15984/koi/doc/att?rev=2-a85de97b9bf75058156d7872a4bc3b89 -X DELETE -H 'Content-Type: application/json' -d '{"oh", "hi"}' --next -s -v http://127.0.0.1:15984 | jq '.' * Trying 127.0.0.1... * TCP_NODELAY set * Connected to 127.0.0.1 (127.0.0.1) port 15984 (#0) * Server auth using Basic with user *** > DELETE /koi/doc/att?rev=2-a85de97b9bf75058156d7872a4bc3b89 HTTP/1.1 > Host: 127.0.0.1:15984 > Authorization: Basic *** > User-Agent: curl/7.64.1 > Accept: */* > Content-Type: application/json > Content-Length: 12 > } [12 bytes data] * upload completely sent off: 12 out of 12 bytes < HTTP/1.1 200 OK < Cache-Control: must-revalidate < Content-Length: 66 < Content-Type: application/json < Date: Thu, 26 Aug 2021 12:50:02 GMT < Server: CouchDB/3.1.1-dirty (Erlang OTP/22) < X-Couch-Request-ID: c5a76b2209 < X-CouchDB-Body-Time: 0 < { [66 bytes data] <strong>* Connection #0 to host 127.0.0.1 left intact</strong> * Found bundle for host 127.0.0.1: 0x7fa78f0056a0 [can pipeline] * Could pipeline, but not asked to! { "ok": true, "id": "doc", "rev": "3-bb43bd1ff8645e9544c8d0700b474337" } * Re-using existing connection! (#0) with host 127.0.0.1 * Connected to 127.0.0.1 (127.0.0.1) port 15984 (#0) > GET / HTTP/1.1 > Host: 127.0.0.1:15984 > User-Agent: curl/7.64.1 > Accept: */* > <strong>* Recv failure: Connection reset by peer * Connection died, retrying a fresh connect * Closing connection 0 * Issue another request to this URL: 'http://127.0.0.1:15984/'</strong> * Hostname 127.0.0.1 was found in DNS cache * Trying 127.0.0.1... * TCP_NODELAY set * Connected to 127.0.0.1 (127.0.0.1) port 15984 (#1) > GET / HTTP/1.1 > Host: 127.0.0.1:15984 > User-Agent: curl/7.64.1 > Accept: */* > < HTTP/1.1 200 OK </pre> So this effectively kills keep-alive capability for all requests to Couch API. Which is kind of expected, because as I read it `mochiweb_request:cleanup/1` is a hack around mochiweb's proc dicts poor-man's state machine of request life-cycle that was added to _implement_ keep-alive in [here](https://github.com/apache/couchdb-mochiweb/blob/main/src/mochiweb_http.erl#L216) by erasing all flags and immediately restarting req loop. So if we clean the flags outside of req lifecycle it's not surprising for it to break. This is why I rather wary about extending scope for this PR to _all_ http requests handling, we can subtly degrade our http stack and I don't trust our CI to catch it all. Another thing to consider that just dropping dict flags without draining payload leaving garbage in socket buffer till the socket closed, i.e. leaking memory a bit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
