On 08/30/2012 06:11 AM, Henrik Nordström wrote: > ons 2012-08-29 klockan 15:32 -0600 skrev Alex Rousskov: > >> Today, we have a choice: >> >> A. Fail some PUTs. Close fewer pconns (current code). >> B. Handle all PUTs. Open more connections (patched code).
The FwdState::checkRetriable() method is used in the following two cases: 1) To decide if it can reuse a healthy persistent connection. Inside FwdState::connectStart, we are getting a persistant connection and even if it is healthy, if we want to send eg a PUT request we are closing the persistent connection and we are opening a new one. 2) To decide if a connection can be retried (inside FwdState::checkRetry()) >From what I can understand there is not any reason to not reuse a healthy persistent connection, for PUT requests. Am I correct? If yes then the problem is only in the case (2). In the case (2) we are using the "if (request->bodyNibbled())" to examine if any body data consumed and probably sent to the server. So we should not have any problem at all. However in the case the HttpRequest::bodyNibbled() is not enough we can add the check "if (request->body_pipe != NULL)" inside FwdState::checkRetry() method after calling checkRetriable. I think this is easier, at least for a short term fix, than delaying sending body... >> >> If this patch is accepted, performance bug 3398 will resurface. Henrik, >> do you think committing the patch is the right decision here even though >> it will reopen bug 3398? > > Yes, but with a plan to fix it correctly. > > A suggested correct fix for 3398 is to make use of Expect: 100-continue. > This delays sending the body a bit, allowing detection of broken > connections before starting to send the body. > > An optimization from that is to add some buffering of request bodies to > allow skipping the 100-continue to speed up forwarding. > > Regards > Henrik > >> The bug title is wrong. There is a long discussion in the bug report >> about what the bug is really about. I think a better bug title would be: >> "persistent server connection closed before PUT". > > yes. > > Fix the bug title, reopen it with comment above and restore the check. > > Regatds > Henrik > >