summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon MacMullen <simon@rabbitmq.com>2014-04-23 14:57:50 +0100
committerSimon MacMullen <simon@rabbitmq.com>2014-04-23 14:57:50 +0100
commit3059534426122b0aa27182286958d46910d9cdef (patch)
tree3f3db401246d5b863f9d5bae8a1c1aff68e4aa48
parent074d731e6e978855006aa6d924e568db98e9d775 (diff)
downloadrabbitmq-server-3059534426122b0aa27182286958d46910d9cdef.tar.gz
Introduce a new function for the case of dropping a master because we synced - remove a deadlock.bug26125
-rw-r--r--src/rabbit_mirror_queue_misc.erl24
-rw-r--r--src/rabbit_mirror_queue_slave.erl8
2 files changed, 26 insertions, 6 deletions
diff --git a/src/rabbit_mirror_queue_misc.erl b/src/rabbit_mirror_queue_misc.erl
index 256543de..b0f092a9 100644
--- a/src/rabbit_mirror_queue_misc.erl
+++ b/src/rabbit_mirror_queue_misc.erl
@@ -21,7 +21,8 @@
report_deaths/4, store_updated_slaves/1,
initial_queue_node/2, suggested_queue_nodes/1,
is_mirrored/1, update_mirrors/2, validate_policy/1,
- maybe_auto_sync/1, log_info/3, log_warning/3]).
+ maybe_auto_sync/1, maybe_drop_master_after_sync/1,
+ log_info/3, log_warning/3]).
%% for testing only
-export([module/1]).
@@ -57,6 +58,7 @@
-spec(is_mirrored/1 :: (rabbit_types:amqqueue()) -> boolean()).
-spec(update_mirrors/2 ::
(rabbit_types:amqqueue(), rabbit_types:amqqueue()) -> 'ok').
+-spec(maybe_drop_master_after_sync/1 :: (rabbit_types:amqqueue()) -> 'ok').
-spec(maybe_auto_sync/1 :: (rabbit_types:amqqueue()) -> 'ok').
-spec(log_info/3 :: (rabbit_amqqueue:name(), string(), [any()]) -> 'ok').
-spec(log_warning/3 :: (rabbit_amqqueue:name(), string(), [any()]) -> 'ok').
@@ -346,6 +348,26 @@ update_mirrors0(OldQ = #amqqueue{name = QName},
maybe_auto_sync(NewQ),
ok.
+%% The arrival of a newly synced slave may cause the master to die if
+%% the policy does not want the master but it has been kept alive
+%% because there were no synced slaves.
+%%
+%% We don't just call update_mirrors/2 here since that could decide to
+%% start a slave for some other reason, and since we are the slave ATM
+%% that allows complicated deadlocks.
+maybe_drop_master_after_sync(Q = #amqqueue{name = QName,
+ pid = MPid}) ->
+ {DesiredMNode, DesiredSNodes} = suggested_queue_nodes(Q),
+ case node(MPid) of
+ DesiredMNode -> ok;
+ OldMNode -> false = lists:member(OldMNode, DesiredSNodes), %% [0]
+ drop_mirror(QName, OldMNode)
+ end,
+ ok.
+%% [0] ASSERTION - if the policy wants the master to change, it has
+%% not just shuffled it into the slaves. All our modes ensure this
+%% does not happen, but we should guard against a misbehaving plugin.
+
%%----------------------------------------------------------------------------
validate_policy(KeyList) ->
diff --git a/src/rabbit_mirror_queue_slave.erl b/src/rabbit_mirror_queue_slave.erl
index f6acd91a..ee889f84 100644
--- a/src/rabbit_mirror_queue_slave.erl
+++ b/src/rabbit_mirror_queue_slave.erl
@@ -922,8 +922,6 @@ update_ram_duration(BQ, BQS) ->
rabbit_memory_monitor:report_ram_duration(self(), RamDuration),
BQ:set_ram_duration_target(DesiredDuration, BQS1).
-%% [1] - the arrival of this newly synced slave may cause the master to die if
-%% the admin has requested a migration-type change to policy.
record_synchronised(#amqqueue { name = QName }) ->
Self = self(),
case rabbit_misc:execute_mnesia_transaction(
@@ -934,9 +932,9 @@ record_synchronised(#amqqueue { name = QName }) ->
[Q1 = #amqqueue { sync_slave_pids = SSPids }] ->
Q2 = Q1#amqqueue{sync_slave_pids = [Self | SSPids]},
rabbit_mirror_queue_misc:store_updated_slaves(Q2),
- {ok, Q1, Q2}
+ {ok, Q2}
end
end) of
- ok -> ok;
- {ok, Q1, Q2} -> rabbit_mirror_queue_misc:update_mirrors(Q1, Q2) %% [1]
+ ok -> ok;
+ {ok, Q} -> rabbit_mirror_queue_misc:maybe_drop_master_after_sync(Q)
end.