[INC-1227] dl/translation: fix busy loop under coordinator backpressure - #31419
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a translator busy-loop when the Iceberg coordinator applies backpressure by ensuring backpressured iterations take the same jittered sleep path as failed fetches, reducing CPU churn. Adds unit/integration coverage to detect regressions in backpressure pacing.
Changes:
- Mark coordinator backpressure explicitly in
translation_offsetsand route backpressured iterations through the jittered retry path. - Add a C++ unit test to assert backpressured coordinator polling is paced (not spinning).
- Extend the ducktape backpressure test to log scheduling-group CPU and bound backoff-loop iterations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/rptest/tests/datalake/coordinator_backpressure_test.py | Adds CPU visibility helper and a regression assertion bounding backoff iterations under coordinator backpressure. |
| src/v/datalake/translation/tests/partition_translator_tests.cc | Adds a unit test to ensure backpressure handling is paced (limits coordinator polling rate). |
| src/v/datalake/translation/partition_translator.h | Extends translation_offsets with a backpressure flag to propagate coordinator load-shedding state. |
| src/v/datalake/translation/partition_translator.cc | Sets the new backpressure flag on fetch and applies jittered pacing when backpressured. |
| nodes=self.redpanda.nodes, | ||
| metrics_endpoint=MetricsEndpoint.METRICS, | ||
| ) | ||
| assert samples is not None |
| err_msg="pending/translated file counts never stabilized under backpressure", | ||
| ) | ||
|
|
||
| # Regresison check for a case where translation would spin and |
Comment on lines
+478
to
484
| // A backpressured iteration takes the jittered retry path like a | ||
| // failed fetch, rather than immediately polling the coordinator | ||
| // again. A requested finish still proceeds below. | ||
| if ((!offsets || offsets->backpressure) && !finish_now) { | ||
| continue; | ||
| } | ||
| if (offsets->next_translation_begin_offset && !finish_now) { |
Contributor
There was a problem hiding this comment.
I don't think you're correct here, robot.
andrwng
force-pushed
the
iceberg-backpressure
branch
from
August 4, 2026 22:39
34e7ac1 to
e8d93c0
Compare
andrwng
changed the title
andrwng
force-pushed
the
iceberg-backpressure
branch
from
August 4, 2026 22:54
e8d93c0 to
fe90050
Compare
Collaborator
CI test resultstest results on build#88204
test results on build#88292
|
wdberkeley
previously approved these changes
Aug 5, 2026
wdberkeley
left a comment
wdberkeley
left a comment
Contributor
There was a problem hiding this comment.
LGTM but the ducktape test measuring CPU usage seems unnecessary and overwrought, perhaps fragile?
andrwng
force-pushed
the
iceberg-backpressure
branch
from
August 5, 2026 21:54
fe90050 to
c29fad7
Compare
Contributor
Author
Yea it is a bit much here. In general I do think high CPU load is something worth checking for in tests, but I don't think we've established a good general check for it. Probably overkill here and not sustainable for a regression test. Removed |
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.
andrwng
force-pushed
the
iceberg-backpressure
branch
from
August 5, 2026 21:58
c29fad7 to
837a794
Compare
dotnwat
reviewed
Aug 5, 2026
Comment on lines
475
to
+482
| if (finish_now) { | ||
| vlog(_logger.debug, "Requested for immediate finish"); | ||
| } | ||
| if (!offsets && !finish_now) { | ||
| if (!offsets) { | ||
| // Without reconciled offsets there is nothing to translate or | ||
| // finish against. | ||
| continue; | ||
| } |
Member
There was a problem hiding this comment.
I'm sure I'm missing something, but it reads buggy that finish_now isn't the highest priority check here, allowing other conditions to take precedence in continuing in the loop
Contributor
Author
There was a problem hiding this comment.
The issue is that there isn't a well thought through way to stop work without information from the coordinator, because finishing typically entails sending completed work to the coordinator.
There's probably a different fix that entails discarding the data we have, but it seems a bit more involved to ensure correctness. At the very least avoiding a null dereference seems like an easier win
dotnwat
approved these changes
Aug 6, 2026
andrwng
enabled auto-merge
August 6, 2026 21:58
Contributor
Author
|
/backport v26.2.x |
Contributor
Author
|
/backport v26.1.x |
Contributor
Author
|
/backport v25.3.x |
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
This also adds a ducktape test that bounds the cumulative backoff counter, which records a spin no matter when it is sampled. Without the change, this test saw 1.1 cores of the datalake scheduling group runtime, vs 0.001 core with this commit (and 1.6M loop iterations vs 260).
Backports Required
Release Notes
Bug Fixes