Skip to content

Commit d3bf9ae

Browse files
skunkworkerclaude
andcommitted
Fix nats-pure migration regressions; harden + speed up client and server (0.13.1)
Fixes a production 500 (RPC_ERROR / "EOF") and a broad set of related issues, all of the same class -- assumptions left over from the JNats -> nats-pure migration in 0.13.0 that became silently wrong -- then hardens and speeds up the client muxer and the server. Every change ships with negative/regression specs; benchmarks prove the performance and resilience wins. Full suite: 190 examples, 0 failures. Client / transport - Retry the transport errors nats-pure and the socket layer actually raise (EOFError, IOError, Errno::ECONNRESET/EPIPE/ECONNREFUSED/ETIMEDOUT, NATS::IO::ConnectionClosedError, ConnectionPool::TimeoutError, and java.io.IOException on JRuby) via Errors::RETRYABLE_TRANSPORT_ERRORS, routed through the reconnect_delay loop. The 0.13.0 collapse of IOException to the never-raised MriIOException had turned this into dead code. - Bounded + jittered retry: PB_NATS_CLIENT_MAX_RETRIES and PB_NATS_CLIENT_RECONNECT_DELAY_SPLAY_LIMIT; reconnect sleep is jittered so a fleet doesn't reconnect in lockstep. - Removed the dead :disable_reconnect_buffer connect option; connection_options now forwards only nats-pure-recognized keys. - Connection lifecycle: register callbacks before connect; close the half-open client on a failed handshake so reader/flusher threads aren't leaked. Response muxer (performance + robustness) - Dropped the per-message pending_size lock from the dispatch hot path (disable the byte-based slow-consumer limit, rely on the message-count limit): ~2.6x faster per message and removes the pending_size-drift bug that could silently drop all responses. - Dispatch loop no longer busy-spins during a restart window (nil @resp_sub guard); self-healing crash counter is a Concurrent::AtomicFixnum that decays once healthy (a plain Integer lost ~45% of updates under concurrent crashes). Server (performance + reliability + observability) - Parallelized request intake across PB_NATS_SERVER_SUBSCRIPTION_HANDLERS threads (default processor_count on JRuby, 1 on CRuby), each with per-thread self-healing. NATS queue-group semantics and subscription counts are unchanged -- each request is still delivered to exactly one consumer. ~8.4x intake throughput; head-of-line stall ~505ms -> ~0.5ms at 8 handlers. - Handler observability, long ops first-class: handlers are never aborted; in-flight tracking + new notifications (inflight_count, inflight_oldest_age_ms, overdue_handler_count, handler_overdue, pending_intake_queue_size, slow_handler (opt-in), thread_pool_saturated, subscription_handler_count/_crashed). A handler is "overdue" only once it outlives the client's response_timeout (PB_NATS_SERVER_HANDLER_OVERDUE_MS, default 65s). Durations use a monotonic clock. - Fail the caller fast: publish an encoded RPC_ERROR response (via PbError) when a request fails after it has been ACKed. - Thread pool: wait_for_termination prunes under the mutex and returns a real result; replenish respawns workers killed by a non-StandardError; shutdown drain timeout tracks handler_overdue so long handlers aren't killed mid-flight. - Error callbacks dispatch off the nats read/flush thread via a bounded executor. Config - No crash when the yml has no section for the current environment (or is empty). - YAML safe_load(aliases: true) instead of unsafe_load. Refactors / quality - Single source of truth for the monotonic clock (Protobuf::Nats.monotonic_time), the instrumentation suffix (Protobuf::Nats.instrument), and the self-healing backoff formula (Protobuf::Nats.crash_backoff_seconds). - Reuse Protobuf::Rpc::PbError for the server error response. Docs / benchmarks / tests - README (env vars, How it works, Resilience, Benchmarks) and CHANGELOG updated. - bench/muxer_resilience_bench.rb, bench/server_intake_bench.rb, and an opt-in bench/soak.rb (spawns nats-server, bounces it mid-run, asserts recovery). - wait_until spec helper for deterministic concurrency tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c77a5e5 commit d3bf9ae

24 files changed

Lines changed: 1619 additions & 222 deletions

CHANGELOG.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,25 @@
11
## Changelog
22

3+
### 0.13.1
4+
Fixes a production regression and a set of related issues, all of the same class: assumptions left over from the JNats → nats-pure migration in 0.13.0 that became silently wrong.
5+
6+
- **Dropped-connection retries were silently disabled.** Dropping JNats collapsed `Errors::IOException` to the never-raised `MriIOException`, so the client's reconnect/retry `rescue` became dead code. A dropped NATS connection then escaped immediately as an `RPC_ERROR` (surfacing as a 500) instead of being retried. The client now rescues the transport errors `nats-pure` and the socket layer actually raise (`EOFError`, `IOError`, `Errno::ECONNRESET`/`EPIPE`/`ECONNREFUSED`/`ETIMEDOUT`, `NATS::IO::ConnectionClosedError`, and `java.io.IOException` on JRuby) via `Errors::RETRYABLE_TRANSPORT_ERRORS` and rides them out with the existing `reconnect_delay` retry loop.
7+
- **Response muxer `pending_size` drift (could silently drop all responses).** nats-pure increments a subscription's `pending_size` (synchronized) for every inbound message and uses it to enforce the slow-consumer byte limit; for a callback-less subscription it never decrements it, so the muxer would be the sole consumer. Rather than mirror that accounting with a lock on every message, the muxer now **disables the byte-based limit** on its response subscription and relies on the message-count limit (the `SizedQueue` depth, tracked accurately for free). This removes the per-message lock from the dispatch hot path (~**2.7× faster** per message on JRuby — see `bench/muxer_resilience_bench.rb`) and eliminates the drift bug entirely.
8+
- **Dispatcher no longer busy-spins during a restart window.** If `@resp_sub` was briefly `nil` while the muxer restarted, the dispatch loop raised `NoMethodError` every iteration — busy-spinning and emitting a logged error + error-callback per spin. It now parks briefly (~0.2% of the old wasted work, zero errors).
9+
- **Self-healing backoff counter is now thread-safe.** The shared dispatcher crash counter was a plain `Integer` mutated by multiple dispatcher threads (it lost ~45% of updates under true parallelism on JRuby, corrupting the exponential backoff). It is now a `Concurrent::AtomicFixnum` that decays once a dispatcher is healthy.
10+
- **Client connection lifecycle hardening.** Connection callbacks (`on_disconnect`/`on_reconnect`/`on_close`/`on_error`) are now registered before `connect`, so handshake-time events are observed; and a failed handshake closes the half-open client so nats-pure's reader/flusher threads aren't leaked.
11+
- **Removed the dead `:disable_reconnect_buffer` connect option.** nats-pure has no such option (it was a JNats concept), so it was silently ignored. Transient disconnects are now handled by the client's transport-error retry path and `ack_timeout`.
12+
- **Server no longer leaves clients hanging on handler/publish failure.** If processing a request fails after the ACK was sent, the server now publishes an encoded `RPC_ERROR` response so the client fails fast instead of blocking until `response_timeout` (60s).
13+
- **Config no longer crashes when the YAML file has no section for the current environment** (or is empty); it falls back to defaults.
14+
- **TLS now floors at 1.2 and ceilings at 1.3** (replacing the deprecated `ssl_version = :TLSv1_2` hard pin), so TLS 1.3 is used when the server supports it and a TLS-1.2-only transport still negotiates down to 1.2. Verified on JRuby 9.4 and 10.0.
15+
- **Server request intake is now parallelized.** `SuperSubscriptionManager` drained the shared intake queue with a single thread that also published every ACK/NACK, so on JRuby intake was pinned to one core and one slow publish (e.g. nats-pure's buffer during a reconnect) head-of-line blocked *every* subject. Intake now fans out to `PB_NATS_SERVER_SUBSCRIPTION_HANDLERS` threads (default `processor_count` on JRuby, 1 on CRuby) with per-thread self-healing backoff. NATS queue-group semantics and subscription counts are unchanged — each request is still delivered to exactly one consumer. Measured **~8.5× intake throughput** and head-of-line stall **~505ms → ~0.4ms** at 8 handlers (`bench/server_intake_bench.rb`).
16+
- **Client retry is bounded and jittered.** `PB_NATS_CLIENT_MAX_RETRIES` (default 3) and `PB_NATS_CLIENT_RECONNECT_DELAY_SPLAY_LIMIT` (default 1000ms) make retries configurable, and the reconnect sleep now adds random jitter so a fleet hitting the same outage doesn't reconnect in lockstep.
17+
- **More transient errors are retried.** `ConnectionPool::TimeoutError` (subscription-pool exhaustion during a reconnect) is now treated as transient instead of surfacing as an `RPC_ERROR`.
18+
- **`connection_options` only forwards nats-pure-recognized keys** (servers, max_reconnect_attempts, connect_timeout, tls); app-level settings are read via their own accessors and no longer leak into `nats.connect`. YAML config now uses `safe_load`.
19+
- **Thread-pool robustness.** `wait_for_termination` prunes under its mutex and returns a real drained/timed-out result; a new `replenish` (called each server tick) respawns a worker killed by a non-StandardError. On shutdown the drain timeout tracks `handler_overdue_ms` so a legitimate long handler isn't killed mid-flight, and abandoned in-flight handlers are logged/instrumented.
20+
- **Error callbacks run off the read loop.** The nats `on_error` hooks dispatch via a bounded executor (`notify_error_callbacks_async`) so a slow user callback can't stall message processing for every subject.
21+
- **Server handler observability (long operations are first-class).** Handlers are never aborted — long-running operations (up to and beyond a minute) are allowed. The server now tracks in-flight handlers and emits `server.inflight_count`, `server.inflight_oldest_age_ms`, `server.overdue_handler_count`, `server.handler_overdue`, `server.pending_intake_queue_size`, `server.slow_handler` (opt-in via `PB_NATS_SERVER_SLOW_HANDLER_THRESHOLD_MS`), and `server.thread_pool_saturated`. A handler is only flagged "overdue" once it outlives the client's `response_timeout` (`PB_NATS_SERVER_HANDLER_OVERDUE_MS`, default 65s), so normal long ops are not mislabeled. Server duration metrics now use a monotonic clock.
22+
323
### 0.13.0
424
This is a large overhaul of the client and server internals.
525

README.md

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,23 @@ file is removed it will resubscribe and restart slow start (default: `nil`).
3939

4040
`PB_NATS_SERVER_SUBSCRIPTIONS_PER_RPC_ENDPOINT` - Number of subscriptions to create for each rpc endpoint. This number is
4141
used to allow JVM based servers to warm-up slowly to prevent jolts in runtime performance across your RPC network
42-
(default: 10).
42+
(default: 10). Each subscription joins the NATS queue group for its endpoint, so every request is still delivered to
43+
exactly one consumer — this knob controls subscription/interest count, not duplicate delivery.
44+
45+
`PB_NATS_SERVER_SUBSCRIPTION_HANDLERS` - Number of threads that drain the shared intake queue and publish ACK/NACKs
46+
(see [How it works](#how-it-works)). Defaults to `Concurrent.processor_count` on JRuby and `1` on CRuby. This is the
47+
*consumer* parallelism for messages this server has already received; it does not change how many topics are subscribed
48+
to or the queue-group delivery semantics. Minimum of 1.
49+
50+
`PB_NATS_SERVER_SLOW_HANDLER_THRESHOLD_MS` - If set (> 0), emit `server.slow_handler` when a handler runs longer than this
51+
many milliseconds. Informational/SLA only — handlers are never aborted (default: 0, off).
52+
53+
`PB_NATS_SERVER_HANDLER_OVERDUE_MS` - A handler still running past this many milliseconds is reported as "overdue"
54+
(`server.handler_overdue` + `server.overdue_handler_count`) — i.e. the client has already given up (`response_timeout`)
55+
so the work is orphaned. Defaults above the client's 60s response timeout so legitimate long operations are not flagged
56+
(default: 65000). **This should track your clients' `PB_NATS_CLIENT_RESPONSE_TIMEOUT`** — set it to roughly that value (plus
57+
a small grace). If clients use a longer response timeout, raise this so handlers aren't flagged overdue while a client is
58+
still waiting; if shorter, lower it so orphaned work is surfaced promptly.
4359

4460
`PB_NATS_CLIENT_ACK_TIMEOUT` - Seconds to wait for an ACK from the rpc server (default: 5 seconds).
4561

@@ -50,7 +66,11 @@ used to allow JVM based servers to warm-up slowly to prevent jolts in runtime pe
5066

5167
`PB_NATS_CLIENT_RESPONSE_TIMEOUT` - Seconds to wait for a non-ACK response from the rpc server (default: 60 seconds).
5268

53-
`PB_NATS_CLIENT_RECONNECT_DELAY` - If we detect a reconnect delay, we will wait this many seconds (default: the ACK timeout).
69+
`PB_NATS_CLIENT_RECONNECT_DELAY` - When a request hits a transient transport error (e.g. the NATS connection drops or is reset), the client sleeps this many seconds before retrying to give the connection time to re-establish (default: the ACK timeout). See [Resilience](#resilience).
70+
71+
`PB_NATS_CLIENT_RECONNECT_DELAY_SPLAY_LIMIT` - Random jitter (milliseconds, `0..limit`) added to the reconnect delay so a fleet hitting the same NATS outage does not reconnect in lockstep (default: 1000). Set to 0 to disable jitter.
72+
73+
`PB_NATS_CLIENT_MAX_RETRIES` - Number of attempts for ack-timeouts and transient transport errors before raising (default: 3).
5474

5575
`PB_NATS_CLIENT_SUBSCRIPTION_POOL_SIZE` - If subscription pooling is desired for the request/response cycle then the pool size maximum should be set; the pool is lazy and therefore will only start new subscriptions as necessary (default: 0)
5676

@@ -93,6 +113,8 @@ An example config looks like this:
93113
- "original_service": "replacement_service"
94114
```
95115

116+
When `uses_tls` is set, the client negotiates TLS with a floor of 1.2 and a ceiling of 1.3: it uses TLS 1.3 where the NATS server supports it and falls back to 1.2 otherwise (verified on JRuby 9.4 and 10.0).
117+
96118
## Usage
97119

98120
This library is designed to be an alternative transport implementation used by the `protobuf` gem. In order to make
@@ -162,13 +184,64 @@ If we were to add another service endpoint called `search` to the `UserService`
162184
- **ResponseMuxer** (`lib/protobuf/nats/response_muxer.rb`) — the client uses a single wildcard subscription to multiplex
163185
all RPC responses (similar to the Golang NATS client) instead of subscribing/unsubscribing per request. One or more
164186
dispatcher threads drain the shared subscription and route each reply to the waiting caller via a `Concurrent::Map`,
165-
keyed by a UUIDv7 request token. Tune the dispatcher count with `PB_NATS_RESPONSE_MUXER_DISPATCHERS`.
187+
keyed by a UUIDv7 request token. Tune the dispatcher count with `PB_NATS_RESPONSE_MUXER_DISPATCHERS`. Slow-consumer
188+
protection on the response subscription is by message count (the queue depth); the dispatch hot path does no per-message
189+
locking. Dispatcher threads self-heal: a crashed dispatcher is restarted with exponential backoff (capped at 60s) that
190+
decays once healthy.
166191
- **SuperSubscriptionManager** (`lib/protobuf/nats/super_subscription_manager.rb`) — the server manages the lifecycle of
167-
RPC endpoint subscriptions, including slow start, pausing, and resubscription.
192+
RPC endpoint subscriptions (NATS queue groups, so each request is delivered to one consumer), including slow start,
193+
pausing, and resubscription. All subscriptions feed one shared intake queue drained by `PB_NATS_SERVER_SUBSCRIPTION_HANDLERS`
194+
handler threads, so a slow ACK publish on one message can't head-of-line block every other subject. Handler threads
195+
self-heal with exponential backoff.
196+
- **Server observability** — beyond the thread-pool gauges, the server emits in-flight handler metrics
197+
(`server.inflight_count`, `server.inflight_oldest_age_ms`, `server.overdue_handler_count`, `server.handler_overdue`,
198+
`server.pending_intake_queue_size`, `server.thread_pool_saturated`). Long-running handlers are allowed and never aborted;
199+
a handler is only "overdue" once it outlives the client's `response_timeout` (see `PB_NATS_SERVER_HANDLER_OVERDUE_MS`).
200+
201+
## Resilience
202+
203+
The client is built to ride out transient NATS hiccups rather than surface them as request failures:
204+
205+
- **Transient transport errors are retried.** If a request hits a dropped/reset/closed connection (`EOFError`,
206+
`IOError`, `Errno::ECONNRESET`/`EPIPE`/`ECONNREFUSED`/`ETIMEDOUT`, `NATS::IO::ConnectionClosedError`, or a Java
207+
`IOException` on JRuby — see `Errors::RETRYABLE_TRANSPORT_ERRORS`), the client sleeps `PB_NATS_CLIENT_RECONNECT_DELAY`
208+
and retries (up to 3 attempts) while `nats-pure` re-establishes the connection in the background.
209+
- **Missing ACKs and NACKs are retried** with their own timeouts/backoff (`PB_NATS_CLIENT_ACK_TIMEOUT`,
210+
`PB_NATS_CLIENT_NACK_BACKOFF_INTERVALS`).
211+
- **Server-side failures fail the caller fast.** If the server cannot process a request after it has ACKed, it publishes
212+
an encoded RPC error response so the client raises immediately instead of blocking until `PB_NATS_CLIENT_RESPONSE_TIMEOUT`.
213+
- **The response dispatcher self-heals.** A crashed muxer dispatcher restarts with exponential backoff, and a brief
214+
subscription-restart window won't busy-spin the dispatch loop.
215+
216+
See `bench/muxer_resilience_bench.rb` for microbenchmarks of the dispatch hot path and these resilience paths.
217+
218+
## Delivery semantics (at-least-once)
219+
220+
**Current design choice:** RPC delivery is **at-least-once**, and the gem does **not** deduplicate requests. The resilience features above are the reason: when the client retries on an ACK/response timeout or a transient transport error, the server may have *already received and processed* the original request, so a single client call can run a handler **more than once**. (NATS queue groups guarantee each *delivered* message goes to one consumer, but they do not prevent the client from re-sending after a timeout.)
221+
222+
The gem deliberately favors at-least-once over at-most-once: dropping work on a transient blip is usually worse than occasionally repeating it. Making this safe is therefore the **service author's responsibility** — handlers that have side effects should be written to be idempotent:
223+
224+
- Key writes on a natural/business id or a client-supplied idempotency token (upsert / `find_or_create`) rather than blind inserts.
225+
- Make external side effects (charges, emails, downstream RPCs) safe to repeat, or guard them with your own dedup keyed on a request id you put in the message.
226+
- Naturally idempotent operations (reads, idempotent upserts) need no special handling.
227+
228+
**Why no built-in dedup (yet):** correct dedup across a horizontally-scaled service requires a *shared* store (a retry can land on a different server instance), a tuned TTL, and a cached response to replay on duplicates — and it only helps RPCs that aren't already idempotent. A future, **opt-in per-RPC** dedup with a pluggable store may be added; it will not be the default. Until then, treat handlers as potentially re-run.
168229

169230
## Future Improvements (locked behind ruby version)
170231
- Migrate from the `uuid7` gem to native `Random#uuid_v7` once the minimum Ruby version supports it (see `UUIDv7Helper`).
171232

233+
## Benchmarks
234+
235+
Microbenchmarks live in `bench/` and measure both the old and new behavior in one process (no NATS server required). See `bench/bench.md` for details. Highlights on JRuby:
236+
237+
- `bench/muxer_resilience_bench.rb` — response-muxer dispatch hot path (~2.5× faster per message with the per-message lock removed), restart-window resilience, and crash-counter accuracy.
238+
- `bench/server_intake_bench.rb` — server intake fan-out (~8× throughput, head-of-line stall ~505ms → ~2ms) and the handler-exhaustion observability.
239+
240+
```
241+
bundle exec ruby -Ilib bench/server_intake_bench.rb
242+
bundle exec ruby -Ilib bench/muxer_resilience_bench.rb
243+
```
244+
172245
## Development
173246

174247
After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake test` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment.

0 commit comments

Comments
 (0)