Skip to content

Commit b65c8da

Browse files
Vitaly Berovmeta-codesync[bot]
authored andcommitted
Add FIFO fragmentation backpressure
Summary: FIFO queues data for later EventBase callbacks, but it ignores downstream `Backpressure`. It never registers for write-ready and never reports the pressure upstream. It also records fragments after the downstream call, so an inline batch completion could run before its tracker entry existed. This diff makes FIFO handle `Backpressure` like the current SRPT handler and yielding SRPT. FIFO keeps sending in its normal callbacks and limits, registers for write-ready, and reports `Backpressure` upstream until write-ready. Pass-through frames follow the same rule. When the socket drains, the pipeline wakes each handler that got `Backpressure` once, in the order they got it. So a handler above FIFO can wake first, while FIFO is still blocked. After this diff, FIFO asks for one more round when it unblocks, so that handler can send. It asks only if it answered `Backpressure` itself, for a write that the socket took or that FIFO queued. A writer that the socket blocked waits for the next drain. So a full socket gets one round per drain. Like yielding SRPT, FIFO now records each fragment before the downstream call. It removes that record if the write returns `Error`. The benchmark handlers remain disconnected from production pipelines. Graceful shutdown is unchanged. Reviewed By: robertroeser Differential Revision: D124885417 fbshipit-source-id: 51395eea2fab4f6513eaa4c8226cc00ffd80bdc7
1 parent de2067d commit b65c8da

3 files changed

Lines changed: 752 additions & 27 deletions

File tree

‎thrift/lib/cpp2/fast_thrift/frame/write/handler/FifoFrameFragmentationHandler.h‎

Lines changed: 78 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <folly/ExceptionWrapper.h>
2626
#include <folly/Utility.h>
2727

28+
#include <thrift/lib/cpp2/fast_thrift/channel_pipeline/Backpressure.h>
2829
#include <thrift/lib/cpp2/fast_thrift/channel_pipeline/Common.h>
2930
#include <thrift/lib/cpp2/fast_thrift/channel_pipeline/Event.h>
3031
#include <thrift/lib/cpp2/fast_thrift/channel_pipeline/TypeErasedBox.h>
@@ -48,6 +49,12 @@ namespace apache::thrift::fast_thrift::frame::write::handler {
4849
* turns as there are live streams when it starts, and each turn sends one
4950
* fragment. The callback starts no new turn after its budget reaches zero. The
5051
* turn that crosses the budget still sends its whole fragment.
52+
* Backpressure does not stop a callback or delay the next one. After
53+
* downstream returns Backpressure, `onWrite()` returns Backpressure for each
54+
* accepted frame until write-ready. If downstream took one of those frames
55+
* without Backpressure, or the handler queued it, the handler asks for another
56+
* write-ready round. A writer above that already ran in the round can then
57+
* write.
5158
*
5259
* A frame that cannot be fragmented goes downstream at once. It can reach the
5360
* peer before a queued data frame of its stream. `CANCEL` and `ERROR` first
@@ -58,6 +65,8 @@ namespace apache::thrift::fast_thrift::frame::write::handler {
5865
template <FragmentCompletionTracker Tracker = NoOpFragmentCompletionTracker>
5966
class FifoFrameFragmentationHandlerT : private folly::NonCopyableNonMovable {
6067
public:
68+
channel_pipeline::WriteReadyHook writeReadyHook_;
69+
6170
explicit FifoFrameFragmentationHandlerT(
6271
QuantumFragmentationHandlerConfig config = {}) noexcept
6372
: state_(config) {}
@@ -75,7 +84,8 @@ class FifoFrameFragmentationHandlerT : private folly::NonCopyableNonMovable {
7584
}
7685

7786
template <typename Context>
78-
void handlerRemoved(Context& /*ctx*/) noexcept {
87+
void handlerRemoved(Context& ctx) noexcept {
88+
cancelAwaitWriteReady(ctx);
7989
state_.detach();
8090
}
8191

@@ -94,17 +104,30 @@ class FifoFrameFragmentationHandlerT : private folly::NonCopyableNonMovable {
94104
const uint32_t streamId = frame.streamId;
95105
return passThrough(ctx, std::move(msg), streamId);
96106
}
97-
if (state_.enqueue(msg.take<ComposedFrame>())) {
98-
return channel_pipeline::Result::Success;
107+
if (!state_.enqueue(msg.take<ComposedFrame>())) {
108+
return channel_pipeline::Result::Error;
99109
}
100-
return channel_pipeline::Result::Error;
110+
return acceptedResult();
101111
}
102112

103113
template <typename Context>
104-
void onWriteReady(Context& /*ctx*/) noexcept {}
114+
void onWriteReady(Context& ctx) noexcept {
115+
const bool blockedWriterAbove = blockedWriterAbove_;
116+
cancelAwaitWriteReady(ctx);
117+
// A round wakes each handler once. A writer above may have run earlier in
118+
// this round and got Backpressure only from this handler. That writer needs
119+
// another round now. A writer that got Backpressure from downstream waits
120+
// for the next write-ready of downstream. If it got a round here too, a
121+
// writer that writes in every round would keep getting rounds while the
122+
// transport stays full.
123+
if (blockedWriterAbove) {
124+
ctx.pipeline()->onWriteReady();
125+
}
126+
}
105127

106128
template <typename Context>
107-
void onPipelineInactive(Context& /*ctx*/) noexcept {
129+
void onPipelineInactive(Context& ctx) noexcept {
130+
cancelAwaitWriteReady(ctx);
108131
state_.clearPending();
109132
}
110133

@@ -134,6 +157,8 @@ class FifoFrameFragmentationHandlerT : private folly::NonCopyableNonMovable {
134157
return state_.isScheduled();
135158
}
136159

160+
[[nodiscard]] bool isBackpressured() const noexcept { return backpressured_; }
161+
137162
Tracker& tracker() noexcept { return tracker_; }
138163

139164
using PublishedEvents = typename Tracker::PublishedEvents;
@@ -145,22 +170,52 @@ class FifoFrameFragmentationHandlerT : private folly::NonCopyableNonMovable {
145170
}
146171

147172
private:
173+
template <typename Context>
174+
void awaitWriteReady(Context& ctx) noexcept {
175+
backpressured_ = true;
176+
ctx.awaitWriteReady();
177+
}
178+
179+
template <typename Context>
180+
void cancelAwaitWriteReady(Context& ctx) noexcept {
181+
if (backpressured_) {
182+
ctx.cancelAwaitWriteReady();
183+
backpressured_ = false;
184+
}
185+
blockedWriterAbove_ = false;
186+
}
187+
188+
channel_pipeline::Result acceptedResult() noexcept {
189+
if (!backpressured_) {
190+
return channel_pipeline::Result::Success;
191+
}
192+
blockedWriterAbove_ = true;
193+
return channel_pipeline::Result::Backpressure;
194+
}
195+
148196
template <typename Context>
149197
[[nodiscard]] channel_pipeline::Result passThrough(
150198
Context& ctx,
151199
channel_pipeline::TypeErasedBox&& msg,
152200
uint32_t streamId) noexcept {
201+
tracker_.onFragment(streamId, true);
153202
const channel_pipeline::Result result = ctx.fireWrite(std::move(msg));
154-
if (result != channel_pipeline::Result::Error) {
155-
tracker_.onFragment(streamId, true);
203+
if (result == channel_pipeline::Result::Error) {
204+
tracker_.onFragmentAborted();
205+
return result;
206+
}
207+
if (result == channel_pipeline::Result::Backpressure) {
208+
awaitWriteReady(ctx);
209+
return result;
156210
}
157-
return result;
211+
return acceptedResult();
158212
}
159213

160214
template <typename Context>
161215
void flushAndPropagateErrors(Context& ctx) noexcept {
162216
if (flushOnePass(ctx) == channel_pipeline::Result::Error) {
163217
state_.stopAfterError();
218+
cancelAwaitWriteReady(ctx);
164219
ctx.fireException(
165220
folly::make_exception_wrapper<std::runtime_error>(
166221
"FifoFrameFragmentationHandler: downstream write failed"));
@@ -177,19 +232,27 @@ class FifoFrameFragmentationHandlerT : private folly::NonCopyableNonMovable {
177232
if (std::optional<FifoStreamQueue::ScheduledFragment> scheduled =
178233
state_.nextFragment();
179234
scheduled.has_value()) {
180-
if (const channel_pipeline::Result result = ctx.fireWrite(
181-
channel_pipeline::erase_and_box(
182-
std::move(scheduled->fragment.outFrame)));
183-
result == channel_pipeline::Result::Error) {
184-
return result;
185-
}
186235
tracker_.onFragment(
187236
scheduled->streamId, scheduled->fragment.currentFrameDone);
237+
const channel_pipeline::Result result = ctx.fireWrite(
238+
channel_pipeline::erase_and_box(
239+
std::move(scheduled->fragment.outFrame)));
240+
if (result == channel_pipeline::Result::Error) {
241+
tracker_.onFragmentAborted();
242+
return result;
243+
}
244+
if (result == channel_pipeline::Result::Backpressure) {
245+
awaitWriteReady(ctx);
246+
}
188247
}
189248
}
190249
return channel_pipeline::Result::Success;
191250
}
192251

252+
bool backpressured_{false};
253+
// Set when onWrite() answers Backpressure for a frame that downstream took or
254+
// the handler queued. The writer above then waits for this handler alone.
255+
bool blockedWriterAbove_{false};
193256
[[no_unique_address]] Tracker tracker_{};
194257
FifoFrameFragmentationHandlerState state_;
195258
};

‎thrift/lib/cpp2/fast_thrift/frame/write/handler/FragmentCompletionTracker.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,9 @@ concept FrameWriteCompleteEventFactory =
8181
/**
8282
* Maps downstream batch completions to original frames.
8383
*
84-
* `onFragment()` appends {streamId, isLastFragment} to a FIFO. A caller that
85-
* records after the downstream write calls it only for a non-Error result. A
86-
* caller that records before the write calls `onFragmentAborted()` if the
87-
* result is Error. The caller must abort before another `onFragment()` call
88-
* because the abort removes the newest record.
84+
* `onFragment()` appends {streamId, isLastFragment} to a FIFO.
85+
* `onFragmentAborted()` removes the newest record, and does nothing when the
86+
* FIFO is empty.
8987
*
9088
* Each `BatchWriteComplete` pops `frameCount` records from the front and fires
9189
* FrameWriteComplete{streamId} for each record whose `isLastFragment` is true.

0 commit comments

Comments
 (0)