You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/06 01:56:41 UTC
[GitHub] [flink] Myracle opened a new pull request, #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Myracle opened a new pull request, #19651:
URL: https://github.com/apache/flink/pull/19651
## What is the purpose of the change
*When we start the job by command with args "-p $parallelism", the error is thrown with "The parallelism must be a positive number: -1". Since we can use AdaptiveBatch with config parallelism.default: -1, we should support it from Cli.*
## Brief change log
- *Remove the limit when the Cli parallelism is -1 in class ProgramOptions.*
- *Apply to the configuration when Cli parallelism is set.*
## Verifying this change
This change added tests and can be verified as follows:
- *Add unittest for this feature in CliFrontendRunTest*
## 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
[GitHub] [flink] zhuzhurk commented on a diff in pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on code in PR #19651:
URL: https://github.com/apache/flink/pull/19651#discussion_r870927910
##########
flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendRunTest.java:
##########
@@ -208,6 +209,31 @@ public void testParallelismWithOverflow() throws Exception {
testFrontend.run(parameters);
}
+ @Test(expected = CliArgsException.class)
+ public void testParallelismWithInvalidValue() throws Exception {
+ String[] parameters = {"-v", "-p", "-2", getTestJarPath()};
+ Configuration configuration = new Configuration();
+ CliFrontend testFrontend =
+ new CliFrontend(configuration, Collections.singletonList(getCli()));
+ testFrontend.run(parameters);
+ }
+
+ @Test
+ public void testParallelismWithNegativeOne() throws Exception {
+ final Configuration configuration = getConfiguration();
+ String[] parameters = {"-v", "-p", "-1", getTestJarPath()};
+ verifyCliFrontend(configuration, getCli(), parameters, -1, false);
+ }
+
+ @Test
+ public void testParallelismWithNegativeOneAndConfig() throws Exception {
Review Comment:
maybe "testParallelismWithNegativeOneAndConfig" -> "testDefaultParallelismOptionOverridesConfiguration"
##########
flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendRunTest.java:
##########
@@ -208,6 +209,31 @@ public void testParallelismWithOverflow() throws Exception {
testFrontend.run(parameters);
}
+ @Test(expected = CliArgsException.class)
+ public void testParallelismWithInvalidValue() throws Exception {
+ String[] parameters = {"-v", "-p", "-2", getTestJarPath()};
+ Configuration configuration = new Configuration();
+ CliFrontend testFrontend =
+ new CliFrontend(configuration, Collections.singletonList(getCli()));
+ testFrontend.run(parameters);
+ }
+
+ @Test
+ public void testParallelismWithNegativeOne() throws Exception {
Review Comment:
maybe "testParallelismWithNegativeOne" -> "testDefaultParallelismOption"
##########
flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendRunTest.java:
##########
@@ -208,6 +209,31 @@ public void testParallelismWithOverflow() throws Exception {
testFrontend.run(parameters);
}
+ @Test(expected = CliArgsException.class)
+ public void testParallelismWithInvalidValue() throws Exception {
+ String[] parameters = {"-v", "-p", "-2", getTestJarPath()};
Review Comment:
Looks to me option "v" is deprecated and is nod needed by the test.
##########
flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendRunTest.java:
##########
@@ -208,6 +209,31 @@ public void testParallelismWithOverflow() throws Exception {
testFrontend.run(parameters);
}
+ @Test(expected = CliArgsException.class)
+ public void testParallelismWithInvalidValue() throws Exception {
+ String[] parameters = {"-v", "-p", "-2", getTestJarPath()};
+ Configuration configuration = new Configuration();
+ CliFrontend testFrontend =
+ new CliFrontend(configuration, Collections.singletonList(getCli()));
+ testFrontend.run(parameters);
+ }
+
+ @Test
+ public void testParallelismWithNegativeOne() throws Exception {
+ final Configuration configuration = getConfiguration();
+ String[] parameters = {"-v", "-p", "-1", getTestJarPath()};
Review Comment:
I think it's better to replace -1 with ExecutionConfig.PARALLELISM_DEFAULT.
This also applies to other -1
##########
flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendRunTest.java:
##########
@@ -208,6 +209,31 @@ public void testParallelismWithOverflow() throws Exception {
testFrontend.run(parameters);
}
+ @Test(expected = CliArgsException.class)
+ public void testParallelismWithInvalidValue() throws Exception {
Review Comment:
maybe "testParallelismWithInvalidValue" -> "testInvalidNegativeParallelismOption"
because there is already a "testInvalidParallelismOption" test.
--
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
[GitHub] [flink] Myracle commented on pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Posted by GitBox <gi...@apache.org>.
Myracle commented on PR #19651:
URL: https://github.com/apache/flink/pull/19651#issuecomment-1120545073
@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
[GitHub] [flink] zhuzhurk closed pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Posted by GitBox <gi...@apache.org>.
zhuzhurk closed pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
URL: https://github.com/apache/flink/pull/19651
--
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
[GitHub] [flink] flinkbot commented on pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19651:
URL: https://github.com/apache/flink/pull/19651#issuecomment-1119193640
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "16a8590b3b7d370a75250afe76c71f5ebdc3b651",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "16a8590b3b7d370a75250afe76c71f5ebdc3b651",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 16a8590b3b7d370a75250afe76c71f5ebdc3b651 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
[GitHub] [flink] zhuzhurk commented on a diff in pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on code in PR #19651:
URL: https://github.com/apache/flink/pull/19651#discussion_r870922447
##########
flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendRunTest.java:
##########
@@ -208,6 +209,31 @@ public void testParallelismWithOverflow() throws Exception {
testFrontend.run(parameters);
}
+ @Test(expected = CliArgsException.class)
+ public void testParallelismWithInvalidValue() throws Exception {
+ String[] parameters = {"-v", "-p", "-2", getTestJarPath()};
Review Comment:
Looks to me option "v" is deprecated and is not needed by the test.
--
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
[GitHub] [flink] Myracle commented on pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Posted by GitBox <gi...@apache.org>.
Myracle commented on PR #19651:
URL: https://github.com/apache/flink/pull/19651#issuecomment-1119364061
@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
[GitHub] [flink] Myracle commented on pull request #19651: [FLINK-26909][Client/Job Submission] Support AdaptiveBatch when parallelism is -1 from Cli
Posted by GitBox <gi...@apache.org>.
Myracle commented on PR #19651:
URL: https://github.com/apache/flink/pull/19651#issuecomment-1125773764
@zhuzhurk Thanks for the valuable suggestions. All of them are solved. Please review again. Thanks.
--
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