Skip to content

fix: Open a new connection on each stream reconnect - #232

Open
kinyoklion wants to merge 1 commit into
mainfrom
rlamb/stream-connection-pool-reuse
Open

kinyoklion wants to merge 1 commit into
mainfrom
rlamb/stream-connection-pool-reuse

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Summary

After a stream read timeout on an HTTP/2 connection, OkHttp keeps the connection in the pool and treats it as healthy until a 1 second ping deadline passes. After a long-lived stream fails, the first reconnect comes 0.5 to 1 second later. So when the network silently drops the stream connection, the reconnect opens a new stream on the same dead connection and waits for another 5 minute read timeout. Recovery takes about 10 minutes instead of 5.

The stream's OkHttp client now uses a connection pool that keeps no idle connections, so each reconnect opens a new connection. This applies to the FDv1 streaming data source and the FDv2 streaming synchronizer. The stream client has its own pool, so polling and event delivery are unchanged.

Restarts that the SDK starts itself, such as after an FDv2 goodbye or invalid data, also need launchdarkly/okhttp-eventsource#114. Without it, the abandoned HTTP/2 call keeps the old connection in use, so this pool setting cannot close it.

New tests count TCP connections across a reconnect and fail without this change.


Note

Overview
Fixes slow recovery after a silent stream disconnect by configuring the streaming OkHttp client to not keep idle connections (ConnectionPool(0, 1, SECONDS)), applied in both FDv1 StreamProcessor and FDv2 StreamingSynchronizerImpl after httpProperties so it overrides the default pool.

Without this, a reconnect shortly after a read timeout can reuse a dead HTTP/2 connection OkHttp still treats as healthy, stalling until the next long read timeout instead of opening a fresh socket.

Adds TCP connection-count tests for FDv1 and FDv2 that assert a reconnect uses a second connection (tests fail without the pool change). Polling and other HTTP clients are unchanged because only the stream EventSource client gets this pool.

Reviewed by Cursor Bugbot for commit 4f1c743. Bugbot is set up for automated code reviews on this repo. Configure here.

After a stream read timeout on an HTTP/2 connection, OkHttp keeps the
connection pooled and treats it as healthy until a 1 second ping deadline
passes. The first reconnect after a long-lived stream comes 0.5 to 1 second
later, so it reuses the dead connection and waits for another read timeout.

The stream client now uses a connection pool that keeps no idle connections,
for both the FDv1 streaming data source and the FDv2 streaming synchronizer.
@kinyoklion
kinyoklion marked this pull request as ready for review October 12, 2026 17:20
@kinyoklion
kinyoklion requested a review from a team as a code owner October 12, 2026 17:20
@kinyoklion

Copy link
Copy Markdown
Member Author

I'll hold the release of this one until the updated event source is included as well.

This branch has not been deployed

No deployments
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