Merge pull request #627 from rabbitmq/handle-ra-system-down-rpc

Handle Ra system down gracefully on start_server

Author: kjnilsson

src/ra_server_sup_sup.erl

@@ -41,16 +41,17 @@
 
 -spec start_server(System :: atom(), ra_server:ra_server_config()) ->
     supervisor:startchild_ret() |
-    {error, not_new | system_not_started | invalid_initial_machine_version} |
-    {badrpc, term()}.
+    {error, not_new | system_not_started | nodedown |
+    invalid_initial_machine_version} | {badrpc, term()}.
 start_server(System, #{id := NodeId,
                        uid := UId} = Config)
   when is_atom(System) ->
     Node = ra_lib:ra_server_id_node(NodeId),
     rpc:call(Node, ?MODULE, start_server_rpc, [System, UId, Config]).
 
 -spec restart_server(atom(), ra_server_id(), ra_server:mutable_config()) ->
-    supervisor:startchild_ret() | {error, system_not_started} | {badrpc, term()}.
+    supervisor:startchild_ret() | {badrpc, term()} |
+    {error, system_not_started | nodedown}.
 restart_server(System, {RaName, Node}, AddConfig) ->
     rpc:call(Node, ?MODULE, restart_server_rpc,
              [System, {RaName, Node}, AddConfig]).
@@ -259,7 +260,7 @@ init([]) ->
 
 start_child(Name, Config) ->
     Ref = make_ref(),
-    case supervisor:start_child(Name, [Config#{reply_to => {Ref, self()}}]) of
+    try supervisor:start_child(Name, [Config#{reply_to => {Ref, self()}}]) of
         {ok, Pid} ->
             %% we have started the process now and have to wait for reply
             %% that is sent after init but before state machine recovery
@@ -275,4 +276,9 @@ start_child(Name, Config) ->
             end;
         Err ->
             Err
+    catch
+        exit:{noproc, _} ->
+            {error, system_not_started};
+        exit:{{nodedown, _}, _} ->
+            {error, nodedown}
     end.

test/ra_2_SUITE.erl

@@ -36,6 +36,7 @@ all_tests() ->
      cluster_is_deleted_with_server_down,
      cluster_cannot_be_deleted_in_minority,
      diverged_follower,
+     start_server_noproc,
      server_restart_after_application_restart,
      restarted_server_does_not_reissue_side_effects,
      recover,
@@ -954,6 +955,27 @@ enq_deq_n(N, ServerId, Acc) ->
     true = Deq /= empty,
     enq_deq_n(N-1, ServerId, [Deq | Acc]).
 
+start_server_noproc(Config) ->
+    ClusterName = ?config(cluster_name, Config),
+    PrivDir = ?config(priv_dir, Config),
+    ServerId = ?config(server_id, Config),
+    UId = ?config(uid, Config),
+    Conf = conf(ClusterName, UId, ServerId, PrivDir, []),
+    meck:new(ra_system, [passthrough]),
+    meck:expect(ra_system, lookup_name,
+                fun(?SYS, server_sup) ->
+                        {ok, ra_server_sup_sup_noproc_test};
+                   (Sys, Key) ->
+                        meck:passthrough([Sys, Key])
+                end),
+    try
+        ?assertEqual({error, system_not_started},
+                     ra:start_server(?SYS, Conf))
+    after
+        meck:unload(ra_system)
+    end,
+    ok.
+
 conf(ClusterName, UId, ServerId, _, Peers) ->
     #{cluster_name => ClusterName,
       id => ServerId,