Skip to content

Commit 11d0e0d

Browse files
committed
Deprecate queue-master-locator
* this should not be a breaking change - all validation should still pass * CQs can now use `queue-leader-locator` * `queue-leader-locator` takes precedence over `queue-master-locator` if both are used * regardless of which name is used, effectively there are only two values: `client-local` (default) or `balanced` * other values (`min-masters`, `random`, `least-leaders`) are mapped to `balanced` * exclusive queues are always declared locally, as before
1 parent 34d3f94 commit 11d0e0d

18 files changed

+274
-805
lines changed

deps/rabbit/BUILD.bazel

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -644,12 +644,6 @@ rabbitmq_integration_suite(
644644
size = "medium",
645645
)
646646

647-
rabbitmq_integration_suite(
648-
name = "queue_master_location_SUITE",
649-
size = "large",
650-
shard_count = 2,
651-
)
652-
653647
rabbitmq_integration_suite(
654648
name = "queue_parallel_SUITE",
655649
size = "large",

deps/rabbit/Makefile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,6 @@ SLOW_CT_SUITES := backing_queue \
206206
priority_queue \
207207
priority_queue_recovery \
208208
publisher_confirms_parallel \
209-
queue_master_location \
210209
queue_parallel \
211210
quorum_queue \
212211
rabbit_core_metrics_gc \

deps/rabbit/app.bzl

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ def all_beam_files(name = "all_beam_files"):
1414
"src/rabbit_credential_validator.erl",
1515
"src/rabbit_exchange_type.erl",
1616
"src/rabbit_policy_merge_strategy.erl",
17-
"src/rabbit_queue_master_locator.erl",
1817
"src/rabbit_queue_type.erl",
1918
"src/rabbit_tracking.erl",
2019
],
@@ -195,11 +194,6 @@ def all_beam_files(name = "all_beam_files"):
195194
"src/rabbit_queue_decorator.erl",
196195
"src/rabbit_queue_index.erl",
197196
"src/rabbit_queue_location.erl",
198-
"src/rabbit_queue_location_client_local.erl",
199-
"src/rabbit_queue_location_min_masters.erl",
200-
"src/rabbit_queue_location_random.erl",
201-
"src/rabbit_queue_location_validator.erl",
202-
"src/rabbit_queue_master_location_misc.erl",
203197
"src/rabbit_queue_type_util.erl",
204198
"src/rabbit_quorum_memory_manager.erl",
205199
"src/rabbit_quorum_queue.erl",
@@ -272,7 +266,6 @@ def all_test_beam_files(name = "all_test_beam_files"):
272266
"src/rabbit_credential_validator.erl",
273267
"src/rabbit_exchange_type.erl",
274268
"src/rabbit_policy_merge_strategy.erl",
275-
"src/rabbit_queue_master_locator.erl",
276269
"src/rabbit_queue_type.erl",
277270
"src/rabbit_tracking.erl",
278271
],
@@ -454,11 +447,6 @@ def all_test_beam_files(name = "all_test_beam_files"):
454447
"src/rabbit_queue_decorator.erl",
455448
"src/rabbit_queue_index.erl",
456449
"src/rabbit_queue_location.erl",
457-
"src/rabbit_queue_location_client_local.erl",
458-
"src/rabbit_queue_location_min_masters.erl",
459-
"src/rabbit_queue_location_random.erl",
460-
"src/rabbit_queue_location_validator.erl",
461-
"src/rabbit_queue_master_location_misc.erl",
462450
"src/rabbit_queue_type_util.erl",
463451
"src/rabbit_quorum_memory_manager.erl",
464452
"src/rabbit_quorum_queue.erl",
@@ -733,12 +721,6 @@ def all_srcs(name = "all_srcs"):
733721
"src/rabbit_queue_decorator.erl",
734722
"src/rabbit_queue_index.erl",
735723
"src/rabbit_queue_location.erl",
736-
"src/rabbit_queue_location_client_local.erl",
737-
"src/rabbit_queue_location_min_masters.erl",
738-
"src/rabbit_queue_location_random.erl",
739-
"src/rabbit_queue_location_validator.erl",
740-
"src/rabbit_queue_master_location_misc.erl",
741-
"src/rabbit_queue_master_locator.erl",
742724
"src/rabbit_queue_type.erl",
743725
"src/rabbit_queue_type_util.erl",
744726
"src/rabbit_quorum_memory_manager.erl",
@@ -1239,15 +1221,6 @@ def test_suite_beam_files(name = "test_suite_beam_files"):
12391221
erlc_opts = "//:test_erlc_opts",
12401222
deps = ["//deps/amqp_client:erlang_app"],
12411223
)
1242-
erlang_bytecode(
1243-
name = "queue_master_location_SUITE_beam_files",
1244-
testonly = True,
1245-
srcs = ["test/queue_master_location_SUITE.erl"],
1246-
outs = ["test/queue_master_location_SUITE.beam"],
1247-
app_name = "rabbit",
1248-
erlc_opts = "//:test_erlc_opts",
1249-
deps = ["//deps/amqp_client:erlang_app", "//deps/rabbitmq_ct_helpers:erlang_app"],
1250-
)
12511224
erlang_bytecode(
12521225
name = "queue_parallel_SUITE_beam_files",
12531226
testonly = True,

deps/rabbit/priv/schema/rabbit.schema

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,10 @@ end}.
14641464

14651465
%% Queue master locator (classic queues)
14661466
%%
1467+
%% For backwards compatibility only as of 4.0.
1468+
%% We still allow values of min-masters, random and client-local
1469+
%% but the behaviour is only local or balanced.
1470+
%% Use queue_leader_locator instead.
14671471

14681472
{mapping, "queue_master_locator", "rabbit.queue_master_locator",
14691473
[{datatype, string}]}.

deps/rabbit/src/rabbit_classic_queue.erl

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
-module(rabbit_classic_queue).
22
-behaviour(rabbit_queue_type).
3+
-behaviour(rabbit_policy_validator).
34

45
-include("amqqueue.hrl").
56
-include_lib("rabbit_common/include/rabbit.hrl").
@@ -63,6 +64,26 @@
6364
send_drained_credit_api_v1/4,
6465
send_credit_reply/7]).
6566

67+
-export([validate_policy/1]).
68+
69+
-rabbit_boot_step(
70+
{?MODULE,
71+
[{description, "Deprecated queue-master-locator support."
72+
"Use queue-leader-locator instead."},
73+
{mfa, {rabbit_registry, register,
74+
[policy_validator, <<"queue-master-locator">>, ?MODULE]}},
75+
{mfa, {rabbit_registry, register,
76+
[operator_policy_validator, <<"queue-master-locator">>, ?MODULE]}},
77+
{requires, rabbit_registry},
78+
{enables, recovery}]}).
79+
80+
validate_policy(Args) ->
81+
Strategy = proplists:get_value(<<"queue-master-locator">>, Args, unknown),
82+
case lists:member(Strategy, rabbit_queue_location:queue_leader_locators()) of
83+
true -> ok;
84+
false -> {error, "~tp is not a valid master locator", [Strategy]}
85+
end.
86+
6687
-spec is_enabled() -> boolean().
6788
is_enabled() -> true.
6889

@@ -79,10 +100,8 @@ declare(Q, Node) when ?amqqueue_is_classic(Q) ->
79100
{_, true} ->
80101
Node;
81102
_ ->
82-
case rabbit_queue_master_location_misc:get_location(Q) of
83-
{ok, Node0} -> Node0;
84-
_ -> Node
85-
end
103+
{Node0, _} = rabbit_queue_location:select_leader_and_followers(Q, 1, rabbit_classic_queue),
104+
Node0
86105
end,
87106
case rabbit_vhost_sup_sup:get_vhost_sup(VHost, Node1) of
88107
{ok, _} ->
@@ -504,15 +523,15 @@ recover_durable_queues(QueuesAndRecoveryTerms) ->
504523
capabilities() ->
505524
#{unsupported_policies => [%% Stream policies
506525
<<"max-age">>, <<"stream-max-segment-size-bytes">>,
507-
<<"queue-leader-locator">>, <<"initial-cluster-size">>,
526+
<<"initial-cluster-size">>,
508527
%% Quorum policies
509528
<<"delivery-limit">>, <<"dead-letter-strategy">>, <<"max-in-memory-length">>, <<"max-in-memory-bytes">>, <<"target-group-size">>],
510529
queue_arguments => [<<"x-expires">>, <<"x-message-ttl">>, <<"x-dead-letter-exchange">>,
511530
<<"x-dead-letter-routing-key">>, <<"x-max-length">>,
512531
<<"x-max-length-bytes">>, <<"x-max-priority">>,
513532
<<"x-overflow">>, <<"x-queue-mode">>, <<"x-queue-version">>,
514533
<<"x-single-active-consumer">>, <<"x-queue-type">>,
515-
<<"x-queue-master-locator">>],
534+
<<"x-queue-master-locator">>, <<"x-queue-leader-locator">>],
516535
consumer_arguments => [<<"x-cancel-on-ha-failover">>,
517536
<<"x-priority">>, <<"x-credit">>],
518537
server_named => true}.

deps/rabbit/src/rabbit_queue_location.erl

Lines changed: 95 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,18 @@
1010
-include("amqqueue.hrl").
1111

1212
-export([queue_leader_locators/0,
13-
select_leader_and_followers/2]).
13+
select_leader_and_followers/3]).
1414

15-
-define(QUEUE_LEADER_LOCATORS_DEPRECATED, [<<"random">>, <<"least-leaders">>]).
15+
%% these are needed because of they are called with ?MODULE:
16+
%% to allow mecking them in tests
17+
-export([node/0,
18+
queues_per_node/2]).
19+
20+
-ifdef(TEST).
21+
-export([select_members/7, leader_node/6, leader_locator/1]).
22+
-endif.
23+
24+
-define(QUEUE_LEADER_LOCATORS_DEPRECATED, [<<"random">>, <<"least-leaders">>, <<"min-masters">>]).
1625
-define(QUEUE_LEADER_LOCATORS, [<<"client-local">>, <<"balanced">>] ++ ?QUEUE_LEADER_LOCATORS_DEPRECATED).
1726
-define(QUEUE_COUNT_START_RANDOM_SELECTION, 1_000).
1827

@@ -23,26 +32,33 @@
2332
queue_leader_locators() ->
2433
?QUEUE_LEADER_LOCATORS.
2534

26-
-spec select_leader_and_followers(amqqueue:amqqueue(), pos_integer()) ->
35+
-spec select_leader_and_followers(amqqueue:amqqueue(), pos_integer(), atom()) ->
2736
{Leader :: node(), Followers :: [node()]}.
28-
select_leader_and_followers(Q, Size)
29-
when (?amqqueue_is_quorum(Q) orelse ?amqqueue_is_stream(Q)) andalso is_integer(Size) ->
37+
select_leader_and_followers(Q, Size, QueueType)
38+
when (?amqqueue_is_quorum(Q) orelse ?amqqueue_is_stream(Q) orelse ?amqqueue_is_classic(Q)) andalso is_integer(Size) ->
39+
LeaderLocator = leader_locator(Q),
40+
do_select_leader_and_followers(Size, QueueType, LeaderLocator).
41+
42+
-spec do_select_leader_and_followers(pos_integer(), atom(), queue_leader_locator()) ->
43+
{Leader :: node(), Followers :: [node()]}.
44+
do_select_leader_and_followers(1, _, <<"client-local">>) ->
45+
%% optimisation for classic queues
46+
{?MODULE:node(), []};
47+
do_select_leader_and_followers(Size, QueueType, LeaderLocator) ->
3048
AllNodes = rabbit_nodes:list_members(),
3149
RunningNodes = rabbit_nodes:filter_running(AllNodes),
32-
true = lists:member(node(), AllNodes),
33-
QueueType = amqqueue:get_type(Q),
50+
true = lists:member(?MODULE:node(), AllNodes),
3451
GetQueues0 = get_queues_for_type(QueueType),
3552
%% TODO do we always need the queue count? it can be expensive, check if it can be skipped!
3653
%% for example, for random
3754
QueueCount = rabbit_amqqueue:count(),
3855
QueueCountStartRandom = application:get_env(rabbit, queue_count_start_random_selection,
3956
?QUEUE_COUNT_START_RANDOM_SELECTION),
40-
{Replicas, GetQueues} = select_replicas(Size, AllNodes, RunningNodes,
57+
{Members, GetQueues} = select_members(Size, QueueType, AllNodes, RunningNodes,
4158
QueueCount, QueueCountStartRandom, GetQueues0),
42-
LeaderLocator = leader_locator(Q),
43-
Leader = leader_node(LeaderLocator, Replicas, RunningNodes,
59+
Leader = leader_node(LeaderLocator, Members, RunningNodes,
4460
QueueCount, QueueCountStartRandom, GetQueues),
45-
Followers = lists:delete(Leader, Replicas),
61+
Followers = lists:delete(Leader, Members),
4662
{Leader, Followers}.
4763

4864
-spec leader_locator(amqqueue:amqqueue()) ->
@@ -53,7 +69,15 @@ leader_locator(Q) ->
5369
fun (PolVal, _ArgVal) -> PolVal end,
5470
Q) of
5571
undefined ->
56-
application:get_env(rabbit, queue_leader_locator, undefined);
72+
case rabbit_queue_type_util:args_policy_lookup(
73+
<<"queue-master-locator">>,
74+
fun (PolVal, _ArgVal) -> PolVal end,
75+
Q) of
76+
undefined ->
77+
application:get_env(rabbit, queue_leader_locator, undefined);
78+
Val ->
79+
Val
80+
end;
5781
Val ->
5882
Val
5983
end,
@@ -63,40 +87,57 @@ leader_locator0(<<"client-local">>) ->
6387
<<"client-local">>;
6488
leader_locator0(<<"balanced">>) ->
6589
<<"balanced">>;
66-
%% 'random' and 'least-leaders' are deprecated
90+
%% 'random', 'least-leaders' and 'min-masters' are deprecated
6791
leader_locator0(<<"random">>) ->
6892
<<"balanced">>;
6993
leader_locator0(<<"least-leaders">>) ->
7094
<<"balanced">>;
95+
leader_locator0(<<"min-masters">>) ->
96+
<<"balanced">>;
7197
leader_locator0(_) ->
7298
%% default
7399
<<"client-local">>.
74100

75-
-spec select_replicas(pos_integer(), [node(),...], [node(),...],
101+
-spec select_members(pos_integer(), term(), [node(),...], [node(),...],
76102
non_neg_integer(), non_neg_integer(), function()) ->
77103
{[node(),...], function()}.
78-
select_replicas(Size, AllNodes, _, _, _, Fun)
104+
select_members(Size, _, AllNodes, _, _, _, Fun)
79105
when length(AllNodes) =< Size ->
80106
{AllNodes, Fun};
107+
%% Classic queues: above the threshold, pick a random node
108+
%% For classic queues, when there's a lot of queues, if we knew that the
109+
%% distribution of queues between nodes is relatively even, it'd be better
110+
%% to declare this queue locally rather than randomly. However, currently,
111+
%% counting queues on each node is relatively expensive. Users can use
112+
%% the client-local strategy if they know their connections are well balanced
113+
select_members(1, rabbit_classic_queue, _AllNodes, RunningNodes, QueueCount, QueueCountStartRandom, GetQueues)
114+
when QueueCount >= QueueCountStartRandom ->
115+
{RunningNodes, GetQueues};
116+
select_members(1, rabbit_classic_queue, _, RunningNodes, _, _, GetQueues) ->
117+
{RunningNodes, GetQueues};
118+
%% Quorum queues and streams
81119
%% Select nodes in the following order:
82120
%% 1. Local node to have data locality for declaring client.
83121
%% 2. Running nodes.
84-
%% 3.1. If there are many queues: Randomly to avoid expensive calculation of counting replicas
122+
%% 3.1. If there are many queues: Randomly to avoid expensive calculation of counting members
85123
%% per node. Random replica selection is good enough for most use cases.
86-
%% 3.2. If there are few queues: Nodes with least replicas to have a "balanced" RabbitMQ cluster.
87-
select_replicas(Size, AllNodes, RunningNodes, QueueCount, QueueCountStartRandom, GetQueues)
124+
%% 3.2. If there are few queues: Nodes with least members to have a "balanced" RabbitMQ cluster.
125+
select_members(Size, _, AllNodes, RunningNodes, QueueCount, QueueCountStartRandom, GetQueues)
88126
when QueueCount >= QueueCountStartRandom ->
89-
L0 = shuffle(lists:delete(node(), AllNodes)),
127+
L0 = shuffle(lists:delete(?MODULE:node(), AllNodes)),
90128
L1 = lists:sort(fun(X, _Y) ->
91129
lists:member(X, RunningNodes)
92130
end, L0),
93131
{L, _} = lists:split(Size - 1, L1),
94-
{[node() | L], GetQueues};
95-
select_replicas(Size, AllNodes, RunningNodes, _, _, GetQueues) ->
96-
Counters0 = maps:from_list([{N, 0} || N <- lists:delete(node(), AllNodes)]),
132+
{[?MODULE:node() | L], GetQueues};
133+
select_members(Size, _, AllNodes, RunningNodes, _, _, GetQueues) ->
134+
Counters0 = maps:from_list([{N, 0} || N <- lists:delete(?MODULE:node(), AllNodes)]),
97135
Queues = GetQueues(),
98136
Counters = lists:foldl(fun(Q, Acc) ->
99-
#{nodes := Nodes} = amqqueue:get_type_state(Q),
137+
Nodes = case amqqueue:get_type(Q) of
138+
rabbit_classic_queue -> [amqqueue:qnode(Q)];
139+
_ -> maps:get(nodes, amqqueue:get_type_state(Q))
140+
end,
100141
lists:foldl(fun(N, A)
101142
when is_map_key(N, A) ->
102143
maps:update_with(N, fun(C) -> C+1 end, A);
@@ -118,46 +159,35 @@ select_replicas(Size, AllNodes, RunningNodes, _, _, GetQueues) ->
118159
end, L0),
119160
{L2, _} = lists:split(Size - 1, L1),
120161
L = lists:map(fun({N, _}) -> N end, L2),
121-
{[node() | L], fun() -> Queues end}.
162+
{[?MODULE:node() | L], fun() -> Queues end}.
122163

123164
-spec leader_node(queue_leader_locator(), [node(),...], [node(),...],
124165
non_neg_integer(), non_neg_integer(), function()) ->
125166
node().
126167
leader_node(<<"client-local">>, _, _, _, _, _) ->
127-
node();
168+
?MODULE:node();
128169
leader_node(<<"balanced">>, Nodes0, RunningNodes, QueueCount, QueueCountStartRandom, _)
129170
when QueueCount >= QueueCountStartRandom ->
130171
Nodes = potential_leaders(Nodes0, RunningNodes),
131172
lists:nth(rand:uniform(length(Nodes)), Nodes);
132-
leader_node(<<"balanced">>, Nodes0, RunningNodes, _, _, GetQueues)
173+
leader_node(<<"balanced">>, Members0, RunningNodes, _, _, GetQueues)
133174
when is_function(GetQueues, 0) ->
134-
Nodes = potential_leaders(Nodes0, RunningNodes),
135-
Counters0 = maps:from_list([{N, 0} || N <- Nodes]),
136-
Counters = lists:foldl(fun(Q, Acc) ->
137-
case amqqueue:get_pid(Q) of
138-
{RaName, LeaderNode}
139-
when is_atom(RaName), is_atom(LeaderNode), is_map_key(LeaderNode, Acc) ->
140-
maps:update_with(LeaderNode, fun(C) -> C+1 end, Acc);
141-
StreamLeaderPid
142-
when is_pid(StreamLeaderPid), is_map_key(node(StreamLeaderPid), Acc) ->
143-
maps:update_with(node(StreamLeaderPid), fun(C) -> C+1 end, Acc);
144-
_ ->
145-
Acc
146-
end
147-
end, Counters0, GetQueues()),
175+
Members = potential_leaders(Members0, RunningNodes),
176+
Counters = ?MODULE:queues_per_node(Members, GetQueues),
177+
ct:pal("Counters ~p", [Counters]),
148178
{Node, _} = hd(lists:keysort(2, maps:to_list(Counters))),
149179
Node.
150180

151-
potential_leaders(Replicas, RunningNodes) ->
181+
potential_leaders(Members, RunningNodes) ->
152182
case lists:filter(fun(R) ->
153183
lists:member(R, RunningNodes)
154-
end, Replicas) of
184+
end, Members) of
155185
[] ->
156-
Replicas;
157-
RunningReplicas ->
158-
case rabbit_maintenance:filter_out_drained_nodes_local_read(RunningReplicas) of
186+
Members;
187+
RunningMembers ->
188+
case rabbit_maintenance:filter_out_drained_nodes_local_read(RunningMembers) of
159189
[] ->
160-
RunningReplicas;
190+
RunningMembers;
161191
Filtered ->
162192
Filtered
163193
end
@@ -172,3 +202,22 @@ shuffle(L0) when is_list(L0) ->
172202
L1 = lists:map(fun(E) -> {rand:uniform(), E} end, L0),
173203
L = lists:keysort(1, L1),
174204
lists:map(fun({_, E}) -> E end, L).
205+
206+
queues_per_node(Nodes, GetQueues) ->
207+
Counters0 = maps:from_list([{N, 0} || N <- Nodes]),
208+
lists:foldl(fun(Q, Acc) ->
209+
case amqqueue:get_pid(Q) of
210+
{RaName, LeaderNode} %% quorum queues
211+
when is_atom(RaName), is_atom(LeaderNode), is_map_key(LeaderNode, Acc) ->
212+
maps:update_with(LeaderNode, fun(C) -> C+1 end, Acc);
213+
Pid %% classic queues and streams
214+
when is_pid(Pid), is_map_key(node(Pid), Acc) ->
215+
maps:update_with(node(Pid), fun(C) -> C+1 end, Acc);
216+
_ ->
217+
Acc
218+
end
219+
end, Counters0, GetQueues()).
220+
221+
%% for unit testing
222+
-spec node() -> node().
223+
node() -> erlang:node().

0 commit comments

Comments
 (0)