[GitHub] flink pull request #5703: [FLINK-8915] CheckpointingStatisticsHandler fails ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/5703


---

[GitHub] flink pull request #5703: [FLINK-8915] CheckpointingStatisticsHandler fails ...

[GJL]

Github user GJL commented on a diff in the pull request:

https://github.com/apache/flink/pull/5703#discussion_r175125460
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointStatistics.java
 ---
@@ -438,4 +453,58 @@ public int hashCode() {
return Objects.hash(super.hashCode(), failureTimestamp, 
failureMessage);
}
}
+
+   /**
+* Statistics for a pending checkpoint.
+*/
+   public static final class PendingCheckpointStatistics extends 
CheckpointStatistics {
--- End diff --

Ser/des of this class should be tested in `CheckpointingStatisticsTest`.


---

[GitHub] flink pull request #5703: [FLINK-8915] CheckpointingStatisticsHandler fails ...

[GJL]

Github user GJL commented on a diff in the pull request:

https://github.com/apache/flink/pull/5703#discussion_r175126334
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointStatistics.java
 ---
@@ -438,4 +453,58 @@ public int hashCode() {
return Objects.hash(super.hashCode(), failureTimestamp, 
failureMessage);
}
}
+
+   /**
+* Statistics for a pending checkpoint.
+*/
+   public static final class PendingCheckpointStatistics extends 
CheckpointStatistics {
--- End diff --

`@JsonSubTypes.Type(value = 
CheckpointStatistics.PendingCheckpointStatistics.class, name = "in_progress")` 
should be added to `CheckpointStatistics` otherwise deserialization will fail.


---

[GitHub] flink pull request #5703: [FLINK-8915] CheckpointingStatisticsHandler fails ...

[GJL]

Github user GJL commented on a diff in the pull request:

https://github.com/apache/flink/pull/5703#discussion_r175126066
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointStatistics.java
 ---
@@ -273,7 +274,21 @@ public static CheckpointStatistics 
generateCheckpointStatistics(AbstractCheckpoi
failedCheckpointStats.getFailureTimestamp(),
failedCheckpointStats.getFailureMessage());
} else {
-   throw new IllegalArgumentException("Given checkpoint 
stats object of type " + checkpointStats.getClass().getName() + " cannot be 
converted.");
+   final PendingCheckpointStats pendingCheckpointStats = 
((PendingCheckpointStats) checkpointStats);
--- End diff --

The original `else` block should be kept in case the class hierarchy is 
extended.
```
else if (checkpointStats instanceOf PendingCheckpointStats) {
...
} else {
throw new IllegalArgumentException("Given checkpoint stats object of 
type " + checkpointStats.getClass().getName() + " cannot be converted.");
}
```


---

[GitHub] flink pull request #5703: [FLINK-8915] CheckpointingStatisticsHandler fails ...

[yanghua]

GitHub user yanghua opened a pull request:

https://github.com/apache/flink/pull/5703

[FLINK-8915] CheckpointingStatisticsHandler fails to return 
PendingCheckpointStats

## What is the purpose of the change

*This pull request fixed a issue : CheckpointingStatisticsHandler fails to 
return PendingCheckpointStats*


## Brief change log

  - *defined a new static inner class named `PendingCheckpointStatistics`*
  - *replaced the pre-process logic (throw a exception ) by return the 
instance of `PendingCheckpointStatistics`*


## Verifying this change

This change is already covered by existing tests, such as 
*`CheckpointingStatisticsTest`*.


## 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, 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**)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/yanghua/flink FLINK-8915

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/5703.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #5703


commit 1bec2777b5f2d689eed51cbe0fbc4275ebcacab1
Author: yanghua 
Date:   2018-03-15T08:24:35Z

[FLINK-8915] CheckpointingStatisticsHandler fails to return 
PendingCheckpointStats




---