You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #4394 (92e563f83, "Fix Pro renderer transport memory and reuse") changed the plain-Puma (no Fiber.scheduler) non-streaming path of the Pro renderer HTTP transport from connect-per-request to a persistent client per Puma thread. Each such client spawns its own background reactor OS thread and holds a warm keep-alive connection to the node renderer. As a result renderer_http_pool_size now bounds connections per thread, not globally, and the renderer sees roughly one persistent connection per Rails request thread — held open even when idle.
This is partly the intended effect (connection reuse lowers per-request latency), so it is filed as a resource/reliability regression + doc bug, not a correctness bug. But it changes the deployment's connection math and introduces a new stale-connection failure mode.
with_client (:704-720) — the else branch now does yield(persistent_thread_client) (was with_ephemeral_client).
persistent_thread_client (:743-767) caches one client per Thread.current (@thread_clients); reaped only when that thread dies (sweep_dead_thread_clients).
DEFAULT_RENDERER_HTTP_POOL_SIZE = 10; the config doc comment (configuration:66-68) still says "Without a scheduler, clients are per-request and this limits streams on that single request" — now stale.
Consequences (verified against the code)
Resource footprint (bounded, not an unbounded leak). Clients are keyed by Thread.current, so their count is bounded by the Puma thread count. Each adds a reactor OS thread + a warm socket + buffers, multiplied by worker processes; buffer_response (:305-312) also reads the whole renderer response into memory per request. So this is an elevated steady-state footprint, not runaway growth. A narrower, bounded resource leak is possible under Puma thread churn: a dead thread's client (a separate Thread) is only closed on the next request that triggers sweep_dead_thread_clients, so an orphaned reactor thread + socket can linger until then.
Connection errors / render failures (the real risk).
Renderer connection exhaustion. ~one persistent connection per Rails thread, held warm at idle, with no global cap. A renderer (Fastify, worker pool ≈ CPU-1) sized for the documented pool can be overwhelmed; connections beyond its limit are refused/reset → ConnectionError → render failure or fallback. Example: 8 workers × 32 threads = 256 request threads → up to ~256 persistent renderer connections at low load, where the old path opened a connection only during an active request and closed it immediately.
Stale keep-alive — a new failure mode. The Async client uses retries: 0 (:247). A warm connection idle-closed by the renderer's keep-alive timeout, a load balancer, or a proxy fails on next reuse with reset/EOF/broken-pipe instead of transparently reconnecting. The old connect-per-request path never hit this because it always dialed fresh.
Reactor-thread death (transient, self-healing). If run_loop raises, notify_pending_results errors in-flight callers and @thread.alive? goes false, so subsequent request calls short-circuit to worker_thread_exited_error (:231-232) until the next request rebuilds a fresh client.
The outer renderer_request_retry_limit can mask (2)/(3) but amplifies load against a struggling/connection-saturated renderer.
Not a data leak, not unbounded memory. Per-thread isolation (client keyed by Thread.current) plus per-request correlation (each request uses its own result = Queue.new and the reactor processes @queue sequentially, :224/:251-261) mean responses cannot be delivered to the wrong caller and no client is shared across threads/users. HTTP keep-alive reuse is independently framed. Cross-request/cross-tenant bleed is not a realistic consequence here.
Suggested fix
Bound total warm connections independent of Puma thread count — e.g. a shared/global client pool, or a cap on the number of PersistentThreadClients / their connections — so renderer_http_pool_size (or a new setting) means a global ceiling.
Handle stale persistent connections: retry once on a connection-level error (reset/EOF) with a fresh connection, or set a keep-alive idle timeout below the renderer/LB timeout so dead sockets are pruned before reuse (retries: 0 currently makes a stale reuse fail hard).
Reap orphaned per-thread clients proactively (not only lazily on the next request) so thread churn doesn't leave dangling reactor threads/sockets.
Update the stale DEFAULT_RENDERER_HTTP_POOL_SIZE doc comment to state it is a per-thread ceiling on the no-scheduler path.
Reproduced by static audit against main @ b6674a56b (with_client → persistent_thread_client → one PersistentThreadClient-with-reactor-per-Thread.current, retries: 0, per-request result-queue correlation); not runtime load-tested. Affects every plain-Puma (no Fiber.scheduler) Pro deployment doing non-streaming SSR — the default topology — and scales with total Puma thread count.
Summary
PR #4394 (
92e563f83, "Fix Pro renderer transport memory and reuse") changed the plain-Puma (noFiber.scheduler) non-streaming path of the Pro renderer HTTP transport from connect-per-request to a persistent client per Puma thread. Each such client spawns its own background reactor OS thread and holds a warm keep-alive connection to the node renderer. As a resultrenderer_http_pool_sizenow bounds connections per thread, not globally, and the renderer sees roughly one persistent connection per Rails request thread — held open even when idle.This is partly the intended effect (connection reuse lowers per-request latency), so it is filed as a resource/reliability regression + doc bug, not a correctness bug. But it changes the deployment's connection math and introduces a new stale-connection failure mode.
Where
react_on_rails_pro/lib/react_on_rails_pro/renderer_http_client.rbwith_client(:704-720) — theelsebranch now doesyield(persistent_thread_client)(waswith_ephemeral_client).persistent_thread_client(:743-767) caches one client perThread.current(@thread_clients); reaped only when that thread dies (sweep_dead_thread_clients).PersistentThreadClient#initialize(:181-192) →@thread = Thread.new { run_loop(...) };run_loop(:241-268) opensAsync { Async::HTTP::Client.new(endpoint, protocol:, retries: 0, limit: pool_limit) }.DEFAULT_RENDERER_HTTP_POOL_SIZE = 10; the config doc comment (configuration:66-68) still says "Without a scheduler, clients are per-request and this limits streams on that single request" — now stale.Consequences (verified against the code)
Resource footprint (bounded, not an unbounded leak). Clients are keyed by
Thread.current, so their count is bounded by the Puma thread count. Each adds a reactor OS thread + a warm socket + buffers, multiplied by worker processes;buffer_response(:305-312) also reads the whole renderer response into memory per request. So this is an elevated steady-state footprint, not runaway growth. A narrower, bounded resource leak is possible under Puma thread churn: a dead thread's client (a separateThread) is only closed on the next request that triggerssweep_dead_thread_clients, so an orphaned reactor thread + socket can linger until then.Connection errors / render failures (the real risk).
ConnectionError→ render failure or fallback. Example: 8 workers × 32 threads = 256 request threads → up to ~256 persistent renderer connections at low load, where the old path opened a connection only during an active request and closed it immediately.retries: 0(:247). A warm connection idle-closed by the renderer's keep-alive timeout, a load balancer, or a proxy fails on next reuse with reset/EOF/broken-pipe instead of transparently reconnecting. The old connect-per-request path never hit this because it always dialed fresh.run_loopraises,notify_pending_resultserrors in-flight callers and@thread.alive?goes false, so subsequentrequestcalls short-circuit toworker_thread_exited_error(:231-232) until the next request rebuilds a fresh client.The outer
renderer_request_retry_limitcan mask (2)/(3) but amplifies load against a struggling/connection-saturated renderer.Not a data leak, not unbounded memory. Per-thread isolation (client keyed by
Thread.current) plus per-request correlation (eachrequestuses its ownresult = Queue.newand the reactor processes@queuesequentially,:224/:251-261) mean responses cannot be delivered to the wrong caller and no client is shared across threads/users. HTTP keep-alive reuse is independently framed. Cross-request/cross-tenant bleed is not a realistic consequence here.Suggested fix
PersistentThreadClients / their connections — sorenderer_http_pool_size(or a new setting) means a global ceiling.retries: 0currently makes a stale reuse fail hard).DEFAULT_RENDERER_HTTP_POOL_SIZEdoc comment to state it is a per-thread ceiling on the no-scheduler path.Environment / attribution
92e563f83), replacing the prior ephemeral connect-per-request no-scheduler path.main@b6674a56b(with_client→persistent_thread_client→ onePersistentThreadClient-with-reactor-per-Thread.current,retries: 0, per-request result-queue correlation); not runtime load-tested. Affects every plain-Puma (noFiber.scheduler) Pro deployment doing non-streaming SSR — the default topology — and scales with total Puma thread count.