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
[FLINK-31132] Compact without setting parallelism does not follow the configured sink parallelism for HiveTableSink #21977
Conversation
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.
@reswqa Thanks for fixing. I left some comments. PTAL.
...ink-connector-files/src/main/java/org/apache/flink/connector/file/table/batch/BatchSink.java
Outdated
Show resolved
Hide resolved
...ink-connector-files/src/main/java/org/apache/flink/connector/file/table/batch/BatchSink.java
Outdated
Show resolved
Hide resolved
...ctors/flink-connector-hive/src/main/java/org/apache/flink/connectors/hive/HiveTableSink.java
Outdated
Show resolved
Hide resolved
...onnector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableCompactSinkITCase.java
Show resolved
Hide resolved
...flink-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableSinkITCase.java
Outdated
Show resolved
Hide resolved
...-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableCompactSinkTest.java
Outdated
Show resolved
Hide resolved
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.
@reswqa Thanks for creating this pr, I just have a little comment, please take a look.
...-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableCompactSinkTest.java
Outdated
Show resolved
Hide resolved
Thanks @luoyuxia and @JunRuiLee for the review, I have updated this pr in two fixup commits, PTAL agian~ |
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.
@reswqa Thanks for updating. I only left minor comments. PTAL.
...-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableCompactSinkTest.java
Outdated
Show resolved
Hide resolved
...-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableCompactSinkTest.java
Outdated
Show resolved
Hide resolved
...-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableCompactSinkTest.java
Outdated
Show resolved
Hide resolved
I have updated this according to your comments, @luoyuxia would you mind taking a look again? |
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.
@reswqa Thanks for updating. LGTM
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.
@reswqa Thanks for update and most of it looks good except for one small place, I left a small comment. PTAL
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/graph/StreamNode.java
Outdated
Show resolved
Hide resolved
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.
@reswqa Thanks for updating. LGTM
…st, HiveTableSinkITCase to Junit5 and AssertJ.
…lism subject to sink operator's configured parallelism.
…lism subject to sink operator's configured parallelism. This closes apache#21977
What is the purpose of the change
If the parallelism of compact operator was not set, it should use the sink parallelism and disable parallelism inference when using adaptive batch scheduler to avoid take much time to finish compaction.
Brief change log
Verifying this change
This change added test
HiveTableCompactSinkTest
.Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation