[PR #114] fix(proxy): probe upstream liveness with TCP keepalive #190

New issue

Open

opened 2026-10-29 14:20:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:20:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/114
Author: @cleverhu
Created: 8/3/2026
Status: 🔄 Open

Base: main ← Head: fix/proxy-tcp-keepalive

📝 Commits (2)

5b7d1dd perf(net): disable Nagle on the API server, proxy and envd clients
7d784f2 fix(proxy): probe upstream liveness with TCP keepalive

📊 Changes

3 files changed (+32 additions, -3 deletions)

View changed files

📝 src/api/proxy.rs (+13 -0)
📝 src/bin/server.rs (+9 -1)
📝 thirdparty/envd/src/transport.rs (+10 -2)

📄 Description

What

Enable TCP keepalive probes on the sandbox proxy's connector: 3 s idle before the first probe, 1 s between probes, 2 retries.

Depends on #109. This branch is stacked on it because both touch build_proxy_client(); the diff shown here against main therefore also contains #109's commit. Please merge #109 first — this then rebases to a single-commit, three-line change.

Why

Tearing down a VM never delivers FIN or RST to the host side of a proxied connection. The socket stays open from the host's point of view, so a request addressed to a sandbox that is already gone hangs until some higher-level timeout fires instead of failing immediately.

Keepalive probes are what detect that silence. With the values above the failure surfaces in roughly 5 s, which is a prompt error for a caller and still far above any plausible pause in normal proxied traffic.

None. Small, self-contained reliability fix on an existing code path.

Scope and non-goals

Included: keepalive parameters on the proxy connector.

Excluded:

Retry or failover behavior when a probe does detect a dead peer — the request fails, as it should.
Connection pooling. pool_max_idle_per_host(0) already handles the separate problem of interaction IPs being reused across runtime generations.
Any other connector in the tree.

Design and behavior changes

Three constants next to the existing PROXY_CONNECT_TIMEOUT, applied via HttpConnector::set_keepalive{,_interval,_retries}.

Failure mode change: a request to a torn-down sandbox now returns a connection error after ~5 s instead of hanging. Callers that previously relied on a longer timeout to mask this will see the error sooner, which is the intent.

Compatibility and operations

Public API or generated protocol: N/A.
Configuration or defaults: no new config keys. The values are compile-time constants; if maintainers want them tunable I can move them into [orchestrator] or a proxy section.
Snapshot manifest, artifact layout, or storage format: N/A.
Upgrade and rollback: transparent both ways.
Host requirements, permissions, ports, or dependencies: TCP_KEEPIDLE/TCP_KEEPINTVL/TCP_KEEPCNT are Linux socket options and AgentENV is Linux-only.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

$ cargo fmt --all -- --check
(clean)

$ cargo clippy --workspace --all-targets -- -D warnings
Finished `dev` profile

$ cargo check -p agentenv --all-targets
Finished `dev` profile

Skipped checks and reasons:

make test-unit not run. Observing this behavior in a test requires a peer that vanishes without sending FIN/RST, which is not reproducible with an in-process test server; it needs a real VM teardown or an injected netfilter drop.
No benchmark: this is a failure-mode fix, not a throughput or latency change on the healthy path.

Risks and reviewer notes

The risk worth reviewing is the aggressiveness of the values. A 3 s idle threshold with 1 s probes means an idle-but-alive sandbox connection sends a probe every second. That is negligible traffic on a host-local link, but if the proxy is ever used across a slower network the numbers deserve revisiting.

The other question is whether ~5 s to detect is the right target. It is well under typical client timeouts while leaving room for a transient stall.

Most important file: src/api/proxy.rs.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/114 **Author:** [@cleverhu](https://github.com/cleverhu) **Created:** 8/3/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/proxy-tcp-keepalive` --- ### 📝 Commits (2) - [`5b7d1dd`](https://github.com/kvcache-ai/AgentENV/commit/5b7d1dd48f9e7b1c03dc435fefefaa1c7486bbc3) perf(net): disable Nagle on the API server, proxy and envd clients - [`7d784f2`](https://github.com/kvcache-ai/AgentENV/commit/7d784f28e8e0bfc267bde286b80670c1098c4d85) fix(proxy): probe upstream liveness with TCP keepalive ### 📊 Changes **3 files changed** (+32 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/api/proxy.rs` (+13 -0) 📝 `src/bin/server.rs` (+9 -1) 📝 `thirdparty/envd/src/transport.rs` (+10 -2) </details> ### 📄 Description ## What Enable TCP keepalive probes on the sandbox proxy's connector: 3 s idle before the first probe, 1 s between probes, 2 retries. > **Depends on #109.** This branch is stacked on it because both touch `build_proxy_client()`; the diff shown here against `main` therefore also contains #109's commit. Please merge #109 first — this then rebases to a single-commit, three-line change. ## Why Tearing down a VM never delivers FIN or RST to the host side of a proxied connection. The socket stays open from the host's point of view, so a request addressed to a sandbox that is already gone hangs until some higher-level timeout fires instead of failing immediately. Keepalive probes are what detect that silence. With the values above the failure surfaces in roughly 5 s, which is a prompt error for a caller and still far above any plausible pause in normal proxied traffic. ## Related issue None. Small, self-contained reliability fix on an existing code path. ## Scope and non-goals Included: keepalive parameters on the proxy connector. Excluded: - Retry or failover behavior when a probe does detect a dead peer — the request fails, as it should. - Connection pooling. `pool_max_idle_per_host(0)` already handles the separate problem of interaction IPs being reused across runtime generations. - Any other connector in the tree. ## Design and behavior changes Three constants next to the existing `PROXY_CONNECT_TIMEOUT`, applied via `HttpConnector::set_keepalive{,_interval,_retries}`. Failure mode change: a request to a torn-down sandbox now returns a connection error after ~5 s instead of hanging. Callers that previously relied on a longer timeout to mask this will see the error sooner, which is the intent. ## Compatibility and operations - Public API or generated protocol: N/A. - Configuration or defaults: no new config keys. The values are compile-time constants; if maintainers want them tunable I can move them into `[orchestrator]` or a proxy section. - Snapshot manifest, artifact layout, or storage format: N/A. - Upgrade and rollback: transparent both ways. - Host requirements, permissions, ports, or dependencies: `TCP_KEEPIDLE`/`TCP_KEEPINTVL`/`TCP_KEEPCNT` are Linux socket options and AgentENV is Linux-only. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [ ] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [ ] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text $ cargo fmt --all -- --check (clean) $ cargo clippy --workspace --all-targets -- -D warnings Finished `dev` profile $ cargo check -p agentenv --all-targets Finished `dev` profile ``` Skipped checks and reasons: - `make test-unit` not run. Observing this behavior in a test requires a peer that vanishes without sending FIN/RST, which is not reproducible with an in-process test server; it needs a real VM teardown or an injected netfilter drop. - No benchmark: this is a failure-mode fix, not a throughput or latency change on the healthy path. ## Risks and reviewer notes The risk worth reviewing is the aggressiveness of the values. A 3 s idle threshold with 1 s probes means an idle-but-alive sandbox connection sends a probe every second. That is negligible traffic on a host-local link, but if the proxy is ever used across a slower network the numbers deserve revisiting. The other question is whether ~5 s to detect is the right target. It is well under typical client timeouts while leaving room for a transient stall. Most important file: `src/api/proxy.rs`. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-29 14:20:49 +00:00

bsakaguchi referenced this issue

2026-10-29 14:21:22 +00:00

[PR #190] [MERGED] feat: enable dirty-page tracking by default #240