[erlang-questions] gen_statem and multiple timeout vs 1

[Vans S]

> Your mail client really messed the code up so I had to reformat it
> before reading it...


Il make sure to send email as plain text.


> handle_event(internal, {timeout,TimerRef,keep_alive, S, #{named_timeout := #{keep_alive := TimerRef}} = D) ->

I looked at timer functions and something interesting is I don't see any way to retrieve the remaining time of a TimerRef,
nor any way to bump it.  Maybe I missed something?


Another possible option is {named_timeout, incr|decr|set, 15000, {built, BuildUUID}}, so the 3 size tuple implies set.

This does not solve referencing the time thought.  

If named_timeout/n returns a TimerRef, we save that in our state, and there is a way to incr/decr/set/time_remaining it. 
I think then this is ideal.

Maybe also expose named_timeout/2 which will be simpler and return {named_timeout, 15000, Name}. 
For those who do not want to track TimerRef.


A possible extra consideration is if we want to pass extra non-key data for the timer callback. Right now we can only pass 
Name, and we consider Name the timer key.  What if we want something like this {named_timeout, 15000, {built, BuildUUID, BuildingUniqueState}}.

This way we MAY gain some extra flexibility.  I can't think of a practical use case now, but there may be.  Now we cannot cancel this timer. As 
BuildingUniqueState is not referenced anywhere except inside the callback.  Making a 4 size tuple can remedy this.  I am not sure 
where this fits thought.  Anyways this is another consideration that might have a use.


{named_timeout, 15000, {built, BuildUUID}, BuildingUniqueState}





On Friday, October 7, 2016 4:42 AM, Raimo Niskanen <raimo+erlang-questions@REDACTED> wrote:



On Thu, Oct 06, 2016 at 03:46:55PM +0000, Vans S wrote:
> Okay going to take another attempt at it. In evented mode.
> 
> Some points before we get into it. gen_statem fits here because it ensures 2 key points.
> 
> #1 If we get any message while in waiting_socket state, we should not process it / crash / drop it.
> #2 We need to transition out of waiting_socket state, before processing future messages.
> 
> This code might be off as I did not test it, so treat as pseudo code.
> 

Your mail client really messed the code up so I had to reformat it
before reading it...


init() ->
   {ok, waiting_socket, #{},
     [{state_timeout, 2000, no_socket_passed}]}.

%% Timeouts
%% Crash here or handle ourselves
handle_event(state_timeout, no_socket_passed, waiting_socket, D) ->
    {stop, {shutdown, tcp_closed}, D};
%% Crash here or handle, we should NEVER get any message here
handle_event(_, _, waiting_socket, D) ->
    {stop, {shutdown, tcp_closed}, D};
handle_event(named_timeout, endpoint_timeout, _, D) ->
    {stop, {shutdown, tcp_closed}, D};

%% Basic socket ops
handle_event(info, {pass_socket, Socket}, waiting_socket, D) ->
    {next_state, established, D#{socket=> Socket},
      [{named_timeout, 15000, keep_alive},
       {named_timeout, 120000, endpoint_timeout}]};

handle_event(named_timeout, keep_alive, S, D) ->
    send_keepalive(),
    {next_state, S, D, [{named_timeout, 15000, keep_alive}]};

%% Login chain
%% Moves our state to 'ingame' if success
%% Not implemented in example

%% Ingame
handle_event(info, {tcp, Socket, Bin}, ingame, D) ->
   case proc_on_bin(Bin) of
      {build, Building, X, Y} ->
           {BuildUUID, BuildTime} = get_build_time(Building),
           register_build_uuid(BuildUUID, Building, X, Y),        
           %% Make some helper functions to make this cleaner
           % A cleaner option is to send a message to mailbox
           % after parsing packet
           {next_state, ingame, D,
            [{named_timeout, BuildTime, {built, BuildUUID}},
         {named_timeout, 15000, keep_alive},
         {named_timeout, 120000, endpoint_timeout}]};
      {cancel_build, BuildUUID} ->
           unregister_build_uuid(BuildUUID),
           {next_state, ingame, D, [
               {named_timeout, infinity, BuildUUID},
               {named_timeout, 15000, keep_alive},
               {named_timeout, 120000, endpoint_timeout}
           ]}
   end;
handle_event(named_timeout, {built, BuildUUID}, ingame, D) ->
    Socket = maps:get(socket, D),
    notify_client_building_upgraded(Socket, BuildUUID),
    {next_state, ingame, D};

> 
> 
> To me this named_timeout really fits this model, keeps the code clean, keeps it all in one place. state_timeout helps us correctly transition our states from outgame/ingame/lobby/etc.

I also like the look of the named timeouts in that code.  It is quite readable.

How to bump a timer is not solved, but I think needs to be solved for
state_timeout anyway...

Solving it with erlang:start_timer/3,4 might look like this
(selected parts):


handle_event(
  internal, {timeout,TimerRef,keep_alive, S,
  #{named_timeout := #{keep_alive := TimerRef}} = D) ->
    send_keepalive(),
    {next_state, S, named_timeout(1500, keep_alive, D)};

handle_event(info, {tcp,Socket,Bin}, ingame, D) ->
    ...
    {next_state, ingame,
     named_timeout(
     BuildTime, {built,BuildUUID},
         named_timeout(
         15000, keep_alive,
         named_timeout, 120000, endpoint_timeout, D)};
handle_event(
  internal, {timeout,TimerRef,{built,BuildUUID} = Built}, S, D) ->
    case D of
        #{named_timeout := #{Built := TimerRef}} ->
        Socket = maps:get(socket, D),
        notify_client_building_upgraded(Socked, BuildUUID),
        {next_state, ingame, named_timeout(TimerRef, Built, D)};
    _ ->
        %% What to do with unexpected timeout...
    end;


The named_timeout/3 helper function remains to be written, matching
the timer event is much messier, and you need to remember to remove
the TimerRef from the state when you get the timeout event...

So, named_timeout would make timer handling easier.
The question is if it will be flexible enough?

> Possibly we could have 1 process for loggedout/outgame/lobby and 1 process for ingame, so we can chat while playing.
> The original 'timeout' seems out of place, only use case I see for it is if the process is idle to hibernate it, or to kill the process after a certain time, or other similar.
> 
> So to summarize, gen_statem allows us to guarantee state transitions even if other messages arrive to mailbox.  Thus if we were to implement this code using processes or gen_server we
> would be recreating a lot of gen_statem.  Using the handle_event_function callback mode allows us to have this kind of flexible gen_statem.
> 
> This is my final argument in support of named_timeout.  Thank you for your attention.

Your case is certainly worth considering.  We'll start with completing
state_timeout and then we will see...



-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
_______________________________________________
erlang-questions mailing list
erlang-questions@REDACTED
http://erlang.org/mailman/listinfo/erlang-questions