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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
