Skip to content

fix: keep health and metrics responsive with idle connections - #3637

Closed
burak33bb wants to merge 1 commit into
PrimeIntellect-ai:mainfrom
burak33bb:fix/health-idle-connections
Closed

burak33bb wants to merge 1 commit into
PrimeIntellect-ai:mainfrom
burak33bb:fix/health-idle-connections

Conversation

@burak33bb

Copy link
Copy Markdown

An accepted TCP connection that sends no HTTP request blocks HTTPServer in its request handler. While that connection remains open, both HealthServer and MetricsServer stop answering other clients, including Kubernetes liveness probes; stop() also waits for that handler.

Use the standard-library ThreadingHTTPServer in both startup paths so each connection has a daemon handler thread. Extend the existing start/stop tests to keep an idle connection open while fetching the endpoint and stopping the server. There are no new dependencies or configuration fields.

Validation on Python 3.12 (CPU, lightweight uv environment):

  • tests/unit/utils/test_metrics_server.py: 14 passed.
  • The same tests against the original implementation: 2 failed with TimeoutError, 12 passed.
  • Ruff check, Ruff format check, and git diff --check passed.

Full GPU training tests were not run; this change is confined to the HTTP observability servers.

@burak33bb
burak33bb marked this pull request as ready for review September 23, 2026 12:15
@burak33bb

Copy link
Copy Markdown
Author

Could a maintainer take a look when convenient? This keeps an idle HTTP connection from blocking health probes and metrics requests. The two strengthened tests time out on the original implementation; all 14 metrics-server tests pass with the fix in the local CPU environment.

I don't see a CI run on the PR. If running CI requires maintainer approval, could you approve it? Happy to address any feedback. Thanks!

@burak33bb

Copy link
Copy Markdown
Author

Updating against current main revealed that this fix is now superseded by #3767 (f42630873d27199bc2fef5f28267806a2b37f96b), which removed the Kubernetes deployment path, HealthServer/MetricsServer, their configuration and trainer integration, and tests/unit/utils/test_metrics_server.py.

The two merge conflicts are modify/delete conflicts in exactly the implementation and test files this PR changes. Restoring them would reintroduce code that upstream intentionally removed. There is no remaining implementation in current main to apply this idle-connection fix to, so I am closing this PR as no longer applicable. The original fix remains available on the branch if it is needed for a maintained older release.

@burak33bb burak33bb closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant