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 2020/01/14 10:06:32 UTC
[GitHub] [flink] xintongsong opened a new pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
xintongsong opened a new pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852
## What is the purpose of the change
This PR fix the `YarnClusterDescriptorTest#testFailIfTaskSlotsHigherThanMaxVcores` and `#testFailIfTaskSlotsHigherThanMaxVcores`, which should have failed long ago but was covered by other problem.
The original purpose of these two test cases was to verify the validation logic against yarn max allocation vcores. These two cases should have failed when we change the validation logic to get yarn max allocation vcores from yarnClient instead of configuration, because there are no yarn cluster (neither `MiniYARNCluster`) started in these cases, thus `yarnClient#getNodeReports` will never return.
The cases have not failed because another `IllegalConfigurationException` was thrown in `validateClusterSpecification`, because of memory validation failure. The memory validation failure was by design, and in order to verify the original purpose these two test cases should have been updated with reasonable memory sizes, which is unfortunately overlooked.
## Brief change log
- 04ffffe5cf919ab07ddba656a68d4cd2d17c64c5: Update memory setups to uncover the problem.
- I leave this as a separate commit for the convenience of code review. This should be squashed at the merging time.
- 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec: Fix test cases by mocking the yarn max allocation vcores.
## Verifying this change
This change is already covered by existing tests.
## 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, Yarn/Mesos, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] tillrohrmann commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r367350982
##########
File path: flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java
##########
@@ -318,6 +308,20 @@ private void isReadyForDeployment(ClusterSpecification clusterSpecification) thr
}
}
+ @VisibleForTesting
+ protected int getNumYarnMaxVcores() throws YarnDeploymentException {
Review comment:
I agree with Andrey that overriding production code methods should always be our last resort when it comes to testing. The danger is that this method evolves and that we override important behaviour unintentionally. I would propose two solutions to the problem:
1. Introduce a `YarnClusterInformationRetriever` interface which offers the method `getMaxVcores`. The default implementation will simply use the `YarnClient` to retrieve the max vcores. In the test we can provide a testing implementation.
2. Alternatively, similar to `TestingYarnClient` override the `getNodeReports` method from `YarnClientImpl`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144685765 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
* e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144685765) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144685765 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
Hash:9968737a09a938265c97772db07933cc3961a737 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4400 TriggerType:PUSH TriggerID:9968737a09a938265c97772db07933cc3961a737
Hash:9968737a09a938265c97772db07933cc3961a737 Status:CANCELED URL:https://travis-ci.com/flink-ci/flink/builds/144745817 TriggerType:PUSH TriggerID:9968737a09a938265c97772db07933cc3961a737
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
* e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144685765) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390)
* 9968737a09a938265c97772db07933cc3961a737 Travis: [CANCELED](https://travis-ci.com/flink-ci/flink/builds/144745817) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4400)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
* e0f8d6d695bb50aa4cc8883f65a1eae12a634811 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] azagrebin commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r367361636
##########
File path: flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java
##########
@@ -318,6 +308,20 @@ private void isReadyForDeployment(ClusterSpecification clusterSpecification) thr
}
}
+ @VisibleForTesting
+ protected int getNumYarnMaxVcores() throws YarnDeploymentException {
Review comment:
I also looked into `YarnClientImpl`. My only concern about this option was that it is annotated with `@Private` and `@Unstable` but I did not see that we actually already decided once to extend it. I would be ok with it. I guess we will refactor it if this class is gone after updating the Yarn dependency.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] tillrohrmann closed pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
tillrohrmann closed pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] azagrebin commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366981641
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -104,9 +104,7 @@ public void testFailIfTaskSlotsHigherThanMaxVcores() throws ClusterDeploymentExc
clusterDescriptor.setLocalJarPath(new Path(flinkJar.getPath()));
ClusterSpecification clusterSpecification = new ClusterSpecification.ClusterSpecificationBuilder()
- .setMasterMemoryMB(1)
- .setTaskManagerMemoryMB(1)
- .setNumberTaskManagers(1)
+ .setTaskManagerMemoryMB(1024)
Review comment:
Should we actually try to change the default `ClusterSpecificationBuilder.taskManagerMemoryMB` to `1024` if it is already clear that it is inconsistent in general? This would also give us more confidence that what we are fixing now does not happen in other tests as well.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/144685765 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
* e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/144685765) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] wangyang0918 commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366282296
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -115,6 +113,7 @@ public void testFailIfTaskSlotsHigherThanMaxVcores() throws ClusterDeploymentExc
fail("The deploy call should have failed.");
} catch (ClusterDeploymentException e) {
+ e.printStackTrace();
Review comment:
Why do we need `e.printStackTrace()`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144685765 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
Hash:9968737a09a938265c97772db07933cc3961a737 Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:9968737a09a938265c97772db07933cc3961a737
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
* e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144685765) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390)
* 9968737a09a938265c97772db07933cc3961a737 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366286869
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -137,10 +136,7 @@ public void testConfigOverwrite() throws ClusterDeploymentException {
// configure slots
ClusterSpecification clusterSpecification = new ClusterSpecification.ClusterSpecificationBuilder()
- .setMasterMemoryMB(1)
- .setTaskManagerMemoryMB(1)
- .setNumberTaskManagers(1)
- .setSlotsPerTaskManager(1)
+ .setTaskManagerMemoryMB(1024)
Review comment:
Same here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] wangyang0918 commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366282419
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -104,9 +104,7 @@ public void testFailIfTaskSlotsHigherThanMaxVcores() throws ClusterDeploymentExc
clusterDescriptor.setLocalJarPath(new Path(flinkJar.getPath()));
ClusterSpecification clusterSpecification = new ClusterSpecification.ClusterSpecificationBuilder()
- .setMasterMemoryMB(1)
- .setTaskManagerMemoryMB(1)
- .setNumberTaskManagers(1)
+ .setTaskManagerMemoryMB(1024)
Review comment:
Do we really need to set the `TaskManagerMemoryMB` to 1024? Or the default value(768) is 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r367209409
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -104,9 +104,7 @@ public void testFailIfTaskSlotsHigherThanMaxVcores() throws ClusterDeploymentExc
clusterDescriptor.setLocalJarPath(new Path(flinkJar.getPath()));
ClusterSpecification clusterSpecification = new ClusterSpecification.ClusterSpecificationBuilder()
- .setMasterMemoryMB(1)
- .setTaskManagerMemoryMB(1)
- .setNumberTaskManagers(1)
+ .setTaskManagerMemoryMB(1024)
Review comment:
I'll give it a try to change the default to 1024, see if it breaks anything.
I'm actually thinking about remove `ClusterSpecification` entirely. I've visited all the usages of this class, and it seems to me all its information can be fetched directly from configuration. However, I'd like to scope this out from this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574099032
cc @azagrebin
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] azagrebin commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366995297
##########
File path: flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java
##########
@@ -318,6 +308,20 @@ private void isReadyForDeployment(ClusterSpecification clusterSpecification) thr
}
}
+ @VisibleForTesting
+ protected int getNumYarnMaxVcores() throws YarnDeploymentException {
Review comment:
I am wondering whether it is actually less invasive to introduce a `StubYarnClientImpl` and use in `YarnClusterDescriptorTest#setup` instead of changing production class `YarnClusterDescriptor` for this:
```
class StubYarnClientImpl() extends YarnClient {
@Override
public List<NodeReport> getNodeReports(NodeState... states) {
return Collections.singletonList(new NodeReport() {
@Override
public Resource getCapability() {
return new Resource() {
@Override
public int getVirtualCores() {
return NUM_YARN_MAX_VCORES;
}
// ...
};
}
// ...
});
}
// ...
}
public class YarnClusterDescriptorTest extends TestLogger {
// ..
@BeforeClass
public static void setupClass() {
yarnConfiguration = new YarnConfiguration();
yarnClient = new StubYarnClientImpl();
yarnClient.init(yarnConfiguration);
yarnClient.start();
}
//..
}
```
It is quite some useless code but we can put into a separate file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r367270089
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -104,9 +104,7 @@ public void testFailIfTaskSlotsHigherThanMaxVcores() throws ClusterDeploymentExc
clusterDescriptor.setLocalJarPath(new Path(flinkJar.getPath()));
ClusterSpecification clusterSpecification = new ClusterSpecification.ClusterSpecificationBuilder()
- .setMasterMemoryMB(1)
- .setTaskManagerMemoryMB(1)
- .setNumberTaskManagers(1)
+ .setTaskManagerMemoryMB(1024)
Review comment:
Making `ClusterSpecificationBuilder#taskManagerMemoryMB` default 1024 turns out works well. No test broken due to this change. https://travis-ci.org/xintongsong/flink/builds/637743056
I pushed a fixup commit to this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] wangyang0918 commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366282542
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -137,10 +136,7 @@ public void testConfigOverwrite() throws ClusterDeploymentException {
// configure slots
ClusterSpecification clusterSpecification = new ClusterSpecification.ClusterSpecificationBuilder()
- .setMasterMemoryMB(1)
- .setTaskManagerMemoryMB(1)
- .setNumberTaskManagers(1)
- .setSlotsPerTaskManager(1)
+ .setTaskManagerMemoryMB(1024)
Review comment:
Same as above.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r367211936
##########
File path: flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java
##########
@@ -318,6 +308,20 @@ private void isReadyForDeployment(ClusterSpecification clusterSpecification) thr
}
}
+ @VisibleForTesting
+ protected int getNumYarnMaxVcores() throws YarnDeploymentException {
Review comment:
I'm not sure about this.
`YarnClient` is an abstract class, and to introduce a `StubYarnClientImpl` we would need to also implement more than 20 useless methods and mock the `NodeReport` as well. The complication seems unnecessary to me.
It is true the current approach touches the production class `YarnClusterDescriptor`, but only with a trivial refactor. IMO, even without this testability issue, it is not a bad thing to extract the logic getting max vcores from Yarn into a separate method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366286783
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -104,9 +104,7 @@ public void testFailIfTaskSlotsHigherThanMaxVcores() throws ClusterDeploymentExc
clusterDescriptor.setLocalJarPath(new Path(flinkJar.getPath()));
ClusterSpecification clusterSpecification = new ClusterSpecification.ClusterSpecificationBuilder()
- .setMasterMemoryMB(1)
- .setTaskManagerMemoryMB(1)
- .setNumberTaskManagers(1)
+ .setTaskManagerMemoryMB(1024)
Review comment:
The default is not enough. It would cause memory validation failure because the derived network memory is smaller than default min.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574100443
Travis passed: https://travis-ci.org/xintongsong/flink/builds/636391268
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574100040
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec (Tue Jan 14 10:09:50 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r366286430
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java
##########
@@ -115,6 +113,7 @@ public void testFailIfTaskSlotsHigherThanMaxVcores() throws ClusterDeploymentExc
fail("The deploy call should have failed.");
} catch (ClusterDeploymentException e) {
+ e.printStackTrace();
Review comment:
Nice catch. That was for debugging and I forgot to remove it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-574107516
<!--
Meta data
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144299202 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326 TriggerType:PUSH TriggerID:54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144315708 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:494422cbcad72a0aab8c2df77be76422f2e6085e Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332 TriggerType:PUSH TriggerID:494422cbcad72a0aab8c2df77be76422f2e6085e
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144685765 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
Hash:e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390 TriggerType:PUSH TriggerID:e0f8d6d695bb50aa4cc8883f65a1eae12a634811
-->
## CI report:
* 54fdafd9c9d9c4bd6df9bd9b25de461f8f5bbcec Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144299202) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4326)
* 494422cbcad72a0aab8c2df77be76422f2e6085e Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144315708) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4332)
* e0f8d6d695bb50aa4cc8883f65a1eae12a634811 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144685765) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4390)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] xintongsong commented on issue #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to
validate the original intended behavior
Posted by GitBox <gi...@apache.org>.
xintongsong commented on issue #10852: [FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate the original intended behavior
URL: https://github.com/apache/flink/pull/10852#issuecomment-575161611
Thanks for the review and explanations, @tillrohrmann and @azagrebin .
I think `YarnClusterInformationRetriever` sounds to be a really good solution.
For having a testing implementation of `YarnClient`, my concern is that we might need to introduce too many unused implementations. And if the Yarn API is extended in later versions, we might need to further extend our testing implementation, or even run into dependency problems of various Hadoop versions.
I've updated the PR with the `YarnClusterInformationRetriever` approach. Please take another look.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services