You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "SinBex (via GitHub)" <gi...@apache.org> on 2024/01/24 06:52:56 UTC
[PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
SinBex opened a new pull request, #24186:
URL: https://github.com/apache/flink/pull/24186
## What is the purpose of the change
*[FLIP-379](https://cwiki.apache.org/confluence/display/FLINK/FLIP-379%3A+Dynamic+source+parallelism+inference+for+batch+jobs) has introduced support for dynamic source parallelism inference in batch jobs, and we plan to give priority to enabling this feature for the file source connector.*
## Brief change log
- *The FileSource implements the DynamicParallelismInference interface.*
## Verifying this change
This change is already covered by existing tests, such as *(FileSourceTextLinesITCase::testBoundedTextFileSource)*.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no )
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: ( no )
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "SinBex (via GitHub)" <gi...@apache.org>.
SinBex commented on PR #24186:
URL: https://github.com/apache/flink/pull/24186#issuecomment-1909902429
@flinkbot run azure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "zhuzhurk (via GitHub)" <gi...@apache.org>.
zhuzhurk commented on PR #24186:
URL: https://github.com/apache/flink/pull/24186#issuecomment-1911805813
@flinkbot run azure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "zhuzhurk (via GitHub)" <gi...@apache.org>.
zhuzhurk commented on code in PR #24186:
URL: https://github.com/apache/flink/pull/24186#discussion_r1465885202
##########
flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/src/AbstractFileSource.java:
##########
@@ -100,6 +100,14 @@ protected AbstractFileSource(
// Getters
// ------------------------------------------------------------------------
+ public Path[] getInputPaths() {
Review Comment:
Making the method package private should be enough.
##########
flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/src/AbstractFileSource.java:
##########
@@ -100,6 +100,14 @@ protected AbstractFileSource(
// Getters
// ------------------------------------------------------------------------
+ public Path[] getInputPaths() {
+ return inputPaths;
+ }
+
+ public FileEnumerator.Provider getEnumeratorFactory() {
Review Comment:
Making the method package private should be enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "zhuzhurk (via GitHub)" <gi...@apache.org>.
zhuzhurk commented on code in PR #24186:
URL: https://github.com/apache/flink/pull/24186#discussion_r1464840583
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/src/FileSourceTextLinesITCase.java:
##########
@@ -126,8 +127,9 @@ private void testBoundedTextFileSource(
.build();
final StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment();
+ env.setRuntimeMode(RuntimeExecutionMode.BATCH);
Review Comment:
Can we add a new test to verify dynamic parallelism inference for file sources?
We may use `miniCluster.getExecutionGraph(jobId)` to get the execution graph of the job and check the parallelism of the source.
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/src/FileSourceTextLinesITCase.java:
##########
@@ -126,8 +127,9 @@ private void testBoundedTextFileSource(
.build();
final StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment();
+ env.setRuntimeMode(RuntimeExecutionMode.BATCH);
env.setParallelism(PARALLELISM);
- env.setRestartStrategy(RestartStrategies.fixedDelayRestart(1, 0));
+ env.setRestartStrategy(RestartStrategies.fixedDelayRestart(PARALLELISM, 0));
Review Comment:
What would happen if we do not increase it?
##########
flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/src/AbstractFileSource.java:
##########
@@ -177,6 +180,24 @@ public TypeInformation<T> getProducedType() {
return readerFormat.getProducedType();
}
+ @Override
+ public int inferParallelism(Context dynamicParallelismContext) {
Review Comment:
`AbstractFileSource` is a base for both `FileSource` and `HiveSource`. If we only target `FileSource` in the PR, why not directly modify `FileSource`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "zhuzhurk (via GitHub)" <gi...@apache.org>.
zhuzhurk closed pull request #24186: [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs
URL: https://github.com/apache/flink/pull/24186
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "SinBex (via GitHub)" <gi...@apache.org>.
SinBex commented on PR #24186:
URL: https://github.com/apache/flink/pull/24186#issuecomment-1909489041
@zhuzhurk Thanks for reviewing! I have fixed the problems you commented. PTAL~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "SinBex (via GitHub)" <gi...@apache.org>.
SinBex commented on PR #24186:
URL: https://github.com/apache/flink/pull/24186#issuecomment-1911552066
@flinkbot run azure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #24186:
URL: https://github.com/apache/flink/pull/24186#issuecomment-1907491277
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "cf88c4b65ae73600d8efb25d6273a7ea16ae5b2d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "cf88c4b65ae73600d8efb25d6273a7ea16ae5b2d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* cf88c4b65ae73600d8efb25d6273a7ea16ae5b2d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "zhuzhurk (via GitHub)" <gi...@apache.org>.
zhuzhurk commented on PR #24186:
URL: https://github.com/apache/flink/pull/24186#issuecomment-1910439284
@flinkbot run azure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-34145][connector/filesystem] support dynamic source parallelism inference in batch jobs [flink]
Posted by "zhuzhurk (via GitHub)" <gi...@apache.org>.
zhuzhurk commented on PR #24186:
URL: https://github.com/apache/flink/pull/24186#issuecomment-1911841522
@flinkbot run azure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org