Skip to content

Flink: Set generator parallelism to match input in DynamicIcebergSink#15849

Merged
pvary merged 1 commit intoapache:mainfrom
sachinnn99:flink-dynamic-sink-parallelism
Apr 17, 2026
Merged

Flink: Set generator parallelism to match input in DynamicIcebergSink#15849
pvary merged 1 commit intoapache:mainfrom
sachinnn99:flink-dynamic-sink-parallelism

Conversation

@sachinnn99
Copy link
Copy Markdown
Contributor

Summary

  • Set DynamicRecordProcessor (generator) operator parallelism to match upstream input parallelism in DynamicIcebergSink.Builder.append()
  • Without this fix, the generator falls back to the environment default parallelism, ignoring any parallelism configured on the upstream operator
  • Matches the established pattern used in IcebergSink and FlinkSink

Closes #15827

Test plan

  • Added testGeneratorDefaultParallelism in TestDynamicIcebergSink (all 3 Flink versions)
  • Verified existing testOperatorUidsFormat still passes
  • spotlessCheck passes

@github-actions github-actions bot added the flink label Apr 1, 2026
Copy link
Copy Markdown
Contributor

@Below0 Below0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @sachinnn99! LGTM

@sachinnn99
Copy link
Copy Markdown
Contributor Author

Thanks for the review!

Copy link
Copy Markdown
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally, we do the latest Flink version first, and then backport to the other versions, but the changes here are easy enough to review.

LGTM. Thanks @sachinnn99!

@mxm
Copy link
Copy Markdown
Contributor

mxm commented Apr 17, 2026

CC @pvary

@pvary pvary merged commit 8f1f483 into apache:main Apr 17, 2026
17 checks passed
@pvary
Copy link
Copy Markdown
Contributor

pvary commented Apr 17, 2026

Merged to main.
Thanks @sachinnn99 for the PR and @mxm for the review!

@manuzhang
Copy link
Copy Markdown
Member

It looks this PR did not rebase on #15433 and brought in a test regression.

> Task :iceberg-flink:iceberg-flink-2.1:test
TestDynamicIcebergSink > testNoShuffleTopology() FAILED
    org.opentest4j.AssertionFailedError: 
    Expecting value to be true but was false
        at app//org.apache.iceberg.flink.sink.dynamic.TestDynamicIcebergSink.testNoShuffleTopology(TestDynamicIcebergSink.java:330)

#16026 has been submitted to fix it.

@sachinnn99
Copy link
Copy Markdown
Contributor Author

Thanks @mxm for the review and the CC, and @pvary for the approval and merge!

@sachinnn99
Copy link
Copy Markdown
Contributor Author

@manuzhang Thanks for catching this and submitting #16026 to fix it. Apologies for the regression — I should have rebased on #15433 before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flink DynamicIcebergSink: DynamicRecordProcessor does not inherit upstream parallelism

5 participants