Kirill Sazonov

This commit introduces a **new end-to-end test** (`BiDiClientCrashE2ETest`) to validate the **Thrift BiDi streaming** server's resilience. It verifies that server-side handler coroutines promptly unblock when a client process crashes mid-stream, preventing indefinite hangs. The test achieves this by spawning a separate `BiDiCrashClient` binary that intentionally sends data and then self-terminates via `SIGKILL`, accurately simulating a process-level TCP socket close. This **enhances server robustness** by ensuring graceful handling of unexpected client disconnections and preventing resource exhaustion in the **Thrift** framework.

This commit **refactors** the **`RoundRobinRequestPileOptions`** configuration object within the `thrift/lib/cpp2/server` component by converting it from a `struct` with public fields to a `class` with private fields and public getter/setter methods. This change significantly **improves encapsulation** and clarifies the API surface for configuring the request pile. All internal `RoundRobinRequestPile` methods and external callers, including various test suites, have been updated to use the new `get*()` and `set*()` accessors. This **enhances maintainability** and enforces proper object-oriented design for the server's request handling logic.

This commit **refactors the `RoundRobinRequestPileOptions` API** within the **`thrift/lib/cpp2/server`** component by **removing the two-argument constructor** `RoundRobinRequestPileOptions(vector<uint32_t>, PileSelectionFunction)`. All existing call sites, which were exclusively in test and benchmark files like `RoundRobinRequestPileBenchmark.cpp` and `RoundRobinRequestPileTest.cpp`, have been migrated to use the default constructor followed by `setShape()` and `setPileSelectionFunction()` setters. This **API simplification** promotes a more consistent and extensible way to configure `RoundRobinRequestPileOptions`. To maintain conciseness in the updated test code, a `makeOpts()` helper function was introduced in the test fixture.

This commit **enhances documentation** by adding a comprehensive class-level docblock to `thrift/lib/cpp2/server/CPUConcurrencyController.h`. The new documentation details the intricate algorithm used for **Thrift C++2 server concurrency control**, explaining its three-phase control loop (bootstrap, overload, underload), AIMD-like behavior, and enforcement methods. It also includes an ASCII control flow diagram for the `cycleOnce()` function, significantly improving developer understanding of this critical component's operation. This **documentation update** provides crucial insights into the controller's adaptive behavior without altering its runtime functionality.

This commit **significantly expands test coverage** for the **`CPUConcurrencyController`** within `thrift/lib/cpp2/server/test/CPUConcurrencyControllerTest.cpp`. It introduces a **comprehensive suite of unit tests**, increasing coverage from 3 to 37 tests, to validate the controller's core functionalities. The new tests cover critical behaviors such as `requestStarted`/`requestShed` logic, custom `LoadFunc` handling, overload/underload limit adjustments, bootstrap stability, refractory period suppression, and the `DRY_RUN` mode. This **enhances the robustness and reliability** of the **server's CPU concurrency management**, ensuring correct operation across diverse configurations and load scenarios.

This commit introduces a comprehensive suite of **end-to-end tests** to verify the **graceful shutdown** behavior of `ThriftServer` when active **streaming RPCs** (streams, sinks, and BiDi operations) are in progress. It addresses a significant **test coverage gap** by ensuring that `ThriftServer` properly terminates or handles in-flight streaming requests during shutdown, preventing data inconsistencies or hung connections. The new tests, defined in `thrift/lib/cpp2/test/e2e/GracefulShutdownE2ETest.cpp` and using `TestGracefulShutdownService.thrift`, validate scenarios like active stream interruption, new RPC rejection post-shutdown, and concurrent stream termination. This **quality assurance** improvement enhances the reliability of `ThriftServer` for applications utilizing advanced RPC patterns.

This commit introduces a comprehensive **end-to-end test suite** for **ThriftServer resource pool routing**, addressing a critical P0 gap in test coverage. The new tests validate the complete routing pipeline, ensuring correct behavior for scenarios like **concurrency limits**, **custom pool routing** via `selectResourcePool()`, and **priority-based execution**. This **new capability** significantly improves confidence in the reliability and robustness of the `ThriftServer`'s resource management and request dispatching mechanisms. The suite includes tests for dynamic limit changes and isolation between different resource pools, using new infrastructure like `RoutingProcessorFactory` and `BlockableHandler`.

This commit introduces a **comprehensive E2E test suite** for **Thrift stream and sink backpressure mechanisms** within the `thrift/lib/cpp2/test/e2e` module. It specifically verifies credit-based and memory-based flow control, chunk timeouts, stream expiry, and bidirectional interactions over the **Rocket transport**. This **new test coverage** addresses a critical gap, ensuring the reliability and correctness of these flow control features end-to-end. The suite includes various scenarios, such as slow consumers/servers and dynamic credit adjustments, to validate the system's resilience and prevent deadlocks. A new Thrift service, `TestBackpressureService`, was added to facilitate these tests.

This commit **fixes a flaky test** within the **ThriftServerTest** suite, specifically addressing the `ResponseWriteTimeout` scenario. The test was unreliable because its `SlowReadCallback` only throttled reads delivered via `readDataAvailable`, allowing other read paths like `readBufferAvailable` and `readZeroCopyDataAvailable` (used by transports such as `AsyncIoUringSocket`) to bypass the intended delay. To resolve this, a 100ms sleep was added to `readBufferAvailable` and `readZeroCopyDataAvailable` within the test's callback. This ensures consistent read throttling across all underlying I/O delivery mechanisms, making the **test robust** and reliably verifying the **response write timeout** functionality.

This commit **refactors** the **Thrift C++ server's request piling mechanism** by replacing `WeightedRequestPileQueue` with a new, simpler `SingleBucketRequestPileQueue` within `RoundRobinRequestPile`. The previous queue introduced unnecessary complexity and overhead, as its weighted functionality was unused in this context. The new `SingleBucketRequestPileQueue` provides a streamlined MPMC queue with optional, efficient count-based limiting, **reducing complexity and improving performance** for single-bucket request handling. This change optimizes the `RoundRobinRequestPile` by removing unused features and associated overheads, ensuring a more efficient request admission control system. New unit tests for `SingleBucketRequestPileQueue` are also included to validate its behavior.

This commit **refactors** the `addInternalPriorities()` function, moving it from a static method on `RoundRobinRequestPile` to a `const` method on the `RoundRobinRequestPile::Options` struct. This change improves **code organization** by placing the method closer to the `Options` data it exclusively operates on, making the **API for configuring request priorities** more intuitive. All call sites within the **Thrift server's request piling mechanism**, specifically in `ThriftServer.cpp` and various test files, have been updated to use the new method signature. The method now returns a new `Options` object, maintaining its previous by-value parameter semantics. This is a **maintenance refactoring** with no functional changes to the server's request handling.

This commit performs a significant **refactoring** within the **`thrift/lib/cpp2/server`** module by **extracting the nested `Options` struct** from `RoundRobinRequestPile` into a new, standalone `RoundRobinRequestPileOptions` struct. This change **decouples the configuration details** from the `RoundRobinRequestPile` implementation, allowing its options to be defined and used independently. A `using Options = RoundRobinRequestPileOptions;` alias is introduced within the original class to ensure **full backward compatibility** for all existing callers. The `PileSelectionFunction` definition was also moved as part of this modularization effort, enhancing the overall design of the request piling mechanism.

This commit significantly **expands unit test coverage** for the **Thrift C++2 server's `ResourcePoolSet` and `ResourcePool` components**, addressing critical gaps in validation. It introduces 19 new tests in `ResourcePoolSetTest.cpp` to thoroughly verify **lock semantics**, **pool lookup**, **statistics**, and **priority mapping** behaviors of `ResourcePoolSet`. Furthermore, `ResourcePoolsTest.cpp` gains two new tests and refactors existing ones to validate **request queuing** and **concurrency limit adjustments** within `ResourcePool`. This **maintenance effort** ensures greater reliability and correctness for the server's **resource management and concurrency control mechanisms**.

This commit **fixes flakiness** in the **Thrift C++2 Rocket client transport tests**, specifically `KeepAliveWatcherLargeRequestTest` and `KeepAliveEvbDetachAttachTest`. These tests, located in `thrift/lib/cpp2/transport/rocket/client/test/RocketClientTest.cpp`, were intermittently failing on fast CI machines because large data transfers sometimes completed before the intended keep-alive timeout could trigger. To resolve this **test maintenance** issue, the payload size was increased from 20MB to 40MB and the keep-alive timeout was decreased from 50ms to 25ms. This adjustment ensures the timeout reliably fires as expected, significantly improving **CI reliability** for these critical timing-sensitive tests.

This commit **fixes a compilation error** by **renaming non-standard iterator type aliases** within the **`BaseIter` and `BaseMapIter` classes** in `thrift/lib/cpp2/type/detail/Runtime.h`. Previously, `reference_type` and `pointer_type` prevented `std::iterator_traits` from correctly deducing iterator properties, leading to **compile failures** when `gmock` attempted to instantiate matchers on `MapIter`. By updating these to the standard `reference` and `pointer` and adjusting their usage in `operator->` and `operator*`, this **bug fix** ensures **correct `std::iterator_traits` behavior** and resolves these critical compilation issues for **Thrift's C++2 type system iterators**.

This commit **updates the conformance test configuration** for the **Go Thrift server** by removing `InteractionFactoryFunction` related tests from the `nonconforming.txt` list. Previously, these tests were incorrectly listed despite the Go Thrift server already correctly implementing `InteractionFactoryFunction`, `InteractionFactoryFunctionPersistsState`, and `InteractionFactoryFunctionTermination`. This **maintenance** change resolves an issue where the conformance test harness reported failures because it expected listed tests to fail. The update ensures **accurate reporting of the Go Thrift server's compliance** with these interaction patterns, reflecting its full support for these features.

This commit **re-enables approximately 30 previously disabled `rpc-client-test` conformance tests** for the **Hack**, **Java2**, and **Rust** Thrift clients. These tests, which cover critical RPC functionalities like request-response, streaming, sinks, and exception handling, now pass locally, indicating improved stability and correctness in these client implementations. The work involves **updating `thrift/conformance/data/nonconforming.txt`** to remove entries for the now-passing tests and adding a comment for clarity. This is a **maintenance** effort that significantly **improves test coverage and confidence** in the `rpc-client-test` suite by resolving persistent test failures.

This commit **comprehensively expands and refactors the benchmark suite** for the **`RoundRobinRequestPile`** component within the **`thrift/lib/cpp2/server` module**. It introduces 20 new benchmarks across 9 distinct groups, covering critical code paths such as concurrent enqueue/dequeue, multi-bucket configurations, priority handling, contention, and pre-enqueue filtering, replacing a minimal existing suite. This **new capability** establishes a **robust performance baseline** for `RoundRobinRequestPile`, which is crucial for guiding and validating an **upcoming queue abstraction refactoring**. Additionally, it refactors the benchmark code by extracting a `runConcurrentBenchmark()` helper and correcting `BenchmarkSuspender` usage for more accurate timing.

This commit **adds comprehensive test cases** to the `RoundRobinRequestPile` component, significantly **enhancing test coverage** for the **Thrift C++2 server's request scheduling mechanism**. New tests validate critical code paths for **request expiration delegates**, the `getDefaultPileSelectionFunc` which handles diverse priority-based pile selection, and the `addInternalPriorities` logic for managing per-priority limits. This **testing improvement** ensures the robustness and correctness of the server's request prioritization and handling, bolstering confidence in its core infrastructure.

This commit **adds comprehensive unit tests** for the **`RoundRobinRequestPile`** component within `thrift/lib/cpp2/server`. It introduces 17 new tests to significantly **improve test coverage** of various edge cases and complex interactions, such as empty pile dequeue behavior, mixed priority shapes, strict priority ordering, request counting, debug information, and diverse callback and pre-enqueue filter interactions. This **preventative maintenance** ensures the robustness and correctness of the request pile's behavior across critical scenarios, preparing it for a planned refactoring. The enhanced testing provides greater confidence in the **Thrift server's request handling logic**.