Patrick Steinhardt [off-list ref] writes:

I think the issue is rather simple: we're hitting timeouts in Apache.
[...]
This is because our keepalive mechanisms aren't helping [...]
Whether that's the same issue like we see in macOS sometimes is a
different question.

I think that is the trigger for issues we've been seeing. I spent
some time investigating the Apache side over the last week and maybe
found a mod_http2 bug, which I filed upstream with a potential fix:

  bug:  https://bz.apache.org/bugzilla/show_bug.cgi?id=70131
  fix:  https://github.com/mmontalbo/httpd/pull/2

To Patrick's earlier question of whether this is a Git, curl, or Apache
bug: as best I can tell it's Apache. I could reproduce it with no Git
involved at all (just Apache and a small CGI that goes quiet past the
Timeout), and across several curl versions (8.6.0, which is what the
GitHub runners use, up to 8.20.0), so I don't think bumping curl would
help. It also seems to wear two faces from the same trigger: over
HTTP/1.1 Apache closes the connection and curl bails with the
"transfer closed" error (which looks like what you hit with Timeout=1,
and the recent failures on both macOS and Linux), and over HTTP/2 it
does not reliably reset the stream, so the client just waits, which is
the six-hour macOS hang. I share the pessimism from earlier in the
thread, though: I think the real fix is upstream in Apache, and
anything we do on our side mostly just bounds the symptom in the
meantime.

Given there could be a potential reliability issue with an upstream
dependency like Apache, I was considering what mitigation strategies
might help:

  - Enforce some kind of lower bound speed limit and a client-side
    timeout so runs that wedge fail fast (and loudly) instead of
    hanging.

  - Potentially provide some affordance for retrying flaky tests
    that might fail due to upstream dependencies. Git already has
    some HTTP retry support (http.maxRetries and friends, added
    recently), but as far as I can tell it only triggers on HTTP 429
    rate limiting, so it would not catch a stall like this on its
    own. A test-level retry is not something I like that much, since
    it might encourage papering over flakiness that should be
    resolved, but it was a consideration vs requiring a fresh CI run
    to resolve the flake.

  - Make slow tests faster by optimizing the test itself and/or
    the test runner configuration (e.g., job number matching
    cores) so wedges become less likely.

For the first one, I think Git already provides some affordances. There
is a stall-based timeout that just ships disabled: as I understand it
http.lowSpeedLimit sets a bytes/sec floor and http.lowSpeedTime how long
a transfer can sit below it before curl gives up, so it would catch a
wedged connection without punishing one that is just slow. Enabling it
for the http tests might look something like:

    diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
    @@ GIT_TRACE=$GIT_TRACE; export GIT_TRACE
    +# Abort a transfer that makes essentially no progress for a while,
    +# so a wedged connection fails in seconds instead of hanging to the
    +# job cap. Tiny limit, generous window, so it only trips on a true
    +# stall; override either var, or set the limit to 0, to disable.
    +GIT_HTTP_LOW_SPEED_LIMIT=${GIT_HTTP_LOW_SPEED_LIMIT-1}
    +GIT_HTTP_LOW_SPEED_TIME=${GIT_HTTP_LOW_SPEED_TIME-60}
    +export GIT_HTTP_LOW_SPEED_LIMIT GIT_HTTP_LOW_SPEED_TIME

I went conservative on the values on purpose: a floor of 1 byte/sec
should only really fire on a true zero-progress stall, not on something
that is just crawling on a slow runner, and the 60s window is generous
for the same reason. When I tried it locally against a stall-proxy it
did turn an otherwise indefinite hang into a bounded abort (a tighter
limit/window brings that down to single-digit seconds). It probably does
not need to be suite-wide either; it could be scoped per-command with
git -c, which the http tests already lean on for this kind of thing
(t5551 passes http.postbuffer and http.extraheader that way), if a
narrower blast radius feels safer.

I only dug into the first option in any depth, since I wanted to
sanity-check the direction before writing patches. Does turning on a
stall timeout for the http tests seem reasonable? Are there other
strategies that we should implement?