Skip to content

Commit a1a273b

Browse files
sazonovkirillmeta-codesync[bot]
authored andcommitted
Fix flaky OverloadTest::Test by using Baton synchronization
Summary: The test was flaky because it used a race-prone polling loop with `std::this_thread::yield()` to wait for a request to become active. This approach had several issues: 1. No timeout/deadline - could loop infinitely in edge cases 2. Race condition - checking `getActiveRequests() >= 1` doesn't guarantee the request has actually entered the blocking handler 3. Very short queue timeout (10ms) that could race with the polling Fix by using explicit `folly::Baton` synchronization: - Added an `entered` baton to `BlockInterface` that signals when a request has entered `voidResponse()` - Use `try_wait_for(5s)` instead of spin-waiting with yield - Increased queue timeout to 1s since synchronization is now explicit Reviewed By: tlj77 Differential Revision: D94309711 fbshipit-source-id: 9cafb07a8ef1a4682f330c5d4bac17387b730800
1 parent 9e4ddd3 commit a1a273b

1 file changed

Lines changed: 12 additions & 13 deletions

File tree

third-party/thrift/src/thrift/lib/cpp2/test/server/ThriftServerTest.cpp

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2057,20 +2057,24 @@ INSTANTIATE_TEST_CASE_P(
20572057
HeaderOrRocket,
20582058
testing::Values(TransportType::Header, TransportType::Rocket));
20592059

2060-
// DO_BEFORE(aristidis,20250716): Test is flaky. Find owner or remove.
2061-
TEST_P(OverloadTest, DISABLED_Test) {
2060+
TEST_P(OverloadTest, Test) {
20622061
class BlockInterface : public apache::thrift::ServiceHandler<TestService> {
20632062
public:
20642063
folly::Baton<> block;
2065-
void voidResponse() override { block.wait(); }
2064+
folly::Baton<> entered;
2065+
void voidResponse() override {
2066+
entered.post();
2067+
block.wait();
2068+
}
20662069

20672070
void async_eb_eventBaseAsync(
20682071
HandlerCallbackPtr<std::unique_ptr<::std::string>> callback) override {
20692072
callback->appOverloadedException("method loadshedding request");
20702073
}
20712074
};
20722075

2073-
ScopedServerInterfaceThread runner(std::make_shared<BlockInterface>());
2076+
auto handler = std::make_shared<BlockInterface>();
2077+
ScopedServerInterfaceThread runner(handler);
20742078
folly::EventBase base;
20752079
auto client = makeClient(runner, &base);
20762080

@@ -2094,20 +2098,15 @@ TEST_P(OverloadTest, DISABLED_Test) {
20942098
return {};
20952099
});
20962100

2097-
// force overloaded
20982101
folly::Function<void()> onExit = [] {};
20992102
auto guard = folly::makeGuard([&] { onExit(); });
21002103
if (errorType == ErrorType::Overload) {
2101-
// Thrift is overloaded on max requests
21022104
runner.getThriftServer().setMaxRequests(1);
2103-
runner.getThriftServer().setQueueTimeout(10ms);
2104-
auto handler = dynamic_cast<BlockInterface*>(
2105-
runner.getThriftServer().getProcessorFactory().get());
2105+
runner.getThriftServer().setQueueTimeout(1s);
21062106
client->semifuture_voidResponse();
2107-
while (runner.getThriftServer().getActiveRequests() < 1) {
2108-
std::this_thread::yield();
2109-
}
2110-
onExit = [handler] { handler->block.post(); };
2107+
ASSERT_TRUE(handler->entered.try_wait_for(5s))
2108+
<< "Timed out waiting for blocking request to enter handler";
2109+
onExit = [&handler] { handler->block.post(); };
21112110
}
21122111

21132112
RpcOptions rpcOptions;

0 commit comments

Comments
 (0)