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 2021/01/31 08:58:38 UTC
[GitHub] [flink] SteNicholas opened a new pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
SteNicholas opened a new pull request #14821:
URL: https://github.com/apache/flink/pull/14821
## What is the purpose of the change
*[FLINK-21164](https://issues.apache.org/jira/browse/FLINK-21164) introduced #PackagedProgram#close# to clean up temporary jars, and with [FLINK-9844](https://issues.apache.org/jira/browse/FLINK-9844) the contained classloader will also be closed beforehand. To make it obvious that this also happens in the application cluster entrypoints. `ApplicationClusterEntryPoint` should adjust the entrypoints to explicitly close the `PackagedProgram`.*
## Brief change log
- *`ApplicationClusterEntryPoint` overrides the `close` to invoke the `close` method of `PackagedProgram`.*
## Verifying this change
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
- The serializers: (yes / **no** / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: (yes / **no** / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (**not applicable** / docs / JavaDocs / not documented)
----------------------------------------------------------------
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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
}, {
"hash" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869",
"triggerID" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"triggerType" : "PUSH"
}, {
"hash" : "aa75cbfa98c0494578283d2dab0908cdd4942c3a",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12968",
"triggerID" : "aa75cbfa98c0494578283d2dab0908cdd4942c3a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 90ef99a414d6805f625d07f8f9a180c9617fecfb Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869)
* aa75cbfa98c0494578283d2dab0908cdd4942c3a Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12968)
<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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6916a3097fb35e9db0585aa41e7e1fb5bce541f3 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707)
* 8b73e5d70f29adcefc2bd1c3ce673141b94b0213 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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6916a3097fb35e9db0585aa41e7e1fb5bce541f3 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707)
<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
[GitHub] [flink] zentol commented on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
zentol commented on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770375073
A simple solution might be to introduce a try-finally block in each cluster entry point.
----------------------------------------------------------------
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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770349747
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 aa75cbfa98c0494578283d2dab0908cdd4942c3a (Fri May 28 08:18:20 UTC 2021)
**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
[GitHub] [flink] zentol merged pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
zentol merged pull request #14821:
URL: https://github.com/apache/flink/pull/14821
----------------------------------------------------------------
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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6916a3097fb35e9db0585aa41e7e1fb5bce541f3 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707)
<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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
}, {
"hash" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b73e5d70f29adcefc2bd1c3ce673141b94b0213 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863)
* 90ef99a414d6805f625d07f8f9a180c9617fecfb 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
[GitHub] [flink] zentol commented on a change in pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #14821:
URL: https://github.com/apache/flink/pull/14821#discussion_r570235182
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/ClusterEntrypoint.java
##########
@@ -506,7 +506,7 @@ private Configuration generateClusterConfiguration(Configuration configuration)
*
* @throws IOException if the temporary directories could not be cleaned up
*/
- private void cleanupDirectories() throws IOException {
+ public void cleanupDirectories() throws IOException {
Review comment:
why not protected?
##########
File path: flink-clients/src/main/java/org/apache/flink/client/deployment/application/ApplicationClusterEntryPoint.java
##########
@@ -114,4 +115,16 @@ protected static void configureExecution(
return Collections.unmodifiableList(
classpath.stream().distinct().collect(Collectors.toList()));
}
+
+ @Override
+ public void cleanupDirectories() throws IOException {
+ try {
+ // Close the packaged program explicitly to clean up temporary jars.
+ program.close();
Review comment:
this will never throw an exception, and even if then we wouldn't want to rethrow it because it would skip `super.cleanupDirectories`.
----------------------------------------------------------------
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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
}, {
"hash" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869",
"triggerID" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"triggerType" : "PUSH"
}, {
"hash" : "aa75cbfa98c0494578283d2dab0908cdd4942c3a",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12968",
"triggerID" : "aa75cbfa98c0494578283d2dab0908cdd4942c3a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* aa75cbfa98c0494578283d2dab0908cdd4942c3a Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12968)
<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
[GitHub] [flink] flinkbot commented on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770349747
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 6916a3097fb35e9db0585aa41e7e1fb5bce541f3 (Sun Jan 31 09:01:17 UTC 2021)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
* **This pull request references an unassigned [Jira ticket](https://issues.apache.org/jira/browse/FLINK-21210).** According to the [code contribution guide](https://flink.apache.org/contributing/contribute-code.html), tickets need to be assigned before starting with the implementation work.
<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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b73e5d70f29adcefc2bd1c3ce673141b94b0213 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863)
<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
[GitHub] [flink] flinkbot commented on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6916a3097fb35e9db0585aa41e7e1fb5bce541f3 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
[GitHub] [flink] SteNicholas commented on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-772495313
@zentol , thanks for your detailed review. IMO, I thought that overriding the `cleanupDirectories` method in `ClusterEntrypoint` could solve the situation calling `shutdownAsync()`, `closeAsync ` and `close` method in `ApplicationClusterEntryPoint` for closing the `PackagedProgram`. Please help to check again.
----------------------------------------------------------------
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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
}, {
"hash" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869",
"triggerID" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b73e5d70f29adcefc2bd1c3ce673141b94b0213 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863)
* 90ef99a414d6805f625d07f8f9a180c9617fecfb Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869)
<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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
}, {
"hash" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869",
"triggerID" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 90ef99a414d6805f625d07f8f9a180c9617fecfb Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869)
<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
[GitHub] [flink] zentol commented on a change in pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #14821:
URL: https://github.com/apache/flink/pull/14821#discussion_r570235182
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/ClusterEntrypoint.java
##########
@@ -506,7 +506,7 @@ private Configuration generateClusterConfiguration(Configuration configuration)
*
* @throws IOException if the temporary directories could not be cleaned up
*/
- private void cleanupDirectories() throws IOException {
+ public void cleanupDirectories() throws IOException {
Review comment:
why not protected?
##########
File path: flink-clients/src/main/java/org/apache/flink/client/deployment/application/ApplicationClusterEntryPoint.java
##########
@@ -114,4 +115,16 @@ protected static void configureExecution(
return Collections.unmodifiableList(
classpath.stream().distinct().collect(Collectors.toList()));
}
+
+ @Override
+ public void cleanupDirectories() throws IOException {
+ try {
+ // Close the packaged program explicitly to clean up temporary jars.
+ program.close();
Review comment:
this will never throw an exception, and even if then we wouldn't want to rethrow it because it would skip `super.cleanupDirectories`.
----------------------------------------------------------------
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
[GitHub] [flink] zentol commented on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
zentol commented on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770373198
This is insufficient. There is at least 1 code path in each application-cluster entry point class, where the execution is configured, which would not clean up the resources.
Furthermore, given that the default `close()` implementation calls `closeAsync()` we can conclude that `closeAsync()` does not call `close()`. As such, the added code is unlikely to ever be executed because most(all?) components appear to work against `closeAsync()`.
To top things off, if ClusterEntrypoint#startCluster fails we call `shutdownAsync()`, which never calls into `close[Async]()`.
----------------------------------------------------------------
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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12707",
"triggerID" : "6916a3097fb35e9db0585aa41e7e1fb5bce541f3",
"triggerType" : "PUSH"
}, {
"hash" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12863",
"triggerID" : "8b73e5d70f29adcefc2bd1c3ce673141b94b0213",
"triggerType" : "PUSH"
}, {
"hash" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869",
"triggerID" : "90ef99a414d6805f625d07f8f9a180c9617fecfb",
"triggerType" : "PUSH"
}, {
"hash" : "aa75cbfa98c0494578283d2dab0908cdd4942c3a",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "aa75cbfa98c0494578283d2dab0908cdd4942c3a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 90ef99a414d6805f625d07f8f9a180c9617fecfb Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12869)
* aa75cbfa98c0494578283d2dab0908cdd4942c3a 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
[GitHub] [flink] flinkbot edited a comment on pull request #14821: [FLINK-21210][coordination] ApplicationClusterEntryPoint should explicitly close PackagedProgram
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14821:
URL: https://github.com/apache/flink/pull/14821#issuecomment-770351339
----------------------------------------------------------------
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