Fix HTTP connection reuse in engine health probes - #37
Noah-Tervalon-Nvidia merged 2 commits into
Conversation
|
Thank you for bringing this issue up. We're going to have a look and validate that everything looks good on it. |
|
I think this is worth fixing, although I noticed on golang 1.27, the tests w/o the fix only had 2 or 3 open connections. This seems to be due to I'll send a commit to the branch to make the change be a bit more succinct and align with internal standards. |
|
Just to touch base here, we are working on getting our final batch of internal changes out into the public repo. Once those are in, we'll get this rebased (if necessary) and landed. Thanks again for your contribution! |
Yay! I was struggling with this when I installed for my personal AI bot brain Mac Mini😁 |
The base branch was changed.
|
We'll need to rebase this onto develop (which has had some major changes lately). I can try to do that this morning. |
Drain bounded HTTP response bodies before closing health checks so periodic polling can reuse connections. Preserve probe deadlines and status-based results, and cover HTTP/1 reuse, size limits, and timeouts. Signed-off-by: Psych0h3ad <41975091+Psych0h3ad@users.noreply.github.com>
Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
8eb7ba8 to
6ee0b40
Compare
3 participants
Description
Periodic engine health checks close HTTP response bodies without reading them. With Go 1.26.7, this prevents the HTTP/1 transport from reusing those connections and opens a new connection for every poll. Draining ordinary responses before closing reduces unnecessary connection churn while PAIR monitors Ollama and LM Studio.
The regression tests reproduce 32 connections for 32 polls before the fix and one connection for the same 32 polls after it, for both Content-Length and chunked responses across all three health-check paths.
Release intent
Changelog title
Fix HTTP connection reuse in engine health probes
Changelog body
Periodic engine monitoring now reuses HTTP/1 connections instead of opening a new connection for every health probe.
Bumps
Scope
nvpair-ui-brokerfrom 0.40.2 to 0.40.3 andnvpair-engine-managerfrom 0.17.4 to 0.17.5.No API, configuration, dependency, or desktop changes are required. This change addresses connection churn; it does not reset existing TCP state or establish a cause for any operating-system TCP reclamation failure.
Validation
After rebasing onto
develop, the following passed on Windows amd64 with Go 1.27.0:go test ./... -count=1fromservices/sharedgo test ./... -count=1fromservices/nvpair-engine-managergo test ./... -count=1fromservices/nvpair-ui-brokernode scripts/spdx-headers.mjs— 1,023 files checked, with no missing, review-required, or unclassified headersgit diff --check upstream/develop..HEADThe original targeted validation also passed on macOS arm64 with Go 1.26.7 and
CGO_ENABLED=0.No live engine operation was required because the regression tests exercise the real
net/httpHTTP/1 client and server overnet.Pipewithout consuming TCP source ports.Risk
Health results continue to depend on the HTTP status, including when a body read fails. A stalled response can now keep the check active until the existing timeout or deadline, and an oversized or incomplete body can still cause the connection to be discarded. The drain is bounded in size and time and discards content without logging it. No data migration or wire-format change is involved.
Checklist
git commit -s), certifying the Developer Certificate of Origin.services/versions.jsonis written by automation — do not edit it by hand.