Skip to content

Commit 0a9afb9

Browse files
committed
Do not depend on feature flags in mc
Addresses PR feedback #11174 (comment)
1 parent ac60d3a commit 0a9afb9

File tree

7 files changed

+36
-43
lines changed

7 files changed

+36
-43
lines changed

deps/rabbit/BUILD.bazel

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -795,9 +795,6 @@ rabbitmq_suite(
795795
rabbitmq_suite(
796796
name = "mc_unit_SUITE",
797797
size = "small",
798-
runtime_deps = [
799-
"@meck//:erlang_app",
800-
],
801798
deps = [
802799
"//deps/amqp10_common:erlang_app",
803800
"//deps/rabbit_common:erlang_app",

deps/rabbit/include/mc.hrl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
-define(ANN_DURABLE, d).
1414
-define(ANN_PRIORITY, p).
1515

16+
-define(FF_MC_DEATHS_V2, message_containers_deaths_v2).
17+
1618
-type death_key() :: {SourceQueue :: rabbit_misc:resource_name(), rabbit_dead_letter:reason()}.
1719
-type death_anns() :: #{%% timestamp of the first time this message
1820
%% was dead lettered from this queue for this reason

deps/rabbit/src/mc.erl

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
convert/3,
3535
protocol_state/1,
3636
prepare/2,
37-
record_death/3,
37+
record_death/4,
3838
is_death_cycle/2,
3939
death_queue_names/1
4040
]).
@@ -356,9 +356,11 @@ protocol_state(BasicMsg) ->
356356

357357
-spec record_death(rabbit_dead_letter:reason(),
358358
rabbit_misc:resource_name(),
359-
state()) -> state().
359+
state(),
360+
environment()) -> state().
360361
record_death(Reason, SourceQueue,
361-
#?MODULE{annotations = Anns0} = State)
362+
#?MODULE{annotations = Anns0} = State,
363+
Env)
362364
when is_atom(Reason) andalso
363365
is_binary(SourceQueue) ->
364366
Key = {SourceQueue, Reason},
@@ -398,9 +400,11 @@ record_death(Reason, SourceQueue,
398400
deaths := Deaths#deaths{last = Key,
399401
records = Rs}};
400402
_ ->
401-
Rs = case rabbit_feature_flags:is_enabled(message_containers_deaths_v2) of
402-
true -> [{Key, NewDeath}];
403-
false -> #{Key => NewDeath}
403+
Rs = case Env of
404+
#{?FF_MC_DEATHS_V2 := false} ->
405+
#{Key => NewDeath};
406+
_ ->
407+
[{Key, NewDeath}]
404408
end,
405409
ReasonBin = atom_to_binary(Reason),
406410
Anns0#{<<"x-first-death-reason">> => ReasonBin,
@@ -414,8 +418,8 @@ record_death(Reason, SourceQueue,
414418
records = Rs}}
415419
end,
416420
State#?MODULE{annotations = Anns};
417-
record_death(Reason, SourceQueue, BasicMsg) ->
418-
mc_compat:record_death(Reason, SourceQueue, BasicMsg).
421+
record_death(Reason, SourceQueue, BasicMsg, Env) ->
422+
mc_compat:record_death(Reason, SourceQueue, BasicMsg, Env).
419423

420424
update_death(#death{count = Count,
421425
anns = DeathAnns} = Death, Timestamp) ->

deps/rabbit/src/mc_compat.erl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
protocol_state/1,
2727
%serialize/1,
2828
prepare/2,
29-
record_death/3,
29+
record_death/4,
3030
is_death_cycle/2,
3131
%deaths/1,
3232
death_queue_names/1
@@ -155,7 +155,7 @@ prepare(store, Msg) ->
155155
record_death(Reason, SourceQueue,
156156
#basic_message{content = Content,
157157
exchange_name = Exchange,
158-
routing_keys = RoutingKeys} = Msg) ->
158+
routing_keys = RoutingKeys} = Msg, _Env) ->
159159
% HeadersFun1 = fun (H) -> lists:keydelete(<<"CC">>, 1, H) end,
160160
ReasonBin = atom_to_binary(Reason),
161161
TimeSec = os:system_time(seconds),

deps/rabbit/src/rabbit_dead_letter.erl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@ publish(Msg0, Reason, #exchange{name = XName} = DLX, RK,
3131
_ ->
3232
[RK]
3333
end,
34-
Msg1 = mc:record_death(Reason, SourceQName, Msg0),
34+
Env = case rabbit_feature_flags:is_enabled(?FF_MC_DEATHS_V2) of
35+
true -> #{};
36+
false -> #{?FF_MC_DEATHS_V2 => false}
37+
end,
38+
Msg1 = mc:record_death(Reason, SourceQName, Msg0, Env),
3539
{Ttl, Msg2} = mc:take_annotation(dead_letter_ttl, Msg1),
3640
Msg3 = mc:set_ttl(Ttl, Msg2),
3741
Msg4 = mc:set_annotation(?ANN_ROUTING_KEYS, DLRKeys, Msg3),

deps/rabbit/src/rabbit_fifo_dlx_worker.erl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,11 @@ forward(ConsumedMsg, ConsumedMsgId, ConsumedQRef, DLX, Reason,
324324
_ ->
325325
[RKey]
326326
end,
327-
Msg0 = mc:record_death(Reason, SourceQName, ConsumedMsg),
327+
Env = case rabbit_feature_flags:is_enabled(?FF_MC_DEATHS_V2) of
328+
true -> #{};
329+
false -> #{?FF_MC_DEATHS_V2 => false}
330+
end,
331+
Msg0 = mc:record_death(Reason, SourceQName, ConsumedMsg, Env),
328332
Msg1 = mc:set_ttl(undefined, Msg0),
329333
Msg2 = mc:set_annotation(?ANN_ROUTING_KEYS, DLRKeys, Msg1),
330334
Msg = mc:set_annotation(?ANN_EXCHANGE, DLXName, Msg2),

deps/rabbit/test/mc_unit_SUITE.erl

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,6 @@ all_tests() ->
4545
amqp_amqpl_amqp_bodies
4646
].
4747

48-
init_per_suite(Config) ->
49-
meck:new(rabbit_feature_flags, [passthrough, no_link]),
50-
Config.
51-
52-
end_per_suite(Config) ->
53-
meck:unload(rabbit_feature_flags),
54-
Config.
55-
5648
%%%===================================================================
5749
%%% Test cases
5850
%%%===================================================================
@@ -202,24 +194,18 @@ amqpl_table_x_header_array_of_tbls(_Config) ->
202194
ok.
203195

204196
amqpl_death_v1_records(_Config) ->
205-
meck:expect(rabbit_feature_flags,
206-
is_enabled,
207-
fun(message_containers_deaths_v2) -> false end),
208-
ok = amqpl_death_records().
197+
ok = amqpl_death_records(#{?FF_MC_DEATHS_V2 => false}).
209198

210199
amqpl_death_v2_records(_Config) ->
211-
meck:expect(rabbit_feature_flags,
212-
is_enabled,
213-
fun(message_containers_deaths_v2) -> true end),
214-
ok = amqpl_death_records().
200+
ok = amqpl_death_records(#{?FF_MC_DEATHS_V2 => true}).
215201

216-
amqpl_death_records() ->
202+
amqpl_death_records(Env) ->
217203
Content = #content{class_id = 60,
218204
properties = #'P_basic'{headers = []},
219205
payload_fragments_rev = [<<"data">>]},
220206
Msg0 = mc:prepare(store, mc:init(mc_amqpl, Content, annotations())),
221207

222-
Msg1 = mc:record_death(rejected, <<"q1">>, Msg0),
208+
Msg1 = mc:record_death(rejected, <<"q1">>, Msg0, Env),
223209
?assertEqual([<<"q1">>], mc:death_queue_names(Msg1)),
224210
?assertEqual(false, mc:is_death_cycle(<<"q1">>, Msg1)),
225211

@@ -245,7 +231,7 @@ amqpl_death_records() ->
245231
%% record_death uses a timestamp for death record ordering, ensure
246232
%% it is definitely higher than the last timestamp taken
247233
timer:sleep(2),
248-
Msg2 = mc:record_death(expired, <<"dl">>, Msg1),
234+
Msg2 = mc:record_death(expired, <<"dl">>, Msg1, Env),
249235

250236
#content{properties = #'P_basic'{headers = H2}} = mc:protocol_state(Msg2),
251237
{_, array, [{table, T2a}, {table, T2b}]} = header(<<"x-death">>, H2),
@@ -254,10 +240,6 @@ amqpl_death_records() ->
254240
ok.
255241

256242
is_death_cycle(_Config) ->
257-
meck:expect(rabbit_feature_flags,
258-
is_enabled,
259-
fun(message_containers_deaths_v2) -> true end),
260-
261243
Content = #content{class_id = 60,
262244
properties = #'P_basic'{headers = []},
263245
payload_fragments_rev = [<<"data">>]},
@@ -267,29 +249,29 @@ is_death_cycle(_Config) ->
267249
%% Q1 --rejected--> Q2 --expired--> Q3 --expired-->
268250
%% Q1 --rejected--> Q2 --expired--> Q3
269251

270-
Msg1 = mc:record_death(rejected, <<"q1">>, Msg0),
252+
Msg1 = mc:record_death(rejected, <<"q1">>, Msg0, #{}),
271253
?assertNot(mc:is_death_cycle(<<"q1">>, Msg1),
272254
"A queue that dead letters to itself due to rejected is not considered a cycle."),
273255
?assertNot(mc:is_death_cycle(<<"q2">>, Msg1)),
274256
?assertNot(mc:is_death_cycle(<<"q3">>, Msg1)),
275257

276-
Msg2 = mc:record_death(expired, <<"q2">>, Msg1),
258+
Msg2 = mc:record_death(expired, <<"q2">>, Msg1, #{}),
277259
?assertNot(mc:is_death_cycle(<<"q1">>, Msg2)),
278260
?assert(mc:is_death_cycle(<<"q2">>, Msg2),
279261
"A queue that dead letters to itself due to expired is considered a cycle."),
280262
?assertNot(mc:is_death_cycle(<<"q3">>, Msg2)),
281263

282-
Msg3 = mc:record_death(expired, <<"q3">>, Msg2),
264+
Msg3 = mc:record_death(expired, <<"q3">>, Msg2, #{}),
283265
?assertNot(mc:is_death_cycle(<<"q1">>, Msg3)),
284266
?assert(mc:is_death_cycle(<<"q2">>, Msg3)),
285267
?assert(mc:is_death_cycle(<<"q3">>, Msg3)),
286268

287-
Msg4 = mc:record_death(rejected, <<"q1">>, Msg3),
269+
Msg4 = mc:record_death(rejected, <<"q1">>, Msg3, #{}),
288270
?assertNot(mc:is_death_cycle(<<"q1">>, Msg4)),
289271
?assertNot(mc:is_death_cycle(<<"q2">>, Msg4)),
290272
?assertNot(mc:is_death_cycle(<<"q3">>, Msg4)),
291273

292-
Msg5 = mc:record_death(expired, <<"q2">>, Msg4),
274+
Msg5 = mc:record_death(expired, <<"q2">>, Msg4, #{}),
293275
?assertNot(mc:is_death_cycle(<<"q1">>, Msg5)),
294276
?assert(mc:is_death_cycle(<<"q2">>, Msg5)),
295277
?assertNot(mc:is_death_cycle(<<"q3">>, Msg5)),

0 commit comments

Comments
 (0)