See #7389. Only check tick pid if ts is expired

Author: SimonUnge

deps/rabbit/src/rabbit_fifo.erl

@@ -894,9 +894,9 @@ tick(Ts, #?MODULE{cfg = #cfg{name = _Name,
                              resource = QName}} = State) ->
     case is_expired(Ts, State) of
         true ->
-            [{aux, {handle_tick, spawn_deleter, [QName]}}];
+            [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}];
         false ->
-            [{aux, {handle_tick, handle_tick, [QName, overview(State), all_nodes(State)]}}]
+            [{aux, {handle_tick, [QName, overview(State), all_nodes(State)]}}]
     end.
 
 -spec overview(state()) -> map().
@@ -1029,14 +1029,14 @@ handle_aux(leader, cast, {#return{msg_ids = MsgIds,
         _ ->
             {no_reply, Aux0, Log0}
     end;
-handle_aux(leader, _, {handle_tick, Fun, Args},
+handle_aux(leader, _, {handle_tick, Args},
            #?AUX{tick_pid = Pid} = Aux, Log, _) ->
     NewPid =
-        case Pid =:= undefined orelse is_process_alive(Pid) =:= false of
-            true ->
-                %% No active TICK pid
-                spawn(rabbit_quorum_queue, Fun, Args);
+        case process_is_alive(Pid) of
             false ->
+                %% No active TICK pid
+                spawn(rabbit_quorum_queue, handle_tick, Args);
+            true ->
                 %% Active TICK pid, do nothing
                 Pid
         end,
@@ -1163,6 +1163,10 @@ force_eval_gc(Log, #?MODULE{cfg = #cfg{resource = QR}},
             AuxState
     end.
 
+process_is_alive(Pid) when is_pid(Pid) ->
+    is_process_alive(Pid);
+process_is_alive(_) ->
+    false.
 %%% Queries
 
 query_messages_ready(State) ->

deps/rabbit/test/rabbit_fifo_SUITE.erl

@@ -608,7 +608,7 @@ tick_test(C) ->
     {S3, {_, _}} = deq(C, 4, Cid2, unsettled, Msg2, S2),
     {S4, _, _} = apply(meta(C, 5), rabbit_fifo:make_return(Cid, [MsgId]), S3),
 
-    [{aux, {handle_tick, handle_tick,
+    [{aux, {handle_tick,
             [#resource{},
              #{config := #{name := ?FUNCTION_NAME},
                num_consumers := 1,
@@ -1562,7 +1562,7 @@ purge_nodes_test(C) ->
                            {down, EnqPid, noconnection},
                            State3),
     ?assertMatch(
-       [{aux, {handle_tick, handle_tick,
+       [{aux, {handle_tick,
          [#resource{}, _Metrics,
           [ThisNode, Node]
          ]}}] , rabbit_fifo:tick(1, State4)),
@@ -1578,7 +1578,7 @@ purge_nodes_test(C) ->
     ?assertMatch(#rabbit_fifo{consumers = Cons} when map_size(Cons) == 0,
                                                      State),
     ?assertMatch(
-       [{aux, {handle_tick, handle_tick,
+       [{aux, {handle_tick,
          [#resource{}, _Metrics,
           [ThisNode]
          ]}}] , rabbit_fifo:tick(1, State)),
@@ -1765,33 +1765,33 @@ queue_ttl_test(C) ->
              expires => 1000},
     S0 = rabbit_fifo:init(Conf),
     Now = 1500,
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S0),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S0),
     %% this should delete the queue
-    [{aux, {handle_tick, spawn_deleter, [QName]}}]
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
         = rabbit_fifo:tick(Now + 1000, S0),
     %% adding a consumer should not ever trigger deletion
     Cid = {<<"cid1">>, self()},
     {S1, _} = check_auto(C, Cid, 1, S0),
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S1),
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S1),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S1),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S1),
     %% cancelling the consumer should then
     {S2, _, _} = apply(meta(C, 2, Now),
                        rabbit_fifo:make_checkout(Cid, cancel, #{}), S1),
     %% last_active should have been reset when consumer was cancelled
     %% last_active = 2500
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2),
     %% but now it should be deleted
-    [{aux, {handle_tick, spawn_deleter, [QName]}}]
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
         = rabbit_fifo:tick(Now + 2500, S2),
 
     %% Same for downs
     {S2D, _, _} = apply(meta(C, 2, Now),
                         {down, self(), noconnection}, S1),
     %% last_active should have been reset when consumer was cancelled
     %% last_active = 2500
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2D),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2D),
     %% but now it should be deleted
-    [{aux, {handle_tick, spawn_deleter, [QName]}}]
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
         = rabbit_fifo:tick(Now + 2500, S2D),
 
     %% dequeue should set last applied
@@ -1800,9 +1800,9 @@ queue_ttl_test(C) ->
               rabbit_fifo:make_checkout(Cid, {dequeue, unsettled}, #{}),
               S0),
 
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S1Deq),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S1Deq),
     %% but now it should be deleted
-    [{aux, {handle_tick, spawn_deleter, [QName]}}]
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
         = rabbit_fifo:tick(Now + 2500, S1Deq),
     %% Enqueue message,
     Msg = rabbit_fifo:make_enqueue(self(), 1, msg1),
@@ -1818,9 +1818,9 @@ queue_ttl_test(C) ->
       {wrap_reply, {dequeue, {MsgId, _}, _}}}] = Fun2([Msg]),
     {E3, _, _} = apply(meta(C, 3, Now + 1000),
                        rabbit_fifo:make_settle(Deq, [MsgId]), E2),
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1500, E3),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1500, E3),
     %% but now it should be deleted
-    [{aux, {handle_tick, spawn_deleter, [QName]}}]
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
         = rabbit_fifo:tick(Now + 3000, E3),
     ok.
 
@@ -1833,30 +1833,33 @@ queue_ttl_with_single_active_consumer_test(C) ->
              single_active_consumer_on => true},
     S0 = rabbit_fifo:init(Conf),
     Now = 1500,
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S0),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S0),
     %% this should delete the queue
-    [{aux, {handle_tick, spawn_deleter, [QName]}}] = rabbit_fifo:tick(Now + 1000, S0),
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
+        = rabbit_fifo:tick(Now + 1000, S0),
     %% adding a consumer should not ever trigger deletion
     Cid = {<<"cid1">>, self()},
     {S1, _} = check_auto(C, Cid, 1, S0),
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S1),
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S1),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now, S1),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S1),
     %% cancelling the consumer should then
     {S2, _, _} = apply(meta(C, 2, Now),
                        rabbit_fifo:make_checkout(Cid, cancel, #{}), S1),
     %% last_active should have been reset when consumer was cancelled
     %% last_active = 2500
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2),
     %% but now it should be deleted
-    [{aux, {handle_tick, spawn_deleter, [QName]}}] = rabbit_fifo:tick(Now + 2500, S2),
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
+        = rabbit_fifo:tick(Now + 2500, S2),
     %% Same for downs
     {S2D, _, _} = apply(meta(C, 2, Now),
                         {down, self(), noconnection}, S1),
     %% last_active should have been reset when consumer was cancelled
     %% last_active = 2500
-    [{aux, {handle_tick, handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2D),
+    [{aux, {handle_tick, [_, _, _]}}] = rabbit_fifo:tick(Now + 1000, S2D),
     %% but now it should be deleted
-    [{aux, {handle_tick, spawn_deleter, [QName]}}] = rabbit_fifo:tick(Now + 2500, S2D),
+    [{mod_call, rabbit_quorum_queue, spawn_deleter, [QName]}]
+        = rabbit_fifo:tick(Now + 2500, S2D),
     ok.
 
 query_peek_test(C) ->