flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-5023) Add get() method in State interface
Date Wed, 25 Jan 2017 15:13:26 GMT

    [ 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)

Mime
View raw message