brbzull0 opened a new pull request, #13897:
URL: https://github.com/apache/trafficserver/pull/13897
A `ConnectingEntry` whose origin connect fails (a connect error, or an
inactivity or active timeout during the handshake) notifies the `HttpSM`s
queued on it first, and gives back its `per_server.connection.max` reservation
only in its destructor, after that loop. With
`proxy.config.http.connect_attempts_retry_backoff_base` at its default of 0, a
queued `HttpSM` retries inside the notification (`HandleResponse`, then
`set_next_state()` calls `do_http_server_open()`), reserves again, and still
counts the failed connection. When the failed connection held the last free
slot, the retry is throttled and the client gets `503 Origin server congested`.
`HttpSM::origin_multiplexed()` is true when `dns_info.http_version` is
`HTTP_INVALID`, and that field is set only on the `use_client_target_addr`
path, so HTTP/1.1 origins take this path too.
The reservation moved into the entry in a19054e0a6 (#13545), the same
`ConnectingEntry` change as #13606 for #13605. Before it, the creating `HttpSM`
released the slot in its own failure branch before it retried. The open 10.2.x
backport #13889 carries that change too.
### What this PR does
- Adds `ConnectingEntry::release_outbound_connection_tracking()`, which
returns the reservation and drops the group, like
`PoolableSession::release_outbound_connection_tracking()`.
- The failure branch calls it once the entry has left the pool and its
socket is closed, before any queued `HttpSM` runs. The destructor calls it for
an entry deleted while its connect is still pending.
- Adds `src/proxy/http/unit_tests/test_ConnectingEntry.cc`, with three cases:
- "Failed connect returns its reservation before queued HttpSMs retry"
covers the four failure events, with and without another live connection to the
origin. A probe `HttpSM` records whether the failed entry is still pooled and
what count its own reservation gets. On master it fails with `2 == 1` and `3 ==
2`.
- "Failed connect with no tracking group notifies queued HttpSMs" covers
the default configuration, where the entry has no group.
- "Deleting a pending ConnectingEntry returns its reservation" covers the
destructor path that `HttpSM::cancel_pending_server_connection()` takes.
### What this PR does not do
- The probe stands in for the retry; it does not run `HttpSM`, which needs
`HttpConfig` started. That the retry happens inside the notification comes from
reading `HttpSM`, as traced above.
- The multiplexed success branch has the same order when no queued `HttpSM`
passes `validate_server_certificate_hostname()`: each is sent
`CONNECT_EVENT_DIRECT` and reserves again while `new_session` still holds the
slot, until the session is closed after the loop. That goes to a separate issue.
- No end-to-end test.
#13896 also adds `test_ConnectingEntry.cc` and edits `~ConnectingEntry()`.
Whichever of the two lands second will be rebased onto the other.
--
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]
