[jira] [Comment Edited] (FLINK-12887) Schedule UnfencedMessage would lost envelope info

["Till Rohrmann (JIRA)" <ji...@apache.org>]

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

Till Rohrmann edited comment on FLINK-12887 at 7/24/19 12:26 PM:
-----------------------------------------------------------------

Ok, now I understand the problem. The problem is that we are not talking about Flink but your internal fork of Flink. I'm not sure whether it makes sense to report problems which are fork specific.

Some comments about the problematic use case and the proposed solution approach: Why wouldn't it work to simply call {{RpcEndpoint#scheduleRunAsync}} when the endpoint gains leadership? If it loses the leadership, then the clean up call won't be executed but another leader will eventually kill the stuck container.

You could pass in the {{ActorSystem}} of a local {{AkkaRpcActor}} to the {{AkkaInvocationHandler}}. However, what do you do if the actor is remote? You don't have access to the remote {{ActorSystem}} and hence it would not work. Of course you could add the former case as a special code path but I think that this breaks the abstractions of the {{AkkaInvocationHandler}} which should be separated from these implementation details.

I think you can apply these changes in your fork but I would be hesitant to do them in Flink because there is no reason to do so at the moment. Hence, I would be in favor of closing this issue.


was (Author: till.rohrmann):
Ok, now I understand the problem. The problem is that we are not talking about Flink but your internal fork of Flink. I'm not sure whether it makes sense to report problems which are fork specific.

Some comments about the problematic use case and the proposed solution approach: Why wouldn't it work to simply call {{RpcEndpoint#scheduleRunAsync}} when the endpoint gains leadership? If it loses the leadership, then the clean up call won't be executed but another leader will eventually kill the stuck container.

You could pass in the {{ActorSystem}} of a local {{AkkaRpcActor}} to the {{AkkaInvocationHandler}}. However, what do you do if the actor is remote? You don't have access to the remote {{ActorSystem}} and hence it would not work. Of course you could add the former case as a special code path but I think that this breaking the abstractions of the {{AkkaInvocationHandler}} which should be separated from these implementation details.

I think you can apply these changes in your fork but I would be hesitant to do them in Flink because there is no reason to do so at the moment. Hence, I would be in favor of closing this issue.

> Schedule UnfencedMessage would lost envelope info 
> --------------------------------------------------
>
>                 Key: FLINK-12887
>                 URL: https://issues.apache.org/jira/browse/FLINK-12887
>             Project: Flink
>          Issue Type: Bug
>          Components: Runtime / Coordination
>    Affects Versions: 1.9.0
>            Reporter: TisonKun
>            Assignee: TisonKun
>            Priority: Major
>
> We provide {{runAsync}}, {{callAsync}} and {{scheduleRunAsync}} for {{MainThreadExecutable}}, while providing {{runAsyncWithoutFencing}} and {{callAsyncWithoutFencing}} additionally for {{FencedMainThreadExecutable}}.
> Let's think about a case when we want to schedule a unfenced runnable or any other unfenced message(currently, we don't have such code path but it's semantically valid.). 
> 1. {{FencedAkkaRpcActor}} received an unfenced runnable with delay
> 2. It extracted the runnable from unfenced message and call {{super.handleRpcMessage}}.
> 3. {{AkkaRpcActor}} enveloped the message and schedule it by {{AkkaRpcActor#L410}}.
> However, {{FencedAkkaRpcActor#envelopeSelfMessage}} was called for envelope. Thus the unfenced message now become a fenced message.
> We can anyway implement {{scheduleRunAsyncWithoutFencing}} to schedule unfenced message directly by {{actorsystem.scheduler.scheduleOnce(..., dispatcher)}}, but with current codebase I notice that {{RunAsync}} has a wried {{atTimeNanos}}(i.e., delay) property. Ideally how to schedule a message is shown on what params ScheduleExecutorService called with, at least we cannot extract an unfenced message and envelop it into a fence message and then schedule it, which goes into wrong semantic.
> cc [~till.rohrmann]



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)