Skip to content

Commit

Permalink
Fix dyn shovel child_exists check for old id format
Browse files Browse the repository at this point in the history
Because the clause `({{_, N}, _, _, _}) -> N =:= Name;` matches both
new and old id format but always returns false for old id format,
`child_exists` also returned false for old ids.

Also rename SupId to ChildId to be pedantically correct.
  • Loading branch information
gomoripeti committed Nov 23, 2023
1 parent 558535d commit 8fdd693
Showing 1 changed file with 9 additions and 7 deletions.
16 changes: 9 additions & 7 deletions deps/rabbitmq_shovel/src/rabbit_shovel_dyn_worker_sup_sup.erl
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ obfuscated_uris_parameters(Def) when is_list(Def) ->
rabbit_shovel_parameters:obfuscate_uris_in_definition(Def).

child_exists(Name) ->
lists:any(fun ({{_, N}, _, _, _}) -> N =:= Name;
%% older format, pre 3.13.0 and 3.12.8. See rabbitmq/rabbitmq-server#9894.
({N, _, _, _}) -> N =:= Name
Id = id(Name),
%% older format, pre 3.13.0 and 3.12.8. See rabbitmq/rabbitmq-server#9894.
OldId = old_id(Name),
lists:any(fun ({ChildId, _, _, _}) ->
ChildId =:= Id orelse ChildId =:= OldId
end,
mirrored_supervisor:which_children(?SUPERVISOR)).

Expand Down Expand Up @@ -100,21 +102,21 @@ stop_child({VHost, ShovelName} = Name) ->
cleanup_specs() ->
Children = mirrored_supervisor:which_children(?SUPERVISOR),

SupIdSet = sets:from_list([element(1, S) || S <- Children]),
ChildIdSet = sets:from_list([element(1, S) || S <- Children]),
ParamsSet = params_to_child_ids(rabbit_khepri:is_enabled()),
F = fun(SupId, ok) ->
F = fun(ChildId, ok) ->
try
%% The supervisor operation is very unlikely to fail, it's the schema
%% data stores that can make a fuss about a non-existent or non-standard value passed in.
%% For example, an old style Shovel name is an invalid Khepri query path element. MK.
_ = mirrored_supervisor:delete_child(?SUPERVISOR, SupId)
_ = mirrored_supervisor:delete_child(?SUPERVISOR, ChildId)
catch _:_:_Stacktrace ->
ok
end,
ok
end,
%% Delete any supervisor children that do not have their respective runtime parameters in the database.
SetToCleanUp = sets:subtract(SupIdSet, ParamsSet),
SetToCleanUp = sets:subtract(ChildIdSet, ParamsSet),
ok = sets:fold(F, ok, SetToCleanUp).

params_to_child_ids(_KhepriEnabled = true) ->
Expand Down

0 comments on commit 8fdd693

Please sign in to comment.