Merge pull request #3409 from rabbitmq/lh-increase-queue-shutdown-timeout

Increase classic queue shutdown timeout

Author: michaelklishin

deps/rabbit/src/rabbit_amqqueue_sup.erl

@@ -24,8 +24,8 @@ start_link(Q, StartMode) ->
     Marker = spawn_link(fun() -> receive stop -> ok end end),
     ChildSpec = {rabbit_amqqueue,
                  {rabbit_prequeue, start_link, [Q, StartMode, Marker]},
-                 intrinsic, ?WORKER_WAIT, worker, [rabbit_amqqueue_process,
-                                                rabbit_mirror_queue_slave]},
+                 intrinsic, ?CLASSIC_QUEUE_WORKER_WAIT, worker,
+                 [rabbit_amqqueue_process, rabbit_mirror_queue_slave]},
     {ok, SupPid} = supervisor2:start_link(?MODULE, []),
     {ok, QPid} = supervisor2:start_child(SupPid, ChildSpec),
     unlink(Marker),

deps/rabbit_common/include/rabbit.hrl

@@ -225,9 +225,11 @@
 -define(SUPERVISOR_WAIT,
         rabbit_misc:get_env(rabbit, supervisor_shutdown_timeout, infinity)).
 -define(WORKER_WAIT,
-        rabbit_misc:get_env(rabbit, worker_shutdown_timeout, 30000)).
+        rabbit_misc:get_env(rabbit, worker_shutdown_timeout, 300000)).
 -define(MSG_STORE_WORKER_WAIT,
         rabbit_misc:get_env(rabbit, msg_store_shutdown_timeout, 600000)).
+-define(CLASSIC_QUEUE_WORKER_WAIT,
+        rabbit_misc:get_env(rabbit, classic_queue_shutdown_timeout, 600000)).
 
 -define(HIBERNATE_AFTER_MIN,        1000).
 -define(DESIRED_HIBERNATE,         10000).

deps/rabbitmq_federation/test/queue_SUITE.erl

@@ -15,7 +15,8 @@
 
 -import(rabbit_federation_test_util,
         [wait_for_federation/2, expect/3, expect/4,
-         set_upstream/4, set_upstream/5, clear_upstream/3, set_policy/5, clear_policy/3,
+         set_upstream/4, set_upstream/5, clear_upstream/3, clear_upstream_set/3,
+         set_policy/5, clear_policy/3,
          set_policy_pattern/5, set_policy_upstream/5, q/2, with_ch/3,
          maybe_declare_queue/3, delete_queue/2,
          federation_links_in_vhost/3]).
@@ -35,7 +36,7 @@ groups() ->
                     multiple_upstreams,
                     multiple_upstreams_pattern,
                     multiple_downstreams,
-                    bidirectional,
+                    message_flow,
                     dynamic_reconfiguration,
                     federate_unfederate,
                     dynamic_plugin_stop_start
@@ -239,7 +240,7 @@ multiple_downstreams(Config) ->
               expect_federation(Ch, <<"upstream">>, <<"fed.downstream2">>, ?EXPECT_FEDERATION_TIMEOUT)
       end, upstream_downstream(Config) ++ [q(<<"fed.downstream2">>, Args)]).
 
-bidirectional(Config) ->
+message_flow(Config) ->
     %% TODO: specifc source / target here
     Args = ?config(source_queue_args, Config),
     with_ch(Config,
@@ -257,7 +258,13 @@ bidirectional(Config) ->
               [publish(Ch, <<>>, <<"two">>, <<"bulk">>) || _ <- Seq],
               expect(Ch, <<"two">>, repeat(100, <<"bulk">>)),
               expect_empty(Ch, <<"one">>),
-              expect_empty(Ch, <<"two">>)
+              expect_empty(Ch, <<"two">>),
+              %% We clear the federation configuration to avoid a race condition
+              %% when deleting the queues in quorum mode. The federation link
+              %% would restart and lead to a state where nothing happened for
+              %% minutes.
+              clear_upstream_set(Config, 0, <<"one">>),
+              clear_upstream_set(Config, 0, <<"two">>)
       end, [q(<<"one">>, Args),
             q(<<"two">>, Args)]).
 

release-notes/3.8.23.md

@@ -45,10 +45,16 @@ consistent release schedule.
 
 #### Bug Fixes
 
-* TLS information delivered in [Proxy protocol](https://www.rabbitmq.com/networking.html#proxy-protocol) header is now attached to connection metrics as if it was provided by a non-proxying client.
+ * TLS information delivered in [Proxy protocol](https://www.rabbitmq.com/networking.html#proxy-protocol) header is now attached to connection metrics as if it was provided by a non-proxying client.
 
    GitHub issue: [#3175](https://github.com/rabbitmq/rabbitmq-server/pull/3175) contributed by @prefiks, sponsored by CloudAMQP
 
+ * Classic queue shutdown now uses a much higher timeout (up to 10 minutes instead of 30 seconds).
+
+   In environments with many queues (especially mirrored queues) and many consumers this means that
+   the chance of queue indices rebuilding after node restart is now substantially lower.
+
+   GitHub issue: [#3409](https://github.com/rabbitmq/rabbitmq-server/pull/3409)
 
 
 ## Dependency Upgrades

release-notes/3.9.6.md

@@ -45,6 +45,13 @@ Contributors are encouraged to update them together with their changes. This hel
 
    GitHub issue: [#3340](https://github.com/rabbitmq/rabbitmq-server/pull/3340)
 
+ * Classic queue shutdown now uses a much higher timeout (up to 10 minutes instead of 30 seconds).
+
+   In environments with many queues (especially mirrored queues) and many consumers this means that
+   the chance of queue indices rebuilding after node restart is now substantially lower.
+
+   GitHub issue: [#3409](https://github.com/rabbitmq/rabbitmq-server/pull/3409)
+
 
 ### Management Plugin