Skip to content

Commit 202d674

Browse files
abd-ulbasitclaude
andcommitted
docs: state honestly that the L4 pool cannot register a hit
README advertised sluice_pool_hits_total 4821 for a pool that is structurally incapable of a hit on the L4 path: each copy direction half-closes its destination, and TestPooledConnHalfClosedIsNotPooled asserts a half-closed connection is never pooled, so every L4 request dials a fresh socket. Making the counter move requires not forwarding the client FIN. That was implemented and reverted (kept on attempted/tcp-pool-reuse) because it broke two things that matter more than reuse: a client that half-closes and waits received an empty response, and the abandoned socket went back into the pool still owing a reply, so the next client could be served the previous client's response body. Both reproduced. config.example.yaml shipped enabled: true while the README called pooling opt-in. It now ships disabled, with the reason inline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 75933b7 commit 202d674

2 files changed

Lines changed: 24 additions & 2 deletions

File tree

‎README.md‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,28 @@ misspelled optional key disables a feature with no error and no log line.
296296
sluice_backend_health{backend="10.0.0.5:8080"} 1
297297
sluice_circuit_breaker_state{backend="10.0.0.5:8080"} 0 # 0=closed 1=open 2=half-open
298298
sluice_rate_limiter_requests_total{result="rejected"} 0
299-
sluice_pool_hits_total{backend="10.0.0.5:8080"} 4821
299+
sluice_pool_hits_total{backend="10.0.0.5:8080"} 0
300+
sluice_pool_misses_total{backend="10.0.0.5:8080"} 0
300301
sluice_request_duration_seconds_bucket{le="0.05"} 19204
301302
```
302303

304+
### The L4 pool hit counter stays at zero, by construction
305+
306+
Not a misconfiguration. Each copy direction ends by half-closing its
307+
destination (`internal/proxy/tcp.go`, the `CloseWrite` after the copy loop),
308+
and a half-closed connection is never returned to the pool. That second half is
309+
asserted by `TestPooledConnHalfClosedIsNotPooled` in
310+
`internal/backend/backend_test.go`. Put together, every L4 request dials a
311+
fresh backend socket, so the hit counter cannot move on that path.
312+
313+
Making it move means not forwarding the client's FIN to the backend. That was
314+
tried and it is worse than an idle counter: a client that half-closes and then
315+
waits receives an empty response, and because the abandoned socket is returned
316+
to the pool with a reply still owed on it, the next client can be served the
317+
previous client's response body. Both were reproduced before the change was
318+
reverted. Connection reuse is not worth a protocol-visible correctness bug, so
319+
the pool remains useful on the L7 path and the L4 counter stays honest.
320+
303321
Admin API: `GET /health`, `GET /stats`, `GET /backends`.
304322

305323
## Architecture

‎config.example.yaml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,12 @@ server:
7979
# Reuse of idle TCP connections to backends, for the L4 path. Connections are
8080
# liveness-checked before reuse (zero read deadline + 1-byte read), so a
8181
# backend that closed an idle connection does not surface as a failed request.
82+
# Off by default. On the L4 path this can never register a hit: each copy
83+
# direction half-closes its destination and a half-closed connection is not
84+
# poolable, so enabling it only adds bookkeeping. See the observability
85+
# section of the README for why making it work is the wrong trade.
8286
tcp_pool:
83-
enabled: true
87+
enabled: false
8488
max_idle: 128
8589
idle_timeout: 30s
8690
max_lifetime: 2m

0 commit comments

Comments
 (0)