[ 
https://issues.apache.org/jira/browse/FLINK-5982?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15939521#comment-15939521
 ] 

Wei-Che Wei commented on FLINK-5982:
------------------------------------

Hi [~till.rohrmann]

Thanks for your replying. For the first question, I sill feel a little confused 
and I would like to getting your suggestion. Thank you.

# It's a good choice to check passed environment in {{AbstractInvokable}} and 
pass a {{DummyEnvironment}} instead of {{null}}. I am not sure if it is good to 
state instances as well, since sometimes the state instances will be {{null}} 
in {{Task}}. If checking state instances are not {{null}}, it might need to add 
one more constructor or pass an empty state object in {{Task}}. What do you 
think?
# The exception is {{IllegalStateException("Found operator state for a 
non-stateful task invokable")}}. Now, I only put it in {{BatchTask}} not it the 
testing classes. I think it would not change their behavior. I will check it 
once again before I finish this task.
# What I want to do is leave it empty and let those classes who need to support 
the state methods to override them, but I think your way is better. I will 
implement like that and let the state methods in {{AbstractInvokable}} be 
abstract methods.

> Refactor AbstractInvokable and StatefulTask
> -------------------------------------------
>
>                 Key: FLINK-5982
>                 URL: https://issues.apache.org/jira/browse/FLINK-5982
>             Project: Flink
>          Issue Type: Improvement
>          Components: Distributed Coordination, State Backends, Checkpointing
>    Affects Versions: 1.2.0
>            Reporter: Wei-Che Wei
>            Assignee: Wei-Che Wei
>
> Currently, running a invokable in {{Task}} needs to call 
> {{setEnvironment(env)}} and {{invoke()}}. If the invokable is also a 
> {{StatefulTask}}, it need to call {{setInitialState(state)}}. That makes the 
> difficulty in doing the eager initialization on invokable during 
> {{DEPLOYING}} state. One solution discussed in FLINK-4714 is to separate 
> {{invoke()}} into {{open()}} and {{invoke()}}, but that makes the complexity 
> for running it in {{Task}}.
> This task wants to refactor {{AbstractInvokable}} and {{StatefulTask}} to 
> make it easier to construct and run an invokable.
> # Refactor abstract class to have one default constructor.
> #* Drop {{StatefulTask}} and assume all subclasses of {{AbstractInvokable}} 
> are stateful.
> #* Remove {{setEnvironment(env)}} and {{setInitialState(state)}}. Make 
> {{AbstractInvokable}} have a two argument constructor with {{Environment}} 
> and {{TaskStateHandles}}.
> # Update all subclass
> #* Make all subclass of {{AbstractInvokable}} have a two argument constructor 
> and call the constructor in {{AbstractInvokable}}.
> #* Throw an error in {{BatchTask}} if the initial state is not null. (This 
> will be removed after {{BatchTask}} have been stateful.)
> # Change the creation of the invokable to call that constructor, update all 
> the tests.
> Then, we can simplify the logic to run an invokable by using constructor and 
> {{invoke()}}. The eager initialization can easily be placed in the 
> constructor to fulfill the requirement such as FLINK-4714.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to