Merge pull request #1542 from rabbitmq/master-locator-min-masters-group-bindings-3.7v3.7.4

Make min-masters locator count uniqueue queue names instead of bindings 3.7

2 files changed, 42 insertions, 11 deletions

diff --git a/src/rabbit_queue_location_min_masters.erl b/src/rabbit_queue_location_min_masters.erl
index 5d6f568d4a..c532714d41 100644
--- a/src/rabbit_queue_location_min_masters.erl
+++ b/src/rabbit_queue_location_min_masters.erl
@@ -57,23 +57,26 @@ count_masters(Node, Masters) ->
 get_bound_queue_masters_per_vhost([], Acc) ->
     lists:flatten(Acc);
 get_bound_queue_masters_per_vhost([VHost|RemVHosts], Acc) ->
-    Bindings          = rabbit_binding:list(VHost),
-    BoundQueueMasters = get_queue_master_per_binding(VHost, Bindings, []),
+    BoundQueueNames =
+        lists:filtermap(
+            fun(#binding{destination =#resource{kind = queue,
+                                                name = QueueName}}) ->
+                    {true, QueueName};
+               (_) ->
+                    false
+            end,
+            rabbit_binding:list(VHost)),
+    UniqQueueNames = lists:usort(BoundQueueNames),
+    BoundQueueMasters = get_queue_masters(VHost, UniqQueueNames, []),
     get_bound_queue_masters_per_vhost(RemVHosts, [BoundQueueMasters|Acc]).
 
 
-get_queue_master_per_binding(_VHost, [], BoundQueueNodes) -> BoundQueueNodes;
-get_queue_master_per_binding(VHost, [#binding{destination=
-                                                  #resource{kind=queue,
-                                                            name=QueueName}}|
-                                     RemBindings],
-                             QueueMastersAcc) ->
+get_queue_masters(_VHost, [], BoundQueueNodes) -> BoundQueueNodes;
+get_queue_masters(VHost, [QueueName | RemQueueNames], QueueMastersAcc) ->
     QueueMastersAcc0 = case rabbit_queue_master_location_misc:lookup_master(
                               QueueName, VHost) of
                            {ok, Master} when is_atom(Master) ->
                                [Master|QueueMastersAcc];
                            _ -> QueueMastersAcc
                        end,
-    get_queue_master_per_binding(VHost, RemBindings, QueueMastersAcc0);
-get_queue_master_per_binding(VHost, [_|RemBindings], QueueMastersAcc) ->
-    get_queue_master_per_binding(VHost, RemBindings, QueueMastersAcc).
+    get_queue_masters(VHost, RemQueueNames, QueueMastersAcc0).
diff --git a/test/queue_master_location_SUITE.erl b/test/queue_master_location_SUITE.erl
index 457cf47b0c..94101567f6 100644
--- a/test/queue_master_location_SUITE.erl
+++ b/test/queue_master_location_SUITE.erl
@@ -56,6 +56,7 @@ groups() ->
           declare_policy_exactly,
           declare_config,
           calculate_min_master,
+          calculate_min_master_with_bindings,
           calculate_random,
           calculate_client_local
         ]}
@@ -210,6 +211,22 @@ calculate_min_master(Config) ->
     verify_min_master(Config, Q),
     ok.
 
+calculate_min_master_with_bindings(Config) ->
+    setup_test_environment(Config),
+    QueueName = rabbit_misc:r(<<"/">>, queue, Q = <<"qm.test_bound">>),
+    Args = [{<<"x-queue-master-locator">>, longstr, <<"min-masters">>}],
+    declare(Config, QueueName, false, false, Args, none),
+    verify_min_master(Config, Q),
+    %% Add 20 bindings to this queue
+    [ bind(Config, QueueName, integer_to_binary(N)) || N <- lists:seq(1, 20) ],
+
+    QueueName1 = rabbit_misc:r(<<"/">>, queue, Q1 = <<"qm.test_unbound">>),
+    declare(Config, QueueName1, false, false, Args, none),
+    % Another queue should still be on the same node, bindings should
+    % not account for min-masters counting
+    verify_min_master(Config, Q1),
+    ok.
+
 calculate_random(Config) ->
     setup_test_environment(Config),
     QueueName = rabbit_misc:r(<<"/">>, queue, Q = <<"qm.test">>),
@@ -314,6 +331,17 @@ declare(Config, QueueName, Durable, AutoDelete, Args0, Owner) ->
     {new, Queue} = rabbit_ct_broker_helpers:rpc(Config, 0, rabbit_amqqueue, declare, Args1),
     Queue.
 
+bind(Config, QueueName, RoutingKey) ->
+    ExchangeName = rabbit_misc:r(QueueName, exchange, <<"amq.direct">>),
+
+    ok = rabbit_ct_broker_helpers:rpc(
+        Config, 0, rabbit_binding, add,
+        [#binding{source      = ExchangeName,
+                  destination = QueueName,
+                  key         = RoutingKey,
+                  args        = []},
+         <<"acting-user">>]).
+
 verify_min_master(Config, Q, MinMasterNode) ->
     Rpc = rabbit_ct_broker_helpers:rpc(Config, 0,
                                        rabbit_queue_master_location_misc,