HTTP monitors leak one idle connection per check
- Vorherrschende Sprache
- Go
- Sterne
- 1.2k
- Forks
- 69
- PR-Merge-Kennzahlen
- Keine gemergten PRs in 30 T.
Beschreibung
**Describe the bug**
HTTP monitors appear to leak one idle keep-alive connection on every execution.
With a 60-second monitor interval, the target accumulates roughly one additional `ESTAB` socket per minute until it reaches its file-descriptor limit. In my case, the monitored service hit its soft limit of 1,024 open files after about 17 hours. The process stayed up, but it could no longer accept connections or open files.
I reproduced this on Peekaping `0.0.46`. The same transport lifecycle also appears to be present on `main`.
**Database**
- sqlite
**To Reproduce**
1. Create an HTTP monitor for an HTTP/1.1 or HTTPS endpoint that supports persistent connections.
2. Set the monitor interval to 60 seconds.
3. Use `GET` and do not add a `Connection: close` request header.
4. On the target, count established connections from the Peekaping host after each check. For example:
```bash
ss -Htanp state established | grep '' | wc -l
```
5. Continue checking the count for several monitor intervals.
6. Observe that the target gains approximately one additional established connection after every execution.
**Expected behavior**
The number of idle connections should remain bounded. Peekaping should either reuse a bounded HTTP transport and connection pool across executions or close idle connections created for one-off executions.
**Screenshots**
N/A
**Desktop (please complete the following information):**
N/A
**Smartphone (please complete the following information):**
N/A
**Additional context**
Peekaping environment:
- Peekaping version: `0.0.46`
- Image: `0xfurai/peekaping-bundle-sqlite:0.0.46`
- Deployment: Docker Compose
- Monitor type: HTTP
- Monitor interval: 60 seconds
- Request method: GET
- Target: HTTPS service with HTTP keep-alive enabled
> Note: The content below was written by AI, but I reviewed and it checks out.
The production incident that exposed this had a pretty direct one-to-one relationship between monitor executions and retained connections:
- The target process had a soft FD limit of `1024`.
- It had exactly `1024` descriptors open when it failed.
- `1007` target-owned TCP connections came from the Peekaping host.
- All `1007` connections were in `ESTAB` state.
- The process began returning `Too many open files (os error 24)` and could no longer accept TCP connections.
- The failure happened after roughly 17 hours at a one-minute interval, which lines up with one retained connection per execution.
Peekaping had also accumulated 3,807 established sockets across its monitors.
The suspected source is `apps/server/internal/modules/healthcheck/executor/http.go`. `HTTPExecutor.Execute` creates a new transport for each execution:
```go
baseTransport := &http.Transport{}
```
It then creates a client using that transport:
```go
h.client = &http.Client{
Timeout: timeout,
CheckRedirect: checkRedirect,
Transport: tlsInterceptor,
}
```
The response body is read and closed, but I could not find a corresponding call to `CloseIdleConnections` or a shared bounded transport reused across executions. Because this is a custom zero-value `http.Transport`, it also does not inherit the idle timeout configured on `http.DefaultTransport`.
The same lifecycle appears to exist in the normal, NTLM, mTLS, and proxy transport paths.
As a workaround, adding this request header to each HTTP monitor stops the accumulation:
```json
{
"Connection": "close"
}
```
After applying that header to all HTTP monitors and restarting Peekaping once to flush the existing sockets:
- The affected target retained zero connections from Peekaping across eight consecutive samples.
- The target's descriptor count stayed between 18 and 22.
- Peekaping retained zero non-loopback sockets across repeated monitor intervals.
- All monitors continued to report healthy.
A likely fix would be to reuse a configured `http.Transport` with bounded idle connections and a non-zero `IdleConnTimeout`. If each execution needs to own a separate transport, it should call `CloseIdleConnections()` before returning. That cleanup should cover the normal, NTLM, mTLS, and proxy paths.
A regression test could execute the same monitor repeatedly against an `httptest.Server` and verify that the number of idle connections remains bounded after multiple successful checks.
Beitragsleitfaden
Für dieses Repository ist kein Beitragsleitfaden indexiert
Rechercherichtung
The issue is in `apps/server/internal/modules/healthcheck/executor/http.go`. Look at the `HTTPExecutor.Execute` method where a new `http.Transport` is created for each execution. The fix likely involves reusing a transport with `IdleConnTimeout` or calling `CloseIdleConnections()`. Start by reading the existing code and the Go `net/http` package documentation on connection management. A test can be written using `httptest.Server` to verify connections are bounded.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- go
- Bereich
- backend, networking
- Issue-Typ
- Bug
- Schwierigkeit
- 3/5
- Geschätzter Aufwand
- 1-2 Tage
- Aktivitätsstatus
- Ruhig
- Klarheit
- Klar beschrieben
- Anfängerfreundlichkeit
- 65/100