Exception thrown in ra:process_command() when leader stop

[shino]

When trying to reproduce #309 (which failed), I encountered exceptions thrown in ra:process_command():

  !!!!!!! write error by exception !!!!!!!!!: {exit,
                                             {shutdown,
                                              {gen_statem,call,
                                               [{repro,'n1@127.0.0.1'},
                                                {leader_call,
                                                 {command,normal,
                                                  {'$usr',
                                                   {write,51,51},
                                                   await_consensus}}},
                                                100]}},
                                             [{gen,do_call,4,
                                               [{file,"gen.erl"},{line,266}]},
                                              {gen_statem,
                                               '-call_clean/4-fun-0-',5,
                                               [{file,"gen_statem.erl"},
                                                {line,917}]}]}

Script to reproduce:

• https://gist.github.com/shino/f8716ac57d28a4a5b483674ed4b3e1c3
• Short summary of this script:
  • start three nodes (n1, n2, n3)
  • spawn writer process
  • stop the leader node (n1) and start another (n4), when writer running

Steps to reproduce:

• git clone ra and make it
• execute the script above with the argument of ra path, for example
  erlc repro.erl && erl -s repro main "/path/to/ra" -s init stop
• Search write error by exception from its output

Discussion point: Is the exception above should be catched and converted?

• The exception is, I think, not strictly a bug but a matter of design.
• In ra_server_proc:gen_statem_safe_call() https://github.com/rabbitmq/ra/blob/main/src/ra_server_proc.erl#L1414-L1424,
  it catches some exceptions and covert them to normal values
• These catches seems to cover "normal" (in distributed systems sense) paths
  (and I like this design of converting to normal values 😄)
• The exception above happened in "normal" path, leader just stopped

Thanks!

[michaelklishin]

Leader failure can be expressed as an exception. I guess return values would be easier to reason about and handle, so why not convert them, too. It would be a non-trivial API change, though.

@kjnilsson what do you think?

[kjnilsson]

sounds reasonable to me