# Client Keepalive & Idle Timeout Rework

**Date:** 2026-03-29
**Status:** Draft

## Problem

The server's `idle_timeout_secs` (default 300s) closes connections after a period of inactivity regardless of subscription state. This causes two problems:

1. **False disconnects** — A subscribed consumer waiting for messages on a quiet topic gets killed even though it's healthy and intentionally idle.
2. **Slow crash detection** — When a consumer crashes (TCP goes silent without FIN), the server doesn't detect it until the idle timeout fires. With the 300s default, inflight messages sit undelivered for up to 5 minutes before the consumer group can rebalance.

The root cause: the idle timeout conflates "no TCP activity" with "dead connection." A healthy subscriber waiting for messages looks identical to a crashed client.

## Approach

Client-initiated keepalive pings with a tightened server-side idle timeout. The client is responsible for proving it's alive by sending periodic `Ping` frames. The server enforces a deadline — if no frames arrive within `idle_timeout_secs`, the connection is dead.

This is the same model used by MQTT, NATS, and WebSocket. The server stays simple (enforce a deadline), the client does the work (send pings), and the protocol already supports it (Ping/Pong frames exist at 0x0C/0x0D).

**Why not server-initiated pings:** Puts per-connection timer + outbound I/O burden on the server. Scales worse (server manages N timers vs. N clients each managing one). Client-initiated is the industry standard for connection liveness.

## Design

### 1. Config Changes

**`idle_timeout_secs`** — lower the default from 300 to 30. This is the enforcement deadline. If no inbound frames (including Pings) arrive within this window, the server closes the connection. Tier 1 (hot-reloadable), minimum 10 when non-zero, 0 disables.

No new server config fields. The existing `idle_timeout_secs` already does what we need — we're just lowering the default and fixing the client side.

**Example config:**

```toml
[tcp]
# Idle timeout in seconds. Clients must send Ping frames within this window
# to keep the connection alive. 0 = disabled. Default: 30.
idle_timeout_secs = 30
```

**Pre-auth connections:** The idle timeout applies from the moment a TCP connection is accepted, before authentication. Lowering the default from 300s to 30s means clients have 30s to complete the Connect handshake (including TLS negotiation). This is generous for programmatic clients. If a deployment has slow-authenticating clients (e.g., interactive tooling), they can set a higher `idle_timeout_secs` in config.

**Backwards compatibility:** Deployments with an explicit `idle_timeout_secs` in their config are unaffected. Only those relying on the default (300) will see the change — connections that go silent for 30s+ will now be closed. This is safe because well-behaved clients will send keepalive Pings, and the protocol already specifies this requirement.

**Breaking change for third-party SDKs:** Any SDK implementation following the current `ping_interval_ms: 30000` recommendation against the new 30s server timeout will experience sporadic disconnects — a 30s ping interval hitting a 30s deadline leaves zero margin for jitter. SDK authors must update to the new recommended `ping_interval_ms: 10000` (or lower). The general rule: `ping_interval` should be at most `idle_timeout / 3` to provide adequate margin.

### 2. FafbClient Auto-Ping Keepalive

Add a background keepalive task to `FafbClient` that:

1. Sends a `Ping` frame every `ping_interval` (default 10s) when the connection is idle
2. Resets the interval timer whenever the client sends any other frame (Publish, Ack, BatchAck, Subscribe, etc.) — no redundant Pings on busy connections
3. Is transparent to callers — no API changes to CLI commands or SDK consumers

**Additionally**, `FafbClient` must auto-reply `Pong` when it receives a server-initiated `Ping`. Currently `recv_frame()` returns whatever frame it gets, and no caller handles `Ping`. The fix: `recv_frame()` intercepts `Ping`, sends `Pong`, and continues reading the next frame. This is transparent to all callers.

**Default intervals:**
- `ping_interval`: 10s — well under the 30s idle timeout, giving margin for network jitter and scheduling delays
- The client SDK spec already recommends `ping_interval_ms: 30000` and `ping_timeout_ms: 10000`. Update the recommended default for `ping_interval_ms` to 10000 to align with the new 30s idle timeout. The `ping_timeout_ms` setting is for client-side server-liveness detection (unrelated to this change) and stays at 10000.

**Keepalive architecture in FafbClient:**

The current `FafbClient` wraps a `Framed<ClientStream, MessageCodec>` and exposes `send_frame()` / `recv_frame()`. There is no background task — it's a synchronous send/receive API driven by the caller.

Adding a background Ping task requires splitting the framed stream into read/write halves and spawning a tokio task. The design:

- On `connect()`, after the Connect/ConnectAck handshake, spawn a background task that owns a `Ping` sender
- The task sleeps for `ping_interval`, sends `Ping`, resets the timer
- Any outbound frame (`send_frame()`) resets the timer via a shared `Notify` or `watch` channel
- On `close()`, abort the background task via its `JoinHandle` before shutting down the write half. `close()` is already async, so this is a clean join.
- The keepalive task continues running during drain (GoingAway received) — the connection is still alive and the server still enforces idle timeout during drain

**API surface impact:** The current `into_framed()` and `framed_mut()` methods expose the raw `Framed` stream directly. With the stream split into read/write halves, these methods need to be removed or replaced. Since they are internal to the crate (not public API), this is a safe change. Any internal callers (bench commands) will be updated to use `send_frame()` / `recv_frame()` instead.

**`ClientConfig` gains a `ping_interval` field** (default `Duration::from_secs(10)`) so callers can override the keepalive interval or disable it (`Duration::ZERO` = no keepalive). This is important for tests and for SDK consumers with custom requirements.

For `recv_frame()` auto-Pong: when the inbound stream yields `Frame::Ping`, immediately send `Frame::Pong` and loop to read the next frame. This is a simple loop around the existing read logic. No rate-limiting on auto-Pong — if the server sends rapid Pings, the client replies to each. A misbehaving server is the server operator's problem, not the client's.

### 3. CLI Commands

No changes needed. The CLI commands use `FafbClient`, which will handle keepalive automatically. The `sub` command's long-lived connections will stay alive via background Pings. Short-lived commands (pub, fetch) will complete before any timeout.

### 4. Connection Lifecycle (Unchanged Server Behavior)

The server's `run_connection()` select loop is unchanged. The idle timeout already:
- Resets on any inbound frame (including Ping/Pong)
- Resets on any outbound Deliver
- Reads the timeout dynamically from `SharedConfig` (hot-reloadable)
- Sends `Error { InternalError, "idle timeout" }` and closes when it fires

With clients now sending Ping every 10s and the default idle timeout at 30s:
- **Healthy connection, busy**: traffic resets the timer constantly. No Pings needed.
- **Healthy connection, quiet subscriber**: client Pings every 10s keep the timer reset. Never times out.
- **Crashed client**: stops sending frames. Server detects within 30s. Inflight messages are released for redelivery to another consumer group member.

### 5. Message Safety on Disconnect

When the idle timeout fires and the server disconnects a dead client:

**Consumer group members:** All inflight messages belonging to the disconnected client are removed from the inflight map. The group's delivery task picks those offsets up on its next poll iteration and delivers them to another available group member. No messages are lost. Total recovery latency = idle timeout detection (~30s) + delivery task poll interval (typically sub-second).

**Individual subscribers (no group):** Inflight messages are abandoned. This is existing behavior and by design — there's no group to rebalance to.

**Detection speed improvement:** From ~300s (old default) to ~30s (new default). A 10x improvement in consumer group recovery time.

### 6. Documentation Changes

**`docs/protocol.md`** — Add a "Keepalive" section after the session flow:
- Clients MUST send `Ping` frames periodically to prevent idle timeout
- Recommended interval: every 10 seconds
- Server idle timeout default: 30 seconds
- Either side may send Ping; receiver must reply with Pong
- All frame types (not just Ping) reset the server's idle timer

**`docs/client_sdk_spec.md`** — Update:
- `TcpOptions.ping_interval_ms` recommended default: 30000 → 10000
- Add a "Keepalive" section explaining the contract: client sends Ping, server enforces idle timeout, failure to send Ping results in disconnection
- Update the conformance checklist: add "Client sends periodic Ping when connection is idle"
- Existing checklist item "Ping/Pong: respond to server Ping with Pong, send periodic Ping" — this already covers the requirement, but add explicit guidance that periodic Ping is mandatory (not optional)

**`config/config.example.toml`** — Add `idle_timeout_secs` with comment explaining the keepalive contract.

**`CLAUDE.md`** — Update gotchas section to note:
- `idle_timeout_secs` default changed from 300 to 30
- Clients must send keepalive Pings or they'll be disconnected

**`docs/delivery_guarantees.md`** — Update references to idle/inflight timeout defaults. Line 311 references idle timeout as a detection mechanism — update to reflect the new 30s default and the keepalive contract.

**`docs/server-tuning.md`** — Add `idle_timeout_secs` guidance. Currently does not mention it. Include recommendations for different deployment profiles (e.g., small deployments can use a tighter timeout, large deployments with many connections may want a longer interval).

### 7. Testing

**Unit tests:**

1. **Auto-Ping sends on idle** — create FafbClient with `ping_interval = 1s`, authenticate, wait 2s, assert server received Ping frame(s)
2. **Auto-Ping suppressed by traffic** — send Publish frames faster than ping_interval, assert no Pings sent
3. **Auto-Pong on server Ping** — server sends Ping to client, assert client replies Pong automatically, `recv_frame()` returns the next non-Ping frame
4. **Idle timeout with lower default** — set `idle_timeout_secs = 2`, connect, authenticate, don't send any frames, assert disconnected within ~2s with "idle timeout" error

**Integration tests:**

5. **Subscriber survives quiet period** — set `idle_timeout_secs = 3`, `ping_interval = 1s`, subscribe, stop publishing, wait 10s, assert subscription still alive (verifies keepalive without a 60s wall-clock penalty)
6. **Dead client detection** — subscribe with consumer group, kill client TCP without FIN, assert server detects within idle_timeout_secs and cleans up consumer group membership
7. **Inflight redelivery after dead client** — publish messages, deliver to consumer A in a group, kill A, start consumer B in same group, assert B receives the inflight messages after timeout

**Existing tests:**
- `tcp_integration.rs` idle timeout test (set `idle_timeout_secs = 1`, wait, assert error) — still passes, just testing a different default
- `tcp_integration.rs` Ping resets idle test — still passes

**Note on test values:** Tests use `idle_timeout_secs` values below the config validation minimum of 10 (e.g., 1s, 2s, 3s). This works because test helpers set `RuntimeConfig` directly, bypassing `ServerConfig::validate()`. This is intentional — tests need short timeouts to avoid slow test suites.

## Changes Summary

| Area | File(s) | Change |
|---|---|---|
| **Config default** | `src/config.rs` | `idle_timeout_secs` default 300 → 30 |
| **Client keepalive** | `src/client.rs` | Background Ping task on idle, auto-Pong on server Ping in `recv_frame()` |
| **Config example** | `config/config.example.toml` | Add `idle_timeout_secs` with keepalive docs |
| **Protocol doc** | `docs/protocol.md` | Add Keepalive section |
| **SDK spec** | `docs/client_sdk_spec.md` | Update `ping_interval_ms` default, add keepalive contract, update conformance checklist |
| **CLAUDE.md** | `CLAUDE.md` | Update gotchas for new default |
| **Delivery guarantees** | `docs/delivery_guarantees.md` | Update timeout defaults and detection mechanism references |
| **Server tuning** | `docs/server-tuning.md` | Add `idle_timeout_secs` guidance for deployment profiles |
| **Tests** | `tests/tcp_integration.rs`, `src/client.rs` | 7 test cases |

## What Doesn't Change

- Ping/Pong frame format (0x0C/0x0D)
- Server `run_connection()` select loop logic
- Session state machine Ping/Pong handling
- Drain/GoingAway behavior
- Namespace manager / consumer group cleanup (already triggered by connection close)
- Inflight tracking and redelivery mechanics
- Individual subscriber behavior (no redelivery by design)
