[
https://issues.apache.org/jira/browse/FLINK-5023?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15837881#comment-15837881
]
ASF GitHub Bot commented on FLINK-5023:
---------------------------------------
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2768
Hi @shixiaogang !
I went through this pull request and below are a few thoughts. The pull
request changes many things together. Some can work, and for others I would
suggest to do it differently.
Let me know what you think about this:
## Changing the `StateDescriptor`
This is probably a good cleanup. It does currently break the backwards
compatibility, because Flink 1.1 wrote the state descriptors into the
checkpoints. Flink 1.2 and 1.3 do not do that any more, but currently rely on
the unchanged StateDescriptor classes for resuming Flink-1.1-savepoints.
We added a way to define "migration" classes, meaning we can store the old
StateDescriptor classes in a migration package where they are dynamically
loaded only as proxies when resuming a Flink-1.1 savepoint. That way we can
change the classes and maintain backwards compatibility.
## Removing `clear()` from`State`
I think it would be nice to keep `clear()` on the base `State` interface.
Can you explain why you want to remove it? In my opinion, every state needs to
be able to clear, so it makes sense to have this on the case interface.
If this is in preparation for the `MapState`, then the `MapState` can
simply override the `clear()` method with different logic.
- I think that the MapState needs to support clear as well, where it
deletes the sub-map for the current key.
- On the HeapStateBackend, this is quite easy, when we assume that each
(key/namespace) has a map as the value (and the complete map can be dropped)
- For the RocksDBStateBackend, it is a bit more expensive, and would
correspond to a range-iteration-and-deletes.
If an application decides that the MapState clear() is to expensive, it can
decide to not call it. But we should still support it for cases where it is
necessary.
## Changing the `State` interface
I would like to not change `State` to `State<T>` in the Flink master now,
because it cases warnings in all parts of the code (that suddenly use raw
types) and for some user programs.
While this would be done for Flink 2.0, it would make merging simpler if we
don't change it now.
> Add get() method in State interface
> -----------------------------------
>
> Key: FLINK-5023
> URL: https://issues.apache.org/jira/browse/FLINK-5023
> Project: Flink
> Issue Type: Improvement
> Components: State Backends, Checkpointing
> Reporter: Xiaogang Shi
> Assignee: Xiaogang Shi
>
> Currently, the only method provided by the State interface is `clear()`. I
> think we should provide another method called `get()` to return the
> structured value (e.g., value, list, or map) under the current key.
> In fact, the functionality of `get()` has already been implemented in all
> types of states: e.g., `value()` in ValueState and `get()` in ListState. The
> modification to the interface can better abstract these states.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
