flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From StefanRRichter <...@git.apache.org>
Subject [GitHub] flink pull request #5248: [FLINK-5823] [checkpoints] State backends now also...
Date Wed, 10 Jan 2018 16:40:48 GMT
Github user StefanRRichter commented on a diff in the pull request:

    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/state/StateBackend.java
    @@ -84,13 +84,44 @@
     public interface StateBackend extends java.io.Serializable {
     	// ------------------------------------------------------------------------
    -	//  Persistent Bytes Storage
    +	//  Checkpoint storage - the durable persistence of checkpoint data
    --- End diff --
    The interface `StateBackend` is getting more crowded and more functionality is added and
all methods are visible to everybody from `CheckpointCoordinator` down to the `XYZKeyedStateBackend`.
You have already divided the interface into 3 segments and I suggest to actually split it
into 3 parent interfaces, which are all implemented by `StateBackend`. In many places, we
can just pass by the interface of the sub-functionality that is relevant, e.g. `XYZKeyedStateBackend`
probably only needs to see to part about creating streams and should never call things that
create a new backend. This would be  a big step for separation of concerns, to move away from
"global visibility". Our IDE should be able to automatically figure out all the places where
the sub-interfaces are sufficient, but as it might be a bigger diff, we could do a separate
PR for this.


View raw message