-
Notifications
You must be signed in to change notification settings - Fork 18
Adding configurable option to set source parallelism #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding configurable option to set source parallelism #103
Conversation
...nk/src/main/java/com/pinterest/flink/streaming/connectors/psc/table/PscConnectorOptions.java
Outdated
Show resolved
Hide resolved
...flink/src/main/java/com/pinterest/flink/streaming/connectors/psc/table/PscDynamicSource.java
Outdated
Show resolved
Hide resolved
...flink/src/main/java/com/pinterest/flink/streaming/connectors/psc/table/PscDynamicSource.java
Outdated
Show resolved
Hide resolved
...ink/src/main/java/com/pinterest/flink/streaming/connectors/psc/table/PscTableSourceUtil.java
Outdated
Show resolved
Hide resolved
...flink/src/main/java/com/pinterest/flink/streaming/connectors/psc/table/PscDynamicSource.java
Show resolved
Hide resolved
nickpan47
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm overall. Please refactor the dup code block. Thanks!
...in/java/com/pinterest/flink/streaming/connectors/psc/table/UpsertPscDynamicTableFactory.java
Outdated
Show resolved
Hide resolved
...test/java/com/pinterest/flink/streaming/connectors/psc/table/PscDynamicTableFactoryTest.java
Show resolved
Hide resolved
...nk/src/main/java/com/pinterest/flink/streaming/connectors/psc/table/PscTableCommonUtils.java
Show resolved
Hide resolved
nickpan47
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: suggest renaming the util class. Otherwise, lgtm. Thanks!
nickpan47
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ship it!
* Adding configurable optional to set source parallelism * fixes * make source infer parallelism * adding shuffle * revert pom * removing inference and keeping boolean flag * rework rescale apply logic * make common util * create TableCommonUtils --------- Co-authored-by: Kevin Browne <kbrowne@kbrowne-JG645XV.dyn.pinadmin.com>
* Adding configurable option to set source parallelism (#103) * Adding configurable optional to set source parallelism * fixes * make source infer parallelism * adding shuffle * revert pom * removing inference and keeping boolean flag * rework rescale apply logic * make common util * create TableCommonUtils --------- Co-authored-by: Kevin Browne <kbrowne@kbrowne-JG645XV.dyn.pinadmin.com> * Turn off error logging in flink tests --------- Co-authored-by: KevBrowne <kjbrowne0928@gmail.com> Co-authored-by: Kevin Browne <kbrowne@kbrowne-JG645XV.dyn.pinadmin.com>
Summary
Currently, PscDynamicSource defaults to parallelism equal to the number of topic partitions. Under Flink's FLIP-27 Source API, when parallelism is not explicitly set, Flink automatically derives it from the number of splits (topic partitions in this case). This creates bottlenecks when downstream operators need higher parallelism, as the source operator cannot scale beyond the partition count, leading to:
Solution
Added
scan.enable-rescale** configuration option that determines when data redistribution is needed. When enabled, the connector:table.exec.resource.default-parallelism) - https://github.com/apache/flink/blob/master/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/ExecutionConfigOptions.java#L314rescale()ONLY when parallelism > partition countThis eliminates manual calculation and automatically optimizes based on actual cluster state.
Added single boolean configuration option:
scan.enable-rescale(Boolean, default:false)rescale()shuffle to redistribute datafalseto avoid unnecessary overhead when partitions >= pipeline parallelismConfigure Pipeline Parallelism
Or via SQL:
Usage
Scenario 1: Rescale Applied
rescale()automatically"Applying rescale(): configured parallelism (10) > partition count (2)"Scenario 2: Rescale Skipped
rescale()automatically"Skipping rescale(): configured parallelism (4) <= partition count (8)"When to enable:
When to keep disabled (default):
Unit test