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/10/08 17:47:18 UTC
[GitHub] [flink] rkhachatryan opened a new pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
rkhachatryan opened a new pull request #13565:
URL: https://github.com/apache/flink/pull/13565
## What is the purpose of the change
This PR includes changes from #13040 and some refactorings of new and existing code.
## Verifying this change
Changes for backpressure tested by new `ZooKeeperCompletedCheckpointStoreITCase.testChekpointingPausesAndResumeWhenTooManyCheckpoints` test
## 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/Mesos, ZooKeeper: yes
- 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
[GitHub] [flink] tillrohrmann commented on a change in pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #13565:
URL: https://github.com/apache/flink/pull/13565#discussion_r551400670
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CompletedCheckpointStore.java
##########
@@ -84,7 +85,7 @@ default CompletedCheckpoint getLatestCheckpoint(boolean isPreferCheckpointForRec
*
* @param jobStatus Job state on shut down
*/
- void shutdown(JobStatus jobStatus) throws Exception;
+ void shutdown(JobStatus jobStatus, CheckpointsCleaner checkpointsCleaner, Runnable postCleanup) throws Exception;
Review comment:
Why do we need the `postCleanup`? In the current code base I couldn't find a single usage where we don't pass `() -> {}` @rkhachatryan and @pnowojski? If this is not needed, can we get rid of it? I think it does not make a very nice API with the no-ops.
Moreover, it would be nice if we could update the JavaDocs if we change the signature of interfaces.
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705726642
----------------------------------------------------------------
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] etienne-chauchot commented on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
etienne-chauchot commented on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-707217508
@rkhachatryan I don't see my ITest commit in this history. Does this mean that you don't want to merge this part of the code or is this just a forgotten part?
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
}, {
"hash" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522",
"triggerID" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"triggerType" : "PUSH"
}, {
"hash" : "82386f6b770e774839f3c3d4d28de2019e072775",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "82386f6b770e774839f3c3d4d28de2019e072775",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2d4a962962cb0cbac8d0a3fbf8e242261184b7c8 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522)
* 82386f6b770e774839f3c3d4d28de2019e072775 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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
}, {
"hash" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522",
"triggerID" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"triggerType" : "PUSH"
}, {
"hash" : "82386f6b770e774839f3c3d4d28de2019e072775",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7596",
"triggerID" : "82386f6b770e774839f3c3d4d28de2019e072775",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2d4a962962cb0cbac8d0a3fbf8e242261184b7c8 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522)
* 82386f6b770e774839f3c3d4d28de2019e072775 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7596)
<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] etienne-chauchot edited a comment on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
etienne-chauchot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-707217508
@rkhachatryan I don't see my ITest commit in this history. Does this mean that you don't want to merge this part of the code or is it just that the commit was forgotten?
----------------------------------------------------------------
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] pnowojski commented on a change in pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
pnowojski commented on a change in pull request #13565:
URL: https://github.com/apache/flink/pull/13565#discussion_r503809655
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -531,6 +527,30 @@ private void dispose(boolean releaseState, CheckpointsCleaner checkpointsCleaner
}
}
+ @Override
+ public void discard() {
+ synchronized (lock) {
+ if (stateDiscarded) {
+ Preconditions.checkState(discarded, "Checkpoint should be disposed before being discarded");
+ return;
+ } else {
+ stateDiscarded = true;
+ }
+ }
Review comment:
Ok, I think I get it now. Now it's a public method so you are protecting it more.
I think I would prefer:
1. rename `discarded` -> `disposed` or `scheduledForDisacrding`
2. rename `stateDiscarded` -> `discarded`
?
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
}, {
"hash" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522",
"triggerID" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2d4a962962cb0cbac8d0a3fbf8e242261184b7c8 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522)
<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] etienne-chauchot commented on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
etienne-chauchot commented on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-707235444
> I think @etienne-chauchot your test was squashed with the commit that was adding the feature.
Ok fair 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
[GitHub] [flink] pnowojski commented on a change in pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
pnowojski commented on a change in pull request #13565:
URL: https://github.com/apache/flink/pull/13565#discussion_r503350616
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -531,6 +527,30 @@ private void dispose(boolean releaseState, CheckpointsCleaner checkpointsCleaner
}
}
+ @Override
+ public void discard() {
+ synchronized (lock) {
+ if (stateDiscarded) {
+ Preconditions.checkState(discarded, "Checkpoint should be disposed before being discarded");
+ return;
+ } else {
+ stateDiscarded = true;
+ }
+ }
Review comment:
Why do we need this check now? As far as I can tell, previously it was not needed?
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
}, {
"hash" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522",
"triggerID" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5feafbbf87c7cefaf7ebe3a36c1e3f346386d629 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304)
* 2d4a962962cb0cbac8d0a3fbf8e242261184b7c8 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522)
<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] rkhachatryan edited a comment on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
rkhachatryan edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-708400698
I squashed the commit but the build failed.
The reason is unrelated: FLINK-19619 (both public and private repos).
----------------------------------------------------------------
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] rkhachatryan commented on a change in pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on a change in pull request #13565:
URL: https://github.com/apache/flink/pull/13565#discussion_r551426315
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CompletedCheckpointStore.java
##########
@@ -84,7 +85,7 @@ default CompletedCheckpoint getLatestCheckpoint(boolean isPreferCheckpointForRec
*
* @param jobStatus Job state on shut down
*/
- void shutdown(JobStatus jobStatus) throws Exception;
+ void shutdown(JobStatus jobStatus, CheckpointsCleaner checkpointsCleaner, Runnable postCleanup) throws Exception;
Review comment:
`postCleanup` is used by `checkpointsCleaner` but not in case of shutting down. I agree, it should be removed (by pushing down to `DefaultCompletedCheckpointStore`) and javadoc added. My bad.
I've created FLINK-20847 to address this.
----------------------------------------------------------------
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] pnowojski merged pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
pnowojski merged pull request #13565:
URL: https://github.com/apache/flink/pull/13565
----------------------------------------------------------------
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] pnowojski commented on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
pnowojski commented on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-708439967
Thanks for the contribution both @rkhachatryan and @etienne-chauchot . Merging.
----------------------------------------------------------------
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] rkhachatryan commented on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-708400698
CI failure unrelated - FLINK-19619 (both public and private repos).
----------------------------------------------------------------
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] rkhachatryan commented on a change in pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on a change in pull request #13565:
URL: https://github.com/apache/flink/pull/13565#discussion_r503730533
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -531,6 +527,30 @@ private void dispose(boolean releaseState, CheckpointsCleaner checkpointsCleaner
}
}
+ @Override
+ public void discard() {
+ synchronized (lock) {
+ if (stateDiscarded) {
+ Preconditions.checkState(discarded, "Checkpoint should be disposed before being discarded");
+ return;
+ } else {
+ stateDiscarded = true;
+ }
+ }
Review comment:
Yes, previously it wasn't needed. I added it to avoid future mistakes.
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5feafbbf87c7cefaf7ebe3a36c1e3f346386d629 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304)
<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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705726642
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 82386f6b770e774839f3c3d4d28de2019e072775 (Fri May 28 07:00: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] rkhachatryan edited a comment on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
rkhachatryan edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-708400698
@pnowojski, I squashed the commit but the build failed.
The reason is unrelated: FLINK-19619 (both public and private repos).
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5feafbbf87c7cefaf7ebe3a36c1e3f346386d629 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304)
<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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5feafbbf87c7cefaf7ebe3a36c1e3f346386d629 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] rkhachatryan commented on a change in pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on a change in pull request #13565:
URL: https://github.com/apache/flink/pull/13565#discussion_r503866063
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -531,6 +527,30 @@ private void dispose(boolean releaseState, CheckpointsCleaner checkpointsCleaner
}
}
+ @Override
+ public void discard() {
+ synchronized (lock) {
+ if (stateDiscarded) {
+ Preconditions.checkState(discarded, "Checkpoint should be disposed before being discarded");
+ return;
+ } else {
+ stateDiscarded = true;
+ }
+ }
Review comment:
I agree (just wanted to avoid touching additional files calling getter).
Added a fixup commit on top.
----------------------------------------------------------------
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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
}, {
"hash" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7522",
"triggerID" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"triggerType" : "PUSH"
}, {
"hash" : "82386f6b770e774839f3c3d4d28de2019e072775",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7596",
"triggerID" : "82386f6b770e774839f3c3d4d28de2019e072775",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 82386f6b770e774839f3c3d4d28de2019e072775 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7596)
<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 #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705726642
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 5feafbbf87c7cefaf7ebe3a36c1e3f346386d629 (Thu Oct 08 17:49:22 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
[GitHub] [flink] flinkbot edited a comment on pull request #13565: [FLINK-17073][checkpointing] Backpressure checkpointing on slow cleanup
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13565:
URL: https://github.com/apache/flink/pull/13565#issuecomment-705730164
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304",
"triggerID" : "5feafbbf87c7cefaf7ebe3a36c1e3f346386d629",
"triggerType" : "PUSH"
}, {
"hash" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2d4a962962cb0cbac8d0a3fbf8e242261184b7c8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5feafbbf87c7cefaf7ebe3a36c1e3f346386d629 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7304)
* 2d4a962962cb0cbac8d0a3fbf8e242261184b7c8 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