Be smarter about when to use all cluster nodes vs just those specified by ha-mode:nodes when picking initial queue master node

Author: lukebakken

src/rabbit_mirror_queue_misc.erl

@@ -20,7 +20,8 @@
 -export([remove_from_queue/3, on_node_up/0, add_mirrors/3,
          report_deaths/4, store_updated_slaves/1,
          initial_queue_node/2, suggested_queue_nodes/1, actual_queue_nodes/1,
-         is_mirrored/1, update_mirrors/2, update_mirrors/1, validate_policy/1,
+         is_mirrored/1, is_mirrored_ha_nodes/1,
+         update_mirrors/2, update_mirrors/1, validate_policy/1,
          maybe_auto_sync/1, maybe_drop_master_after_sync/1,
          sync_batch_size/1, log_info/3, log_warning/3]).
 
@@ -31,6 +32,8 @@
 
 -include("rabbit.hrl").
 
+-define(HA_NODES_MODULE, rabbit_mirror_queue_mode_nodes).
+
 -rabbit_boot_step(
    {?MODULE,
     [{description, "HA policy validation"},
@@ -355,6 +358,12 @@ is_mirrored(Q) ->
         _        -> false
     end.
 
+is_mirrored_ha_nodes(Q) ->
+    case module(Q) of
+        {ok, ?HA_NODES_MODULE} -> true;
+        _ -> false
+    end.
+
 actual_queue_nodes(#amqqueue{pid             = MPid,
                              slave_pids      = SPids,
                              sync_slave_pids = SSPids}) ->

src/rabbit_queue_master_location_misc.erl

@@ -93,6 +93,16 @@ get_location_mod_by_config(#amqqueue{}) ->
     end.
 
 all_nodes(Queue = #amqqueue{}) ->
+    handle_is_mirrored_ha_nodes(rabbit_mirror_queue_misc:is_mirrored_ha_nodes(Queue), Queue).
+
+handle_is_mirrored_ha_nodes(false, _Queue) ->
+    % Note: ha-mode is NOT 'nodes' - it is either exactly or all, which means
+    % that any node in the cluster is eligible to be the new queue master node
+    rabbit_nodes:all_running();
+handle_is_mirrored_ha_nodes(true, Queue) ->
+    % Note: ha-mode is 'nodes', which explicitly specifies allowed nodes.
+    % We must use suggested_queue_nodes to get that list of nodes as the
+    % starting point for finding the queue master location
     handle_suggested_queue_nodes(rabbit_mirror_queue_misc:suggested_queue_nodes(Queue)).
 
 handle_suggested_queue_nodes({_MNode, []}) ->

test/queue_master_location_SUITE.erl

@@ -186,13 +186,8 @@ declare_policy_exactly(Config) ->
                                                 rabbit_queue_master_location_misc,
                                                 lookup_master, [Q, ?DEFAULT_VHOST_PATH]),
     ?assertEqual(MNode0, MNode1),
-    MIdx = rabbit_ct_broker_helpers:nodename_to_index(Config, MNode1),
-    SIdx = rabbit_ct_broker_helpers:nodename_to_index(Config, SNode),
-    % Note:
-    % We know that the nodes are indexed in the configuration in such a way
-    % that higher indexes have fewer queues assigned to them
-    % Node0 has 15 queues, Node1 has 8 and Node2 has 1
-    ?assert(MIdx > SIdx).
+    Node2 = rabbit_ct_broker_helpers:get_node_config(Config, 2, nodename),
+    ?assertEqual(MNode1, Node2).
 
 declare_config(Config) ->
     setup_test_environment(Config),