StefanRRichter commented on a change in pull request #7970:
[FLINK-11874][checkpoint] Split CheckpointStorage interface to distinguish JM
and TM side
URL: https://github.com/apache/flink/pull/7970#discussion_r265038472
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
##########
@@ -246,8 +247,12 @@ public CheckpointCoordinator(
this.checkpointProperties =
CheckpointProperties.forCheckpoint(retentionPolicy);
try {
- this.checkpointStorage =
checkpointStateBackend.createCheckpointStorage(job);
+ this.checkpointStorage = (CheckpointCoordinatorStorage)
checkpointStateBackend.createCheckpointStorage(job);
Review comment:
I think we can avoid the cast and problems that come with it. If we look at
the situation, starting from the old `CheckpointStorage`, it seems that such
objects implement two side, one that is more of an administration interface
(for JM) and one that is more like a client interface (for TMs). I would think
that a class that implements only one part of this does not make too much
sense. So why not have an interface `CheckpointStorage extends
CheckpointStorageCoordinatorView, CheckpointStorageClientView`. Coordinator
view is the JM side, client view the TM side, the interface that creates the
object returns CheckpointStorage and on both sides, after we create the object
we immediately assign it to either a variable of type
`CheckpointStorageCoordinatorView` or `CheckpointStorageClientView`, so that
from this point the code only sees the view that is for it. This will eliminate
the need for cast and also ensure that every `CheckpointStorage` must implement
both sub-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:
[email protected]
With regards,
Apache Git Services
