[GitHub] [flink] gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true) returns wrong checkpoint
gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true) returns wrong checkpoint URL: https://github.com/apache/flink/pull/9727#issuecomment-53399 If no objections I will merge this later today :) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true) returns wrong checkpoint
gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true)
returns wrong checkpoint
URL: https://github.com/apache/flink/pull/9727#issuecomment-533599941
> Thanks for your updates @gyfora. Tests part looks good to me. I will share
my version of `getLatestCheckpoint`. You're the man decide which one to adopt.
>
> ```java
>default CompletedCheckpoint getLatestCheckpoint(boolean
isPreferCheckpointForRecovery) throws Exception {
>List allCheckpoints = getAllCheckpoints();
>
>if (allCheckpoints.isEmpty()) {
>return null;
>}
>
>Predicate isSavepoint = i ->
allCheckpoints.get(i).getProperties().isSavepoint();
>
>int i = allCheckpoints.size() - 1;
>while (isPreferCheckpointForRecovery && i > 0 &&
isSavepoint.test(i)) {
>i--;
>}
>
>CompletedCheckpoint checkpoint = allCheckpoints.get(i);
>
>LOG.info(
>"Use {} ({}) to recover.",
>checkpoint.getProperties().isSavepoint() ? "savepoint"
: "checkpoint",
>checkpoint);
>
>return checkpoint;
>}
> ```
I don't think one is much better than the other but I feel that from a
readability perspective my proposal is slightly better. It also includes some
nice logging to know when we actually preferred a checkpoint or when we fell
back to a savepoint anyways.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true) returns wrong checkpoint
gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true) returns wrong checkpoint URL: https://github.com/apache/flink/pull/9727#issuecomment-533572489 @TisonKun @1u0 I pushed a new commit that addresses the comments + adds more tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true) returns wrong checkpoint
gyfora commented on issue #9727: [FLINK-14145] Fix getLatestCheckpoint(true) returns wrong checkpoint URL: https://github.com/apache/flink/pull/9727#issuecomment-533556876 Sure I can probably clean this up if a bit. I wanted to the minimal change that makes this correct so I added the explicit return in case the last entry is a checkpoint. 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: us...@infra.apache.org With regards, Apache Git Services
