Remove the http1 connection pool delay.

[jplevyak]

Remove the http1 connection pool delay.

This feature is incomplete in that libevent doesn't guarantee ordering of events
so delaying for a single poll cycle does not address the issue.
Also because the connection is left in the ready queue it is
available for another request coming in the same pool cycle or
in the next poll cycle before ready connection is processed.
Finally, this would only catch connections closed immediately
which should be very infrequent since the HTTP/1.0 fix which
eliminated those cases where a missing Connection: keep-alive
was not interpreted as Connection: close, resulting in reuse
of a connection which was immedately closed.

See #7159 for details.

Signed-off-by: John Plevyak jplevyak@gmail.com

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description:
Risk Level: low
Testing: existing tests
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

[mattklein123]

I'm going to assign this over to @lizan since he did the previous change, but as we already discussed in the other PR, we cannot remove this code. This does catch a very specific case which is that Envoy is already read off EOF before dispatching data.

As I mentioned in the other PR, this change can be simplified by adding a halfClosedStatus() or similar API so we don't have to do a poll cycle.

[jplevyak]

So, this code as it stands doesn't actually do what it purports to do, so IMO it should be fixed (the previous PR does do what this code purports to do) or replaced (e.g. half-close semantics) or removed (this PR). Leaving it as is I don't believe is a reasonable option. IMO it is would be best to either resurrect the previous PR which fixes this or merge this PR and remove the non-working code and then decide on a replacement. Leaving non-working code in the codebase adds complexity without providing benefit.

[mattklein123]

The code does work for the case it was implemented to fix, and we already discussed this in the comment thread in the other PR. I will let @lizan follow up.

[jplevyak]

To be specific, you said "This does catch a very specific case which is that Envoy is already read off EOF before dispatching data." but as I pointed out, this is in fact not true. This was proven by testing in production where this code proved ineffective. The previous PR did in fact address the issue in production.

[jplevyak]

I can provide a test setup to demonstrate that this code is ineffective and that the previous PR is effective if you like.

[mattklein123]

I would really prefer to not go back and forth on this infinitely, but for posterity, here are the two situations:

1. Envoy reads full response + EOF. It then dispatches data, and prevents reuse of the connection so the connection can be closed when the stack unwinds.

2. Envoy reads a full response but not EOF. EOF comes some time later.

The code you are deleting in this PR fixes (1). We won't be removing it, but it can be simplified as I already mentioned. Your previous PR "fixes" (2) which is a race condition that cannot be fixed in any sane way other than by implementing a delay timer.