Skip to content

Commit 837a794

Browse files
committed
dl/translation: fix busy loop under coordinator backpressure
The translation loop treated a backpressured fetch as a successful iteration and canceled its retry jitter, so backpressured translators re-polled the coordinator as fast as the fetch RPC completed. This added noticeable CPU churn in some clusters with existing backpressure. This commit routes backpressured iterations through the same jittered sleep as a failed fetch; a requested finish still proceeds.
1 parent 7ad90e0 commit 837a794

4 files changed

Lines changed: 47 additions & 2 deletions

File tree

src/v/datalake/translation/partition_translator.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ partition_translator::fetch_translation_offsets(retry_chain_node& rcn) {
236236
"[{}] Coordinator applying backpressure (too many pending files); "
237237
"backing off translation",
238238
_data_source->ntp());
239+
offsets.backpressure = true;
239240
co_return offsets;
240241
}
241242

@@ -474,7 +475,15 @@ ss::future<> partition_translator::translate_until_stopped() {
474475
if (finish_now) {
475476
vlog(_logger.debug, "Requested for immediate finish");
476477
}
477-
if (!offsets && !finish_now) {
478+
if (!offsets) {
479+
// Without reconciled offsets there is nothing to translate or
480+
// finish against.
481+
continue;
482+
}
483+
// A backpressured iteration takes the jittered retry path like a
484+
// failed fetch, rather than immediately polling the coordinator
485+
// again. A requested finish still proceeds below.
486+
if (offsets->backpressure && !finish_now) {
478487
continue;
479488
}
480489
if (offsets->next_translation_begin_offset && !finish_now) {

src/v/datalake/translation/partition_translator.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,9 @@ class partition_translator : public scheduling::translator {
163163
// Offset to begin the next translation from.
164164
// set if there is new data to translate
165165
std::optional<kafka::offset> next_translation_begin_offset;
166+
// Set if the coordinator is shedding load; the translation loop
167+
// should back off before polling again.
168+
bool backpressure{false};
166169
};
167170
ss::future<std::optional<translation_offsets>>
168171
fetch_translation_offsets(retry_chain_node&);

src/v/datalake/translation/tests/partition_translator_tests.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ class fake_test_ctx {
7676

7777
void note_backpressure_rejection() { _backpressure_rejections++; }
7878

79+
size_t backpressure_rejections() const { return _backpressure_rejections; }
80+
7981
ss::future<> wait_for_backpressure_rejections(
8082
size_t n, std::chrono::seconds timeout = 10s) {
8183
RPTEST_REQUIRE_EVENTUALLY_CORO(
@@ -509,6 +511,25 @@ TEST_F_CORO(partition_translator_fixture, test_coordinator_backpressure) {
509511
ASSERT_GT_CORO(test_ctx.max_translated_offset(), kafka::offset{0});
510512
}
511513

514+
TEST_F_CORO(
515+
partition_translator_fixture, test_coordinator_backpressure_is_paced) {
516+
auto& test_ctx = make_test_context();
517+
test_ctx.set_coordinator_backpressure(true);
518+
test_ctx.set_should_finish_inflight_translation(false);
519+
co_await add_translator(test_ctx);
520+
521+
co_await test_ctx.wait_for_backpressure_rejections(1);
522+
auto baseline = test_ctx.backpressure_rejections();
523+
co_await ss::sleep(200ms);
524+
auto polls = test_ctx.backpressure_rejections() - baseline;
525+
526+
// Each backpressured iteration should sleep the loop jitter (10ms base in
527+
// this fixture) before polling the coordinator again, so expect ~20 polls.
528+
// Regression check for a bug where we would poll without waiting: a
529+
// translator that spins on fetches would rack up thousands.
530+
ASSERT_LE_CORO(polls, 100);
531+
}
532+
512533
TEST_F_CORO(partition_translator_fixture, test_batching) {
513534
auto& test_ctx = make_test_context();
514535
// Let the translator batch to the limit

tests/rptest/tests/datalake/coordinator_backpressure_test.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ def test_backpressure_on_pending_backlog(self, cloud_storage_type):
133133
err_msg="coordinator never applied backpressure",
134134
)
135135

136-
# Translators respect the signal by backing off rather than spinning.
136+
# Translators respect the signal by backing off.
137137
wait_until(
138138
lambda: self.metric_sum(TRANSLATION_BACKOFF_METRIC) > 0,
139139
timeout_sec=30,
@@ -186,6 +186,18 @@ def quiesced():
186186
err_msg="pending/translated file counts never stabilized under backpressure",
187187
)
188188

189+
# Regression check for a case where translation would spin and
190+
# churn despite backpressure: backing off means each translator
191+
# sleeps its loop jitter between coordinator polls, so the
192+
# cumulative backoff count stays on the order of hundreds, with so
193+
# few partitions.
194+
backoffs = self.metric_sum(TRANSLATION_BACKOFF_METRIC)
195+
self.logger.info(f"translator backoffs while backpressured: {backoffs}")
196+
assert backoffs < 10000, (
197+
f"translators spun on the backpressured coordinator: "
198+
f"{backoffs} backoff loop iterations"
199+
)
200+
189201
# Relieve the pressure so the backlog drains promptly.
190202
self.redpanda.set_cluster_config(
191203
{

0 commit comments

Comments
 (0)