"nodes" policy should not suggest nodes which are not running. Also if nodes policy is completely unconnected with reality, just keep the master alive rather than blow up.

2 files changed, 19 insertions, 5 deletions

diff --git a/src/rabbit_mirror_queue_misc.erl b/src/rabbit_mirror_queue_misc.erl
index 090948e6..3b25df6a 100644
--- a/src/rabbit_mirror_queue_misc.erl
+++ b/src/rabbit_mirror_queue_misc.erl
@@ -244,11 +244,18 @@ policy(Policy, Q) ->
 
 suggested_queue_nodes(<<"all">>, _Params, {MNode, _SNodes}, All) ->
     {MNode, All -- [MNode]};
-suggested_queue_nodes(<<"nodes">>, Nodes0, {MNode, _SNodes}, _All) ->
+suggested_queue_nodes(<<"nodes">>, Nodes0, {MNode, _SNodes}, All) ->
     Nodes = [list_to_atom(binary_to_list(Node)) || Node <- Nodes0],
-    case lists:member(MNode, Nodes) of
-        true  -> {MNode, Nodes -- [MNode]};
-        false -> promote_slave(Nodes)
+    Unavailable = Nodes -- All,
+    Available = Nodes -- Unavailable,
+    case Available of
+        [] -> %% We have never heard of anything? Not much we can do but
+              %% keep the master alive.
+              {MNode, []};
+        _  -> case lists:member(MNode, Available) of
+                  true  -> {MNode, Available -- [MNode]};
+                  false -> promote_slave(Available)
+              end
     end;
 suggested_queue_nodes(<<"exactly">>, Count, {MNode, SNodes}, All) ->
     SCount = Count - 1,
diff --git a/src/rabbit_tests.erl b/src/rabbit_tests.erl
index 6cb34b09..b1d549fb 100644
--- a/src/rabbit_tests.erl
+++ b/src/rabbit_tests.erl
@@ -898,10 +898,17 @@ test_dynamic_mirroring() ->
     Test({a,[b,c]},<<"all">>,'_',{a,[b,c]},[a,b,c]),
     Test({a,[b,c]},<<"all">>,'_',{a,[d]},  [a,b,c]),
 
-    Test({a,[b,c]},<<"nodes">>,[<<"a">>,<<"b">>,<<"c">>],{a,[d]},[a,b,c,d]),
+    %% Add a node
     Test({a,[b,c]},<<"nodes">>,[<<"a">>,<<"b">>,<<"c">>],{a,[b]},[a,b,c,d]),
     Test({b,[a,c]},<<"nodes">>,[<<"a">>,<<"b">>,<<"c">>],{b,[a]},[a,b,c,d]),
+    %% Add two nodes and drop one
+    Test({a,[b,c]},<<"nodes">>,[<<"a">>,<<"b">>,<<"c">>],{a,[d]},[a,b,c,d]),
+    %% Promote slave to master by policy
     Test({a,[b,c]},<<"nodes">>,[<<"a">>,<<"b">>,<<"c">>],{d,[a]},[a,b,c,d]),
+    %% Don't try to include nodes that are not running
+    Test({a,[b]},  <<"nodes">>,[<<"a">>,<<"b">>,<<"f">>],{a,[b]},[a,b,c,d]),
+    %% If we can't find any of the nodes listed then just keep the master
+    Test({a,[]},   <<"nodes">>,[<<"f">>,<<"g">>,<<"h">>],{a,[b]},[a,b,c,d]),
 
     Test({a,[b]},  <<"exactly">>,2,{a,[]},   [a,b,c,d]),
     Test({a,[b,c]},<<"exactly">>,3,{a,[]},   [a,b,c,d]),