Jeff Bahr
90d · built 2026-07-24
90-day totals
- Commits
- 37
- Grow
- 3.0
- Maintenance
- 7.4
- Fixes
- 2.5
- Total ETV
- 12.8
30-day trajectory
Last 30 days vs. the 30 days before. Up arrows on Growth and ETV mean improvement; up arrow on Fixes share means more time on fixes (worse).
↓-84.0 %
vs 25 prior
↓-19.7 pp
recent vs prior
↑+11.8 pp
recent vs prior
Daily performance
Daily ETV, stacked by Growth, Maintenance and Fixes.
Work-mix over time
Share of Growth / Maintenance / Fixes over a rolling 7-day window. Reads as 'where is effort flowing right now'.
Repository spread
Where this developer's commits land. Concentrated work (top1 > 80%) vs polymath spread (top1 < 30%).
Most impactful commits
Top 20 by ETV in the 90-day window.
- 2.9ETVAdd ALPN support to thrift java stack Summary: ## What this diff does Introduces a new server transport for Java Thrift — **`UnifiedServerTransport`** — that listens on a single TLS port and uses **ALPN (Application-Layer Protocol Negotiation)** to dynamically select the wire protocol per connection. Two protocols are advertised and supported: `"rs"` (RSocket) and `"thrift"` (THeader/Rocket). This enables in-place migration from Header → Rocket without touching listening ports, DNS, or connection routing. ## Why Today, a Java Thrift server is bound to exactly one wire protocol at process start (`LegacyServerTransportFactory` for Header, `RSocketServerTransportFactory` for Rocket). Migrating a service from Header to Rocket requires a port change, a load-balancer reconfiguration, or a full restart with a new transport. ALPN lets a single server speak both protocols simultaneously, so individual clients can be migrated one at a time without coordinating with the server-side rollout. This matches the C++ ThriftServer behavior, which has supported the same pattern for years. ## Architecture The diff adds five new classes under `com.facebook.thrift.transport.unified`: - **`UnifiedServerTransportFactory`** — `ServerTransportFactory` impl that constructs `UnifiedServerTransport` instances. Plugged into `RpcThriftServer` dispatch (see below). - **`UnifiedServerTransport`** — Reactor-Netty `TcpServer` bound to the configured address with TLS+ALPN. On `doOnConnection`, inspects `SslHandler.applicationProtocol()` and routes: - ALPN selected `"rs"` → adds RSocket length-field decoder, hands the connection to `RSocketServer.asConnectionAcceptor()` wrapped in `TcpDuplexConnection`. - ALPN selected `"thrift"` → adds `ThriftHeaderFrameLengthBasedDecoder` + `LengthFieldPrepender` and hands the connection to `ThriftConnectionAcceptor`. - ALPN absent or unrecognised → **falls back to HEADER** (matches C++ ThriftServer behavior; downgrades from "fail closed" to "fail safe" so legacy clients without ALPN still work). - **`ThriftConnectionAcceptor`** — Reactor-Netty connection handler that bridges Netty inbound bytes to `RpcServerHandler` via `ReactiveHeaderCodec`. Decodes `ByteBuf` → `ThriftFrame`, dispatches request kind (oneway / req-resp), encodes response back, manages timeouts and per-request reference counting. - **`ReactiveHeaderCodec`** — Stateless encoder/decoder for THeader frames, ported from `HeaderTransportCodec` with full parity to landed bug fixes (multi-transform decompression via `CompressionManager` in reverse order, persistent-only header-section parsing, `readString` bounds check, per-header error logging on parse failure). - **`UnifiedServerTransport$ThriftChannelInitializer`** — `ChannelPipelineConfigurer` that enforces `ThriftServerConfig.connectionLimit` and installs `MetricsChannelDuplexHandler` for accepted/rejected/dropped connection counters. Supporting changes: - **`ThriftTransportType`** — adds an `UNKNOWN` enum constant; `fromProtocol(null | unrecognised)` now returns `UNKNOWN` (instead of throwing) so the unified server can apply the fallback-to-HEADER policy uniformly. - **`RpcServerUtils`** — `getSslContext()` configures `ApplicationProtocolConfig` with `[RSOCKET, HEADER]` when `config.isEnableAlpn()`. Also adds `isSslCloseNotify(Throwable)` helper to filter benign TLS shutdown alerts from error logs. - **`RpcThriftServer`** — dispatch logic now routes to `UnifiedServerTransportFactory` first when `config.isEnableAlpn()`, then `RSocketServerTransportFactory` if `useRSocket`, then `LegacyServerTransportFactory` (the old default). - **Client side** (`ThriftClientInitializer`, `LegacyRpcClientFactory`) — propagates the remote `InetSocketAddress` into `SslContext.newHandler(...)` so the client SNI/ALPN handshake completes correctly. No client behavior change when ALPN is disabled. ## Migration strategy **Opt-in via config flag.** The new transport is activated only when `ThriftServerConfig.setEnableAlpn(true)` is called. Default behavior is unchanged — services that don't opt in continue to use `LegacyServerTransportFactory` or `RSocketServerTransportFactory` exactly as before. **Backwards compatible at the wire level.** A unified server with ALPN enabled accepts: - ALPN-aware clients negotiating `"thrift"` → routed to Header path (same wire format as existing Header servers). - ALPN-aware clients negotiating `"rs"` → routed to Rocket path (same wire format as existing Rocket servers). - Legacy clients that don't send an ALPN extension at all → falls back to Header (per the new no-ALPN handling). This means a Header-speaking legacy client can connect to an ALPN-enabled server without modification. **No client SDK changes required for adoption.** Any existing TLS-enabled Java client that does TLS handshake against an ALPN-enabled server will work — either via ALPN negotiation (when the client supports ALPN) or via the no-ALPN HEADER fallback. **Per-service rollout.** Services adopt the unified transport one at a time by setting `setEnableAlpn(true)` in their `ThriftServerConfig`. **Rollback.** A single-line config revert (`setEnableAlpn(false)`) returns a service to `LegacyServerTransportFactory`. No data-format compatibility concerns — the wire formats are unchanged on each ALPN path. Reviewed By: adolfojunior, RayanRal Differential Revision: D87745292 fbshipit-source-id: a5587ceef699ecf6893a9dcd97ab8fab19596af4github.com-facebook-fbthrift · 972aa12a · 2026-04-30
- 1.3ETVIntroduce CompositeMovingCounter for 1m/10m/1h moving rates Summary: Introduces `CompositeMovingCounter`, a lock-free sliding-window counter that exposes 1-minute, 10-minute, and 1-hour rates from a single hot path. Writers contend on one `LongAdder` reservoir; rotations are coordinated by one packed `AtomicLong` state machine (`(tick << 2) | mode`). A single CAS per rotation fans the closed base-tick delta into three internal rings at progressively coarser bucket resolutions (1s / 10s / 60s × 60 buckets each), so serving three windows costs the same CAS budget as serving one. The lock-free protocol — STABLE / ROTATING_FAST / ROTATING_SLOW modes, paired `(bucketTicks, bucketDeltas)` slot identity, load-bearing `state.get()` bracketing on reads — is documented inline on the class. Coarser rings are derived from the base rotation rather than driven by their own state machines. Also introduces `SpinWait`, a spin helper used by the lock-free write path, with a Java 11 multi-release override that calls `Thread.onSpinWait()`. Reviewed By: adolfojunior Differential Revision: D106699916 fbshipit-source-id: 0b3f55e22ec0d4085e2071b5a91200f03936ad28github.com-facebook-fbthrift · acc29470 · 2026-05-28
- 1.2ETVFree header request buffer right after decode Summary: Java Thrift's header transports (legacy `ThriftServerHandler` and reactive v2 `ThriftConnectionAcceptor`) hold the inbound request frame (the request `ByteBuf`) alive until the response `Mono` completes -- through handler execution and response serialization -- even though the request bytes are only needed until the arguments are decoded. For large requests or slow handlers this pins pooled Netty memory far longer than necessary and shows up as large server-side memory spikes. The bytes are fully materialized into concrete Java objects by `ServerRequestPayload.getData(...)`, which the generated server handler calls synchronously at the start of each method; after that call returns nothing downstream (the delegate, response writing) touches the request buffer, so it can be freed immediately. The subtlety is inheritance: for a `Child extends Parent` service, a request for a parent method enters the child handler, whose dispatcher does not read -- it forwards a deferred `Mono` to `super`. Releasing in the dispatcher would free the frame before the parent actually reads it: a use-after-free. Fix: `ServerRequestPayload` now owns the request buffer and exposes `releaseRequestData()`. The generated `_do<method>` releases it in a `finally` immediately after `getData`, i.e. at the exact site that performs the read -- so inherited methods are released by the parent's `_do<method>`, never by the child dispatcher. The transports call the same idempotent (`AtomicReference.getAndSet`) `releaseRequestData()` as a backstop for the paths where no generated method body runs (unknown method, off-loop scheduler rejection). `ThriftAnyDeserializer` now copies binary `Any` values instead of returning a slice of the request buffer, so they survive the early release; this matches the existing copying `ByteBuf` `TypeAdapter`s, so no materialized object aliases the request buffer (no zero-copy). Rollout: gated behind a kill-switch `RpcResources.isReleaseHeaderFrameAfterDecode()` (system property `thrift.release-header-frame-after-decode`), default off. When off, the transport keeps the frame and releases it at response completion exactly as before; when on, it hands the frame to the payload for eager release. The flag lives in the transport so generated code stays flag-free and the switch can be flipped without redeploying generated services. `releaseRequestData()` is a default no-op on the `ServerRequestPayload` interface so custom/test implementations are unaffected, and RSocket is untouched (its payload owns no buffer here, so the generated release is a no-op). Reviewed By: adolfojunior Differential Revision: D106871318 fbshipit-source-id: d232fc3b6af71ed2dc1f6bd6441922d9768d7f68github.com-facebook-fbthrift · c29039d7 · 2026-06-02
- 1.0ETVUnifiedServerTransport: honor allowPlaintext, peek-and-demux on the same port Summary: The unified factory (`RpcThriftServer` -> `UnifiedServerTransportFactory`, selected when `ThriftServerConfig.enableAlpn=true`) hard-wires reactor-netty's `.secure()` for every connection. As a result `ThriftServerConfig.allowPlaintext` is silently ignored on this code path, even though it is honored by `LegacyServerTransportFactory`. Any service that turns on ALPN therefore drops plaintext clients on the floor — including Tupperware-side fb303 monitoring (fbagent's per-task fb303 collector, the SR fb303 tier `fbagent.fb303.thrift`) which talks header-thrift over plaintext loopback because `Fb303Collector.transport` is not plumbed through `getFb303ConnectionParams` and `FB303ClientBuilder::buildOldClient` hardcodes `thrift_security=disabled` for loopback. This was discovered while debugging an fblite canary failure (`Wait for Tupperware canary targets to become healthy`): ~6% of canary tasks were missing from the per-task ODS metric `snaptu.gateway.AppSrvSessionPool.NumOfSessions.val`, so the Sandcastle wait step (`SandcastleFBLiteCanaryTupperwareWaitForTargetsStep::genIsTaskReportingToOds`) marked them not-ready and the test groups violated the SLO. Direct fb303-status probes against the affected tasks worked over rocket and theader (both ALPN-aware), but fbagent's plaintext header scrape was getting cut off in the SSL handshake because the unified factory has no plaintext path. # What changes `UnifiedServerTransport.createNewInstance` now branches on `config.isAllowPlaintext()`: - `allowPlaintext=false` (default for callers that explicitly opt out): existing `.secure(SslProvider)` path, byte-for-byte unchanged. TLS-only. - `allowPlaintext=true` (the existing `ThriftServerConfig` default): install `OptionalSslPeeker` + `DeferChannelActiveHandler` in `doOnChannelInit` instead of `.secure()`. Per-connection peek of the first 5 bytes: - If the bytes match a TLS record header, install an `SslHandler` (built from the same `SslContext` that `RpcServerUtils.getSslContext(config)` already provides — same cert, ALPN config, cipher list, JDK-vs-OpenSSL provider) immediately after the peeker. The cumulation buffer flushed at `handlerRemoved` flows through the new `SslHandler`. Handshake then proceeds normally and `SslHandshakeCompletionEvent.SUCCESS` fires upstream. - Otherwise, fire `PlaintextConfirmedEvent.INSTANCE` upstream as a Netty user event. `DeferChannelActiveHandler` mirrors reactor-netty's internal `SslReadHandler`: it suppresses the original `channelActive` and re-fires it on either `SslHandshakeCompletionEvent.SUCCESS` (TLS branch) or `PlaintextConfirmedEvent.INSTANCE` (plaintext branch). This keeps `doOnConnection` as the single stack-wide entry point — ALPN selection, rocket/header pipeline install, and `Connection`-object lifecycle are all unchanged. `getProtocol` returns `HEADER` when no `SslHandler` is in the pipeline (instead of throwing). On the plaintext branch ALPN is impossible, so HEADER is the only sane fallback — matching the C++ ThriftServer behavior when no ALPN protocol is negotiated. This mirrors C++'s `setAllowPlaintextOnLoopback` semantics in spirit (peek-then-demux on the same port, fall back to plaintext header), but without adding a new config knob — the Java-side `allowPlaintext` field has existed since the legacy factory and is the natural toggle to plumb through. # Two correctness bugs caught in review and fixed in the same commit 1. **Peek-phase timeout in `OptionalSslPeeker`.** With the peeker installed but no `SslHandler` yet, a client that connects but sends 0-4 bytes would consume a server channel indefinitely — the `SslHandler.handshakeTimeoutMillis` only starts once the SslHandler is in the pipeline. Added a `ScheduledFuture` started in `handlerAdded`/`channelActive` that closes the channel after `PEEK_TIMEOUT_MILLIS` (10s — same default as `SslHandler.handshakeTimeoutMillis` so the worst-case wall time for a slow client is identical on both branches). The timeout is cancelled both on `decode()` reaching a decision and on `handlerRemoved0`. A test-only second constructor takes an explicit timeout for `EmbeddedChannel`-based unit tests. 2. **Unmatched `channelInactive` in `MetricsChannelDuplexHandler`.** The handler unconditionally decremented `channelCount` and bumped `droppedConnections` in `channelInactive`, but its `channelActive` is gated by any upstream defer-active handler (reactor-netty's `SslReadHandler` or our new `DeferChannelActiveHandler`). If a client disconnected before `channelActive` propagated downstream — during the peek wait, during TLS handshake, or any other defer — the gauge would underflow and connection-limit accounting would silently break. Fix: track a per-instance `incremented` flag, only decrement when it's set, and reset it on `channelInactive`. This protects every deferred-active path, not just ours. # Behavioral implications for existing callers `ThriftServerConfig.allowPlaintext` defaults to `true` (set in the legacy factory's day). Existing callers that enable ALPN therefore go from "TLS-only via `.secure()`" to "peek-and-demux via `OptionalSslPeeker`" by default. The peeker is transparent for TLS clients (same `SslContext`, same ALPN handshake), and all 35 existing `UnifiedServerTransportTest` cases pass under the new path. Callers that genuinely want TLS-only must explicitly set `setAllowPlaintext(false)`. This matches the legacy factory's existing behavior: the field has the same default and the same semantics there. Reviewed By: IlayDavid95, cnli87 Differential Revision: D104335933 fbshipit-source-id: 8743de27f11422d1fb61e60d51ea975c1472c594github.com-facebook-fbthrift · 3df68197 · 2026-05-12
- 0.8ETVAdd ThriftEventHandler.preprocess admission-control hook Summary: Adds a new `ThriftEventHandler.preprocess(Object context, String methodName)` lifecycle hook invoked by the generated `RpcServerHandler` dispatchers on the **caller thread** (typically the I/O / decoder thread) BEFORE the request is queued onto the off-loop scheduler. This is the right place to admit or shed: throwing here -- typically a `TApplicationException` with code `LOADSHEDDING` -- bails out before paying queue time, parse CPU, or user-handler dispatch, and frees the request buffer immediately via the transport's outer `doFinally`. Mirrors the C++ server's `preprocess()` hook (`ThriftRocketServerHandler::handleRequestCommon`). Lifecycle and contracts: - New order per request: `getContext` (once per handler) -> `preprocess` -> `preRead` -> `postRead` -> user handler -> `preWrite` -> `postWrite` -> `done`. - The `Object context` returned by `getContext` flows through `preprocess` and every later hook, so per-request state lives on that object, not on the handler instance. - `getContext` and `preprocess` get the `RequestContext` via the explicit `requestContext` argument; the caller-thread `RequestContexts` ThreadLocal is not set during admission. Off-loop hooks (`preRead`, `postRead`, `preWrite`, user handler) still see the ThreadLocal via the inner `Mono.defer` set/restore. - `done()` is guaranteed to fire exactly once on every terminal path -- normal completion, `preprocess` shed (TApplicationException or other Throwable), user handler throws, cancellation -- via a dispatcher-level outer `doFinally(__ -> _chain.done())`. Pair state mutations across `getContext` / `done` for unconditional balance. - `ContextChain` constructor now unwinds partial-construction failures: if a later handler's `getContext` throws, it calls `done()` on every prior handler whose `getContext` succeeded, in reverse order, before rethrowing. Prevents leak in admission counters like `LoadSheddingHandler.activeRequests` when one handler in the chain crashes. Generated dispatcher (all three of `singleRequestSingleResponse` / `singleRequestStreamingResponse` / `singleRequestNoResponse`): - Inheritance: child dispatchers check `_methodMap.containsKey(_name)` and forward to `super` for inherited methods, so the parent's lifecycle fires only once at the level that owns the method (no double-fire). - Constructs `ContextChain` and calls `_chain.preprocess()` on the caller thread, before `Mono.defer` / `Flux.defer`. - `TApplicationException` from `preprocess` -> `Mono.just(fromTApplicationException(_tae, ..., _chain))` so the client gets a proper Thrift error response and the chain's write hooks fire. - Other `Throwable` from `preprocess` -> wrapped via new `RpcPayloadUtil.internalErrorResponse(t, "preprocess", metadata, _chain)` helper that produces a `TApplicationException.INTERNAL_ERROR` (matching the C++ pattern), and emitted as `Mono.just(errorResponse)` (oneway has no response channel so it uses `Mono.error(t)`). - Chain-construction failure (a `getContext` throws) -> `Mono.just(internalErrorResponse(t, "context chain construction", metadata, null))`; the constructor has already unwound any partial state. Handler migration to the new hook: - `LoadSheddingHandler` moved from `preRead` (off-loop, after queue + parse) to `preprocess` (caller thread, before queue). `getContext` returns `activeRequests.incrementAndGet()` as the per-request context (a `Long`); `preprocess` checks THAT value, not the shared atomic, so two concurrent arrivals at the limit don't both observe the post-burst total and both shed. `done()` decrements as before. - `LoadHeaderHandler` increment moved from `preRead` to `getContext` (paired with the existing `done()` decrement) so the admission count is correct even when the chain fails partway through construction. Generated code: all 34 `*RpcServerHandler.java` golden fixtures regenerated. Out-of-tree compatibility note: any third-party `ThriftEventHandler.getContext` or `preprocess` implementation that called `RequestContexts.getCurrentContext()` will now see `null` (or the caller thread's prior context). The request context is the explicit `requestContext` argument to `getContext`; stash it on the returned per-handler context object if `preprocess` needs it. Reviewed By: adolfojunior, prakashgayasen Differential Revision: D108211201 fbshipit-source-id: 77615182b833271dadc75c91488cd47167c2073fgithub.com-facebook-fbthrift · 1996eeaf · 2026-06-11
- 0.8ETVAdd ThriftEventHandler.preprocess admission-control hook Summary: Adds a new `ThriftEventHandler.preprocess(Object context, String methodName)` lifecycle hook invoked by the generated `RpcServerHandler` dispatchers on the **caller thread** (typically the I/O / decoder thread) BEFORE the request is queued onto the off-loop scheduler. This is the right place to admit or shed: throwing here -- typically a `TApplicationException` with code `LOADSHEDDING` -- bails out before paying queue time, parse CPU, or user-handler dispatch, and frees the request buffer immediately via the transport's outer `doFinally`. Mirrors the C++ server's `preprocess()` hook (`ThriftRocketServerHandler::handleRequestCommon`). Lifecycle and contracts: - New order per request: `getContext` (once per handler) -> `preprocess` -> `preRead` -> `postRead` -> user handler -> `preWrite` -> `postWrite` -> `done`. - The `Object context` returned by `getContext` flows through `preprocess` and every later hook, so per-request state lives on that object, not on the handler instance. - `getContext` and `preprocess` get the `RequestContext` via the explicit `requestContext` argument; the caller-thread `RequestContexts` ThreadLocal is not set during admission. Off-loop hooks (`preRead`, `postRead`, `preWrite`, user handler) still see the ThreadLocal via the inner `Mono.defer` set/restore. - `done()` is guaranteed to fire exactly once on every terminal path -- normal completion, `preprocess` shed (TApplicationException or other Throwable), user handler throws, cancellation -- via a dispatcher-level outer `doFinally(__ -> _chain.done())`. Pair state mutations across `getContext` / `done` for unconditional balance. - `ContextChain` constructor now unwinds partial-construction failures: if a later handler's `getContext` throws, it calls `done()` on every prior handler whose `getContext` succeeded, in reverse order, before rethrowing. Prevents leak in admission counters like `LoadSheddingHandler.activeRequests` when one handler in the chain crashes. Generated dispatcher (all three of `singleRequestSingleResponse` / `singleRequestStreamingResponse` / `singleRequestNoResponse`): - Inheritance: child dispatchers check `_methodMap.containsKey(_name)` and forward to `super` for inherited methods, so the parent's lifecycle fires only once at the level that owns the method (no double-fire). - Constructs `ContextChain` and calls `_chain.preprocess()` on the caller thread, before `Mono.defer` / `Flux.defer`. - `TApplicationException` from `preprocess` -> `Mono.just(fromTApplicationException(_tae, ..., _chain))` so the client gets a proper Thrift error response and the chain's write hooks fire. - Other `Throwable` from `preprocess` -> wrapped via new `RpcPayloadUtil.internalErrorResponse(t, "preprocess", metadata, _chain)` helper that produces a `TApplicationException.INTERNAL_ERROR` (matching the C++ pattern), and emitted as `Mono.just(errorResponse)` (oneway has no response channel so it uses `Mono.error(t)`). - Chain-construction failure (a `getContext` throws) -> `Mono.just(internalErrorResponse(t, "context chain construction", metadata, null))`; the constructor has already unwound any partial state. Handler migration to the new hook: - `LoadSheddingHandler` moved from `preRead` (off-loop, after queue + parse) to `preprocess` (caller thread, before queue). `getContext` returns `activeRequests.incrementAndGet()` as the per-request context (a `Long`); `preprocess` checks THAT value, not the shared atomic, so two concurrent arrivals at the limit don't both observe the post-burst total and both shed. `done()` decrements as before. - `LoadHeaderHandler` increment moved from `preRead` to `getContext` (paired with the existing `done()` decrement) so the admission count is correct even when the chain fails partway through construction. Generated code: all 34 `*RpcServerHandler.java` golden fixtures regenerated. Out-of-tree compatibility note: any third-party `ThriftEventHandler.getContext` or `preprocess` implementation that called `RequestContexts.getCurrentContext()` will now see `null` (or the caller thread's prior context). The request context is the explicit `requestContext` argument to `getContext`; stash it on the returned per-handler context object if `preprocess` needs it. Reviewed By: adolfojunior Differential Revision: D107327207 fbshipit-source-id: 941d0fab5f6d09969a1207a28c1efd12b1e6d7d0github.com-facebook-fbthrift · d6bcf895 · 2026-06-05
- 0.8ETVFix stream exception handling for Java Rsocket client/server Summary: Fixes bugs in exception handling for streaming RPC methods where undeclared exceptions were incorrectly decoded as TTransportException and stream-level declared exceptions could be incorrectly sent during the initial response phase. Changes: Server-side (StreamResponseHandlerTemplate): - Properly separate function-level vs stream-level declared exceptions - Stream-declared exceptions thrown before stream establishment are now wrapped in TApplicationException per protocol specification - Added context-aware exception checking (isKnownException now takes checkStreamExceptions parameter) Client-side (RSocketRpcClient): - Check both ResponseRpcMetadata and StreamPayloadMetadata for undeclared exceptions in streaming responses - Function-level exceptions use ResponseRpcMetadata, stream-level use StreamPayloadMetadata per Rocket protocol specification Utility improvements (RpcClientUtils): - Added getUndeclaredException(StreamPayloadMetadata) overload - Changed undefined stream exceptions from TTransportException to TApplicationException for consistency with non-streaming behavior - Refactored using Supplier pattern to eliminate code duplication Test updates (UnifiedServerTransportTest): - Updated expectations to verify TApplicationException (not TTransportException) - Verify exception message content instead of incorrect cause chain This ensures Java server compatibility with C++ clients via Service Router and adherence to the Thrift streaming protocol specification. Reviewed By: robertroeser Differential Revision: D88109675 fbshipit-source-id: 456e35879d7e26d0fcfabb40b9dd13befcc08752github.com-facebook-fbthrift · 2ba3ca18 · 2026-04-25
- 0.6ETVFree RSocket request buffer right after decode Summary: The RSocket server transport (`ThriftServerRSocket`) historically held the request `ByteBuf` and the RSocket `Payload` alive until the response `Mono` completed -- through user-handler execution and response serialization -- even though the request bytes are only needed until the arguments are decoded. For large requests or slow handlers this pinned pooled Netty memory far longer than necessary and showed up as server-side memory spikes. This mirrors the header-transport optimization landed in D106871318 for the RSocket path; RSocket server has no production users yet so this is unconditional (no rollout flag). Mechanism (same shape as D106871318): - `ServerRequestPayload` now owns the framework's reference to the request buffer (when the transport hands one over) and exposes `releaseRequestData()`. `DefaultServerRequestPayload` uses `AtomicReference.getAndSet(null)` so releases are idempotent regardless of how many times the generated handler + transport backstop both call it. - New 4-arg `ServerRequestPayload.create(reader, metadata, ctx, ReferenceCounted)` overload lets the transport pass its owning reference at construction time; existing 3-arg / int-seqId overloads pass `null` and remain a no-op. - `ThriftServerRSocket` constructs a `RequestBuffers extends AbstractReferenceCounted` composite that releases BOTH the decoded request `ByteBuf` and the RSocket `Payload` in a single `deallocate()`. Both refs are required: in the uncompressed case `requestData = payload.sliceData().retain()` -- a retained slice of the same underlying buffer -- so the memory is only freed once both are released. In the compressed case `requestData` is independently allocated. The composite is handed to the payload via the new 4-arg `create(...)`. - All three RSocket entry points (`requestResponse`, `requestStream`, `fireAndForget`) hand the `RequestBuffers` composite to the payload and use the idempotent `payload.releaseRequestData()` as the `doFinally` backstop. The generated `_do<method>` (request-response/oneway) releases the buffers immediately after `getData(...)` via the existing finally-block release that D106871318 added to the whisker template. The streaming dispatcher reads in `StreamResponseHandlerTemplate.handleStream`; this commit adds the same release-after-read pattern there so streams don't pin the buffer for their whole lifetime. Interaction with the `preprocess` admission-control hook (preceding diff in this stack): The `preprocess` hook returns `Mono.just(errorResponsePayload)` (for `TApplicationException`) or `Mono.error(t)` (other Throwables) BEFORE the inner `Mono.defer` is ever subscribed, so the generated `_do<method>` never runs. The transport's outer `doFinally(__ -> finalRequestPayload.releaseRequestData())` is the safety net that catches both paths -- the `RequestBuffers` composite's `deallocate()` fires when the (error) response terminates the chain, freeing both refs. So a shed at `preprocess` time releases the request buffer immediately, not when a timeout cancels the request seconds later. Reviewed By: prakashgayasen, adolfojunior Differential Revision: D108211199 fbshipit-source-id: 2367293289c6d6ba069a1064e1effc852bc2df81github.com-facebook-fbthrift · e78e8221 · 2026-06-11
- 0.6ETVFree RSocket request buffer right after decode Summary: The RSocket server transport (`ThriftServerRSocket`) historically held the request `ByteBuf` and the RSocket `Payload` alive until the response `Mono` completed -- through user-handler execution and response serialization -- even though the request bytes are only needed until the arguments are decoded. For large requests or slow handlers this pinned pooled Netty memory far longer than necessary and showed up as server-side memory spikes. This mirrors the header-transport optimization landed in D106871318 for the RSocket path; RSocket server has no production users yet so this is unconditional (no rollout flag). Mechanism (same shape as D106871318): - `ServerRequestPayload` now owns the framework's reference to the request buffer (when the transport hands one over) and exposes `releaseRequestData()`. `DefaultServerRequestPayload` uses `AtomicReference.getAndSet(null)` so releases are idempotent regardless of how many times the generated handler + transport backstop both call it. - New 4-arg `ServerRequestPayload.create(reader, metadata, ctx, ReferenceCounted)` overload lets the transport pass its owning reference at construction time; existing 3-arg / int-seqId overloads pass `null` and remain a no-op. - `ThriftServerRSocket` constructs a `RequestBuffers extends AbstractReferenceCounted` composite that releases BOTH the decoded request `ByteBuf` and the RSocket `Payload` in a single `deallocate()`. Both refs are required: in the uncompressed case `requestData = payload.sliceData().retain()` -- a retained slice of the same underlying buffer -- so the memory is only freed once both are released. In the compressed case `requestData` is independently allocated. The composite is handed to the payload via the new 4-arg `create(...)`. - All three RSocket entry points (`requestResponse`, `requestStream`, `fireAndForget`) hand the `RequestBuffers` composite to the payload and use the idempotent `payload.releaseRequestData()` as the `doFinally` backstop. The generated `_do<method>` (request-response/oneway) releases the buffers immediately after `getData(...)` via the existing finally-block release that D106871318 added to the whisker template. The streaming dispatcher reads in `StreamResponseHandlerTemplate.handleStream`; this commit adds the same release-after-read pattern there so streams don't pin the buffer for their whole lifetime. Interaction with the `preprocess` admission-control hook (preceding diff in this stack): The `preprocess` hook returns `Mono.just(errorResponsePayload)` (for `TApplicationException`) or `Mono.error(t)` (other Throwables) BEFORE the inner `Mono.defer` is ever subscribed, so the generated `_do<method>` never runs. The transport's outer `doFinally(__ -> finalRequestPayload.releaseRequestData())` is the safety net that catches both paths -- the `RequestBuffers` composite's `deallocate()` fires when the (error) response terminates the chain, freeing both refs. So a shed at `preprocess` time releases the request buffer immediately, not when a timeout cancels the request seconds later. Reviewed By: adolfojunior, robertroeser Differential Revision: D107401072 fbshipit-source-id: 38ccb8359f15f9335c92209cef3bc3abd54f78e0github.com-facebook-fbthrift · aed5c687 · 2026-06-05
- 0.3ETVAlign declared exception messages with C++ Rocket semantics Summary: The C++ Rocket reference always populates name_utf8 and what_utf8 for exception metadata, and generated C++ exceptions always provide a non-null what() fallback. Java was still missing part of that contract: declared exception metadata now sets name_utf8/what_utf8 for both stream-level and function-level responses, and Java exception codegen now guarantees a meaningful getMessage() for exceptions without a literal message field by falling back to the generated class name. This keeps Java aligned with the documented server transport exception contract and with the C++ reference behavior for the three exception-message shapes: - literal message field: use that field - thrift.ExceptionMessage field: use the annotated field - no message field: fall back to the exception class name The rsocket server tests now cover all three cases and verify that declared exception metadata carries the expected what_utf8 values on the wire. Reviewed By: robertroeser Differential Revision: D95857603 fbshipit-source-id: d52672376c976e4c845c9fd85e5d0641d02c27c3github.com-facebook-fbthrift · ced7ee34 · 2026-04-25
- 0.2ETVDelete ClientRuntimeMode plumbing and collapse legacy SR/factory branches Summary: The thrift Java client v2 runtime has been the default in production for a week. This diff removes the `ClientRuntimeMode` enum, all `setClientRuntimeMode` / `getClientRuntimeMode` accessors, both `Config` keys (`thrift.client.runtime` and `servicerouter.client.runtime`), the `-Dthrift.client.runtime` system-property lookup, and `ClientRuntimeSelector`'s mode-resolution methods. With the mode gone, all legacy/v2 dispatch branches collapse. Concrete deletions: - `ClientRuntimeMode.java` — entire enum, no replacement. - `ThriftClientConfig.{get,set}ClientRuntimeMode` and the `Config("thrift.client.runtime")` annotation. - `ServiceRouterProxyClientConfig.{get,set}ClientRuntimeMode`, the `Config("servicerouter.client.runtime")` annotation, and its references in `equals` / `hashCode`. - `RpcResources.getClientRuntimeMode` and `ResourceConfiguration.getClientRuntimeMode` (along with the `thrift.client.runtime` system-property lookup). - `ClientRuntimeSelector.resolve(...)`, `ClientRuntimeSelector.resolveGlobal()`, and the `createSource(Mono, mode)` overload; `createSource(Mono)` simplifies to always build a v2 `BindingRpcClientSource`. - `RpcClientFactory.Builder.buildLegacyFactory()` and the mode-branching in `build()`; `build()` now unconditionally delegates to `RpcClientFactoryV2.builder()`. - `SidecarRpcClientFactory.java` — entire legacy SR transport-factory class. - `useV2`, `resolveRuntimeMode`, and the entire legacy `else` branches in `ServiceRouterV2ClientFactoryImpl`, `ServiceRouterV2Sidecar`, `ServiceRouterV2BindingsClientFactoryImpl`, and `ServiceRouterV2Bindings`; the legacy `PooledRpcClientFactory` / `SidecarRpcClientFactory` fields and constructors collapse to v2-only. - `SidecarRpcClientFactoryV2`'s now-dead `if (config.getClientRuntimeMode() != null)` propagation block. - `ClientRuntimeSelectorTest.testResolveHonorsExplicitConfigOverride` and `testLegacySourceKeepsDisposeNoop` — both tested the deleted methods; the remaining v2 source tests stay (mode-less). - `ClientRuntimeThriftClientTest.testLegacyFactoryBuildKeepsReactiveClientUsableAfterDispose` — removed; `createFactory(ClientRuntimeMode, ...)` helpers collapse to their no-mode equivalents; v2-prefixed test names drop the now-redundant prefix. - `ServiceRouterCLFClientFactoryTest`'s `setUp(testMode, ClientRuntimeMode)` overload + the duplicate `explicitV2*` test variants (each duplicated its non-prefixed counterpart now that V2 is the only mode); the unique borrowed-close-semantics case is kept as `usesBorrowedCloseSemantics`. `MonoBackedRpcClientManager`, `LegacyRpcClientSource`, `RpcClientSource`, the raw-Mono constructors in generated typed clients, and the broader legacy factory chain (`Reconnecting` / `Pooled` / `SimpleLoadBalancing` / `Cached` `*Factory` + `*Mono`) remain for now and are removed in follow-up diffs in this stack. Reviewed By: prakashgayasen Differential Revision: D106403787 fbshipit-source-id: 012dba2ccc67a2d63600dca7f968c2b9d76b3172github.com-facebook-fbthrift · 3c1018f8 · 2026-06-11
- 0.2ETVMigrate test + benchmark Mono-ctor callers to RpcClientFactory.builder() Summary: Replaces direct `new XServiceReactiveClient(protocolId, factory.createRpcClient(addr).cache())` construction across runtime tests, integration tests, and benchmarks with the v2-native pattern `XService.{Reactive,Async}.clientBuilder().setProtocolId(...).build(factory, addr)` where the factory is built via `RpcClientFactory.builder()...build()`. The new path gives manager-backed connection caching natively (via `SingleRpcClientManager`), which is what the prior `.cache()` was approximating. No API change in this diff. The Mono-ctor on generated `*ReactiveClient` classes is still present and still functional; this diff just stops using it from these specific call sites. The subsequent diff in this stack updates the codegen template to stop generating those ctors, and the third diff deletes the underlying shim (`ClientBuilder.build(Mono)`, `ClientRuntimeSelector.createSource(Mono)`, `MonoBackedRpcClientManager`). Also deletes two test files that exercised legacy classes scheduled for removal in a subsequent stack diff: `TestReconnectingRpcClientMono.java` and `FusableReconnectingRpcClientMonoTest.java`. Both test classes (`ReconnectingRpcClientMono`, `FusableReconnectingRpcClientMono`) are part of the legacy factory chain that goes away once the v2 manager stack is the sole runtime; deleting their tests now avoids spending effort migrating Mono-ctor calls in soon-to-be-removed files. Files touched: - `fbcode/thrift/lib/java/runtime/src/test/java/com/facebook/thrift/server/{TestGeneratedServerHandler,TestServerDecoder}.java` - `fbcode/thrift/lib/java/benchmarks/src/main/java/com/facebook/thrift/runner/{ReactiveClient,LoadBalancedReactiveClient,UdsReactiveClient,MultiUdsReactiveClient,RSocketClient,AsyncWrapperClient,BlockingWrapperClient}.java` + `jmh/ReactiveRpcBenchmarks.java` - `fbcode/thrift/facebook/java/swift/swift-service/src/test/java/.../{SimpleReactiveTest,TestGeneratedServerHandler}.java` - `fbcode/thrift/facebook/java/swift/swift-integration-test/src/test/java/.../RSocketClientTest.java` - `fbcode/thrift/facebook/java/swift/swift-benchmarks/src/main/java/com/facebook/swift/runner/{ReactiveClient,RSocketReactiveClient,SimpleStreamClient,RSocketStreamClient,AsyncWrapperClient,BlockingWrapperClient}.java` + `jmh/ReactiveNetty4Benchmarks.java` - Deletions: `TestReconnectingRpcClientMono.java`, `FusableReconnectingRpcClientMonoTest.java` Reviewed By: adolfojunior Differential Revision: D106543247 fbshipit-source-id: 351227ffb9674d409986dae5a9b90a8a276df834github.com-facebook-fbthrift · 9d563543 · 2026-06-11
- 0.2ETVMigrate thrift offloop scheduler metrics to internal HdrHistogram distributions Summary: Migrates the `ThreadPoolScheduler` offloop metrics off the third-party `io.airlift.stats` classes (`Distribution`, `DecayCounter`, `ExponentialDecay`) onto the internal HdrHistogram-based `SingleWindowDistribution` and `CompositeMovingCounter`. Key changes: - `poolSizeAvg`, `pendingTasksAvg`, `activeTasksAvg`, and `executionTime` now use `SingleWindowDistribution`; `getStats()` reads them via `getOneMinuteQuantiles()`. - `completedTasksSum` now uses `CompositeMovingCounter`. Because `ThreadPoolExecutor.getCompletedTaskCount()` is cumulative and `CompositeMovingCounter.add()` expects an increment, `captureThreadPoolExecutorMetrics()` now feeds the per-tick delta and reads `oneMinuteRate()`. Previously the cumulative total was fed into a decaying counter, so `thrift_offloop.complete_tasks.sum.60` did not track real throughput. - Removed the write-only, never-pruned `perThreadExecutionTimes` map. It was never read by `getStats()`, and with the new distribution type each entry would have registered a perpetual sampler that the `WeakReference` cleanup could never reclaim (the map held a strong reference). - `execution_time` now emits the full quantile set (`p50/p75/p90/p95/p99/avg/min/max/sum`) instead of only `avg` (previously `getP50()`) and `p90`. Note `execution_time.avg` now reports the true mean rather than the median. - Renamed `ExecutionRecordingThread.recordExecutionTimeNanos` to `recordExecutionTimeMicros` to match the microsecond value the caller actually passes. - Dropped the now-dead `Double.isInfinite` guards; the new quantile accessors always return finite longs. Reviewed By: adolfojunior Differential Revision: D113069942 fbshipit-source-id: 58e1ad2432b4f9c187162441570f76e75ad562b4github.com-facebook-fbthrift · 380f1920 · 2026-07-21
- 0.2ETVHarden PooledRpcClientManager host discovery (fail-fast on empty, bounded error retry) Summary: Improves the netty-4 `PooledRpcClientManager` -- used by the ServiceRouter sidecar client path (`clf/servicerouter/client/ServiceRouterV2ClientFactoryImpl`, which builds a `PooledRpcClientManagerFactory`) -- to handle host-discovery edge cases the way ServiceRouter/SMC do, instead of hanging: - An authoritative empty resolution is applied immediately: it publishes an empty host set so `acquire()` fails fast with `No hosts available for tier ...`, and does not preserve a stale host set (no dialing hosts that discovery has dropped). - Only transient discovery errors are retried, with a small bounded backoff (~3 attempts). On persistent error the last-known-good pool is kept when one exists (ride out a transient discovery outage); on a first-fetch failure (nothing to fall back to) an empty set is published so callers fail fast instead of blocking until their request timeout. - A selector that completes without emitting is defensively treated as empty. - Discovery failures are logged. - An `isDisposed()` guard closes a dispose-vs-refresh race that the longer error burst widens. Also renames the no-hosts error from `No rpc clients available for tier ...` to `No hosts available for tier ...` to reflect that the cause is empty discovery. This is a standalone upgrade to the existing SR-sidecar client path; the crypto-lib migration stacks on top. Reviewed By: adolfojunior Differential Revision: D109871162 fbshipit-source-id: 7863c9be7d40cb8a7b8550087d9b046f1167d78cgithub.com-facebook-fbthrift · 4254e179 · 2026-06-29
- 0.2ETVFix ByteBuf leak in legacy THeader server on synchronous decode/dispatch failure Summary: `ThriftServerHandler.messageReceived` releases the request `ThriftFrame` only through the reactive response pipeline's `doFinally` (`frame.release()`). `decodeMessage`'s own `catch` only covers a throw inside `decodeMessage` itself. Between them — extracting the metadata and calling `rpcServerHandler.singleRequest*(payload)` inside `handleRequestResponse` / `handleRequestNoResponse` to assemble the response `Mono` — there was no frame-releasing guard. If any of that synchronous work throws (for example a dispatch-time failure, or a generated handler that reads request args on the calling thread and hits a corrupt or oversized body), the exception escapes `messageReceived` before `.subscribe()` engages the `doFinally`, so the frame is never released. Because the underlying buffer is an unpooled no-cleaner direct epoll receive buffer, the native memory leaks permanently and accumulates until `PlatformDependent.usedDirectMemory()` reaches its limit. This was confirmed in production via a Netty `ResourceLeakDetector` record on `GenericIrisProd`: the leaked epoll receive buffer's most recent access was `ByteBufTCompactProtocol.readMessageBegin` inside `ThriftServerHandler.decodeMessage`, with no subsequent `release()`. Fix: wrap the decode and response-assembly section in a `try/catch (Throwable)` that releases the frame (guarded by `refCnt() > 0`, so it is idempotent with `decodeMessage`'s own release), restores the prior `RequestContext`, and re-throws via `Exceptions.propagate(t)` to preserve the existing `exceptionCaught` routing. The async `doFinally` behavior on the success path is unchanged. Leak Suspect: ``` [twshared43337.03.eag3.facebook.com] [2026-05-30 11:53:34,144] [ERROR] LEAK: ByteBuf.release() was not called before it's garbage-collected. See https://netty.io/wiki/reference-counted-objects.html for more information. Recent access records: #1: io.netty.buffer.AdvancedLeakAwareByteBuf.getBytes(AdvancedLeakAwareByteBuf.java:251) com.facebook.thrift.util.Utf8Util.readString(Utf8Util.java:169) com.facebook.thrift.protocol.ByteBufTCompactProtocol.readString(ByteBufTCompactProtocol.java:458) com.facebook.thrift.protocol.ByteBufTCompactProtocol.readMessageBegin(ByteBufTCompactProtocol.java:321) com.facebook.thrift.legacy.server.ThriftServerHandler.decodeMessage(ThriftServerHandler.java:318) com.facebook.thrift.legacy.server.ThriftServerHandler.messageReceived(ThriftServerHandler.java:132) com.facebook.thrift.legacy.server.ThriftServerHandler.channelRead(ThriftServerHandler.java:84) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) com.facebook.thrift.legacy.codec.HeaderTransportCodec.channelRead(HeaderTransportCodec.java:89) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:361) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:325) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.codec.ByteToMessageDecoder.handlerRemoved(ByteToMessageDecoder.java:270) io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:553) io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:484) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.flush.FlushConsolidationHandler.channelRead(FlushConsolidationHandler.java:152) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) com.facebook.thrift.util.MetricsChannelDuplexHandler.channelRead(MetricsChannelDuplexHandler.java:57) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.logging.LoggingHandler.channelRead(LoggingHandler.java:280) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:805) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) #2: io.netty.buffer.AdvancedLeakAwareByteBuf.readByte(AdvancedLeakAwareByteBuf.java:401) com.facebook.thrift.legacy.codec.HeaderTransportCodec.readVarInt32(HeaderTransportCodec.java:336) com.facebook.thrift.legacy.codec.HeaderTransportCodec.readString(HeaderTransportCodec.java:321) com.facebook.thrift.legacy.codec.HeaderTransportCodec.decodeHeaders(HeaderTransportCodec.java:308) com.facebook.thrift.legacy.codec.HeaderTransportCodec.decodeFrame(HeaderTransportCodec.java:277) com.facebook.thrift.legacy.codec.HeaderTransportCodec.channelRead(HeaderTransportCodec.java:81) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:361) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:325) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.codec.ByteToMessageDecoder.handlerRemoved(ByteToMessageDecoder.java:270) io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:553) io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:484) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.flush.FlushConsolidationHandler.channelRead(FlushConsolidationHandler.java:152) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) com.facebook.thrift.util.MetricsChannelDuplexHandler.channelRead(MetricsChannelDuplexHandler.java:57) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.logging.LoggingHandler.channelRead(LoggingHandler.java:280) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:805) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) #3: Hint: 'HeaderTransportCodec#0' will handle the message from this point. io.netty.channel.DefaultChannelPipeline.touch(DefaultChannelPipeline.java:115) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:417) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:361) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:325) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.codec.ByteToMessageDecoder.handlerRemoved(ByteToMessageDecoder.java:270) io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:553) io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:484) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.flush.FlushConsolidationHandler.channelRead(FlushConsolidationHandler.java:152) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) com.facebook.thrift.util.MetricsChannelDuplexHandler.channelRead(MetricsChannelDuplexHandler.java:57) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.logging.LoggingHandler.channelRead(LoggingHandler.java:280) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:805) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) #4: io.netty.buffer.AdvancedLeakAwareByteBuf.retainedSlice(AdvancedLeakAwareByteBuf.java:95) com.facebook.thrift.legacy.server.ThriftHeaderFrameLengthBasedDecoder.decode(ThriftHeaderFrameLengthBasedDecoder.java:146) com.facebook.thrift.legacy.server.ThriftHeaderFrameLengthBasedDecoder.decode(ThriftHeaderFrameLengthBasedDecoder.java:79) io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:545) io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:484) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.codec.ByteToMessageDecoder.handlerRemoved(ByteToMessageDecoder.java:270) io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:553) io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:484) io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.flush.FlushConsolidationHandler.channelRead(FlushConsolidationHandler.java:152) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) com.facebook.thrift.util.MetricsChannelDuplexHandler.channelRead(MetricsChannelDuplexHandler.java:57) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.logging.LoggingHandler.channelRead(LoggingHandler.java:280) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:805) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) #5: Hint: 'MetricsChannelDuplexHandler#0' will handle the message from this point. io.netty.channel.DefaultChannelPipeline.touch(DefaultChannelPipeline.java:115) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:417) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.handler.logging.LoggingHandler.channelRead(LoggingHandler.java:280) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:805) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) #6: Hint: 'LoggingHandler#0' will handle the message from this point. io.netty.channel.DefaultChannelPipeline.touch(DefaultChannelPipeline.java:115) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:417) io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:805) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) #7: Hint: 'DefaultChannelPipeline$HeadContext#0' will handle the message from this point. io.netty.channel.DefaultChannelPipeline.touch(DefaultChannelPipeline.java:115) io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:417) io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:805) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) Created at: io.netty.buffer.UnpooledByteBufAllocator.newDirectBuffer(UnpooledByteBufAllocator.java:96) io.netty.buffer.AbstractByteBufAllocator.directBuffer(AbstractByteBufAllocator.java:188) io.netty.buffer.AbstractByteBufAllocator.directBuffer(AbstractByteBufAllocator.java:179) io.netty.channel.unix.PreferredDirectByteBufAllocator.ioBuffer(PreferredDirectByteBufAllocator.java:53) io.netty.channel.DefaultMaxMessagesRecvByteBufAllocator$MaxMessageHandle.allocate(DefaultMaxMessagesRecvByteBufAllocator.java:120) io.netty.channel.epoll.EpollRecvByteAllocatorHandle.allocate(EpollRecvByteAllocatorHandle.java:75) io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:790) io.netty.channel.epoll.EpollDomainSocketChannel$EpollDomainUnsafe.epollInReady(EpollDomainSocketChannel.java:138) io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501) io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399) io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) java.base/java.lang.Thread.run(Thread.java:1474) : 32 leak records were discarded because the leak record count is targeted to 4. Use system property io.netty.leakDetection.targetRecords to increase the limit. (thread: thrift-eventloop-41) (logger: io.netty.util.ResourceLeakDetector) (twuser: iris) (twcluster: tsp_eag) (twjobname: GenericIrisProd) (twtaskid: 630) ``` Reviewed By: jvshahid Differential Revision: D107022950 fbshipit-source-id: 3c42feae53ee6003d1f345423e50c77d5045616cgithub.com-facebook-fbthrift · 0ec22d88 · 2026-06-02
- 0.2ETVMigrate legacy THeader client off deprecated reactor MonoProcessor to Sinks Summary: `reactor.core.publisher.MonoProcessor` is deprecated and slated for removal in Reactor 3.5; the recommended replacement is `Sinks.One` / `Sinks.Empty` (see `MonoProcessor.create()` javadoc: "Use `Sinks#one()`"). This migrates all `MonoProcessor` usage in the legacy THeader client (`RequestContext`, `ThriftClientHandler`, `LegacyRpcClient`) to `Sinks`. The migration is behavior-preserving: - `RequestContext.future` becomes `Sinks.One<R>` (oneway uses `Sinks.One<Void>`; only its inherited `Sinks.Empty` emit methods are used). Subscriptions use `processor.asMono()`. - `processor.onNext(value)` becomes `processor.emitValue(value, FAIL_FAST)`. This is faithful: a `Mono` `onNext` is terminal — `NextProcessor.onNext(value)` completes all subscribers with the value — so the existing `doFinally(... frame.release())` already ran on success, and `emitValue` preserves that. - `processor.onComplete()` / `processor.onError(t)` become `emitEmpty(FAIL_FAST)` / `emitError(t, FAIL_FAST)`. - `Sinks.EmitFailureHandler.FAIL_FAST` drops late/double terminals instead of throwing (`InternalEmptySink.emitError` routes `FAIL_TERMINATED` to `Operators.onErrorDropped`), matching the previous `MonoProcessor` semantics that `clearWithException` and `channelInactive` rely on. It only throws on `FAIL_NON_SERIALIZED` (concurrent emit), which cannot occur here since all handler callbacks run on the channel`s single Netty event loop. - `onClose` becomes `Sinks.Empty<Void>` fed by an explicit subscriber on the channel close future; `isDisposed()` is backed by a `closed` flag set on terminal, preserving the prior `MonoProcessor.isDisposed()` contract. No dependency or source-layout changes, so no BUCK/pom updates are required. Reviewed By: adolfojunior Differential Revision: D107117728 fbshipit-source-id: 627f97a0e91d00f59be47f1791fa4f449815dc31github.com-facebook-fbthrift · 2aa980e1 · 2026-06-02
- 0.1ETVPopulate unified RSocket connection context Summary: Thread the per-connection Nifty context through the Thrift RSocket acceptor and server socket. Build the unified RSocket acceptor inside the per-connection branch so it uses the accepted Reactor Netty connection remote address and SSL session, matching the header transport context. Reviewed By: adolfojunior Differential Revision: D109503107 fbshipit-source-id: 62ed375dccfd60e99a2ad15bb055de2d133add7agithub.com-facebook-fbthrift · 7c1b0709 · 2026-06-24
- 0.1ETVSplit RpcClientFactory: introduce RpcClientTransportFactory for raw-transport tier Summary: The legacy `RpcClientFactory` interface had a double role: callers used it both as a raw connection producer (`createRpcClient(SocketAddress) -> Mono<RpcClient>`) and as a builder of fully-configured client bindings (`createRpcClientBinding(SocketAddress) -> RpcClientBinding`). After the v2 manager-backed runtime became the default, the binding-producing role is the only legitimate use for the public-facing `RpcClientFactory`; the transport-producing role belongs to lower-level building blocks (`LegacyRpcClientFactory`, `RSocketRpcClientFactory`, `HeaderAwareRpcClientFactory`, `EventHandlerRpcClientFactory`, `InstrumentedRpcClientFactory`, `TokenPassingRpcClientFactory`, `TimeoutRpcClientFactory`, `DelegatingRpcClientFactory`) and managers (`SingleRpcClientManager`, `ReconnectingRpcClientManager`). This diff makes that split explicit: - Adds `RpcClientTransportFactory` as a `FunctionalInterface` over `createRpcClient(SocketAddress)`. All raw-transport factories now `implement RpcClientTransportFactory` instead of `RpcClientFactory`. - `RpcClientFactory` keeps only `createRpcClientBinding(SocketAddress) -> RpcClientBinding` and the `Builder` API. The unused `createRpcClient` method (which had become a UOE-only shim in `RpcClientFactoryV2`) is gone. - Managers (`SingleRpcClientManager`, `ReconnectingRpcClientManager`) and their factories now accept `RpcClientTransportFactory` directly. - `ClientBuilder` gets a third `build(RpcClientTransportFactory, SocketAddress)` overload that wires a `SingleRpcClientManager` for callers who construct a transport factory directly (kept to avoid forcing every caller through `RpcClientFactory.builder()`). - The `ThriftRpcClientFactoryBuilder` in `clf/client/java` had a now-broken inner `CachedRpcClientFactory` wrapper (would have thrown UOE on every call after the binding-only switch). Removed; per-address caching is provided by `SingleRpcClientManager`. - Tests that mocked or declared `RpcClientFactory` to mean "raw transport" updated to `RpcClientTransportFactory`. After this diff `RpcClientFactory` and `RpcClientTransportFactory` express two distinct, non-overlapping responsibilities. Next diff renames `RpcClientFactoryV2` -> `RpcClientFactory` and collapses the now-thin interface into the class. Reviewed By: robertroeser, adolfojunior Differential Revision: D106530905 fbshipit-source-id: e500a0c845859c3843f6b2f77a60602a5b9de6bdgithub.com-facebook-fbthrift · ae4401a3 · 2026-06-11
- 0.1ETVCollapse RpcClientFactoryV2 into RpcClientFactory Summary: After the previous diff split off `RpcClientTransportFactory` for the raw-transport tier, the public `RpcClientFactory` interface had a single abstract method (`createRpcClientBinding`) and the single implementation `RpcClientFactoryV2` was the only thing anyone ever instantiated. The interface/impl split was a leftover from the v1/v2 cutover; once the v2 runtime became the default, the indirection stopped paying for itself. This diff folds `RpcClientFactoryV2` into `RpcClientFactory`: - `RpcClientFactory` becomes a `final` class in `com.facebook.thrift.client` holding the manager factory and exposing `createRpcClientBinding(SocketAddress)` directly. - The `Builder` inner class is the same one callers already use (same setters, same defaults); it now also exposes `buildManagerFactory()` for SR-proxy callers that compose their own manager pipeline on top. - `RpcClientFactoryV2.java` is deleted. - Callers in `servicerouter/client/java`, `servicerouter/client/java_bindings`, and `ClientRuntimeThriftClientTest` updated to use `RpcClientFactory.builder()` / `RpcClientFactory.Builder` directly. Safe to flatten because no code outside this file implemented `RpcClientFactory` (only `RpcClientFactoryV2` did), no caller held an `RpcClientFactoryV2`-typed reference (they only used the static `.builder()`), and the v2 manager-backed runtime has been the only path for a week+. Reviewed By: adolfojunior Differential Revision: D106544313 fbshipit-source-id: 5029867e2b09ef8373deb1a3c789778aec789254github.com-facebook-fbthrift · 4c6ae53c · 2026-06-11
- 0.1ETVSet declared exception name in request-response replies Summary: Pure request-response declared exceptions were sent over the wire with only the `whatUtf8` field populated. The Rocket server-transport contract — the same one that D95857603 enforced for streams — requires both `nameUtf8` (the qualified exception class name, surfaced as the `uex` header) and `whatUtf8` (the message, surfaced as `uexw`) on every declared exception payload. Without `nameUtf8`, clients can read the message but cannot key on the exception type from headers, which breaks parity with C++ and with the stream path. This diff adds a 4-arg overload to `RpcPayloadUtil.createServerResponsePayload(payload, writer, name, what)` and updates the `RpcServerHandler.whisker` template to pass `_t.getClass().getName()` for declared exceptions thrown from request-response handlers. The 3-arg overload (used elsewhere) still routes through the new code path with `name == null`, preserving existing behavior for callers that don't provide a name. `PayloadExceptionMetadataBase.Builder` initializes its fields to `null` and its setters simply assign — so `setNameUtf8(null)` is equivalent to never calling it. The 4-arg overload uses the chained-builder form directly without inner null checks. Affected generated handlers were regenerated via `buck2 run fbsource//xplat/thrift/compiler/test:build_fixtures`. Reviewed By: adolfojunior Differential Revision: D102438521 fbshipit-source-id: 1aa0169c350b38480b5f29455699e997850ac5ffgithub.com-facebook-fbthrift · 9f5fc8ba · 2026-04-28