From dd3256cdc7048de9e2f32569b875cd104e03a11a Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 31 Oct 2014 13:08:16 +0000 Subject: Inline parse_field_value/1 and thus prevent sub-binary construction in accordance with the principles documented at http://www.erlang.org/doc/efficiency_guide/binaryhandling.html --- Makefile | 2 +- src/rabbit_binary_parser.erl | 103 ++++++++++++++++++++++++++++++------------- 2 files changed, 74 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index c955a8fc..1b66a306 100644 --- a/Makefile +++ b/Makefile @@ -57,7 +57,7 @@ USE_PROPER_QC:=$(shell erl -noshell -eval 'io:format({module, proper} =:= code:e endif #other args: +native +"{hipe,[o3,verbose]}" -Ddebug=true +debug_info +no_strict_record_tests -ERLC_OPTS=-I $(INCLUDE_DIR) -Wall -v +debug_info $(call boolean_macro,$(USE_SPECS),use_specs) $(call boolean_macro,$(USE_PROPER_QC),use_proper_qc) +ERLC_OPTS=-I $(INCLUDE_DIR) -Wall -v +debug_info +bin_opt_info $(call boolean_macro,$(USE_SPECS),use_specs) $(call boolean_macro,$(USE_PROPER_QC),use_proper_qc) ifdef INSTRUMENT_FOR_QC ERLC_OPTS += -DINSTR_MOD=gm_qc diff --git a/src/rabbit_binary_parser.erl b/src/rabbit_binary_parser.erl index 3ab82cad..8850d69e 100644 --- a/src/rabbit_binary_parser.erl +++ b/src/rabbit_binary_parser.erl @@ -41,48 +41,91 @@ %% parse_table supports the AMQP 0-8/0-9 standard types, S, I, D, T %% and F, as well as the QPid extensions b, d, f, l, s, t, x, and V. +-define(SIMPLE_PARSE_TABLE(BType, Pattern, RType), + parse_table(<>) -> + [{NameString, RType, Value} | parse_table(Rest)]). + +%% Note that we try to put these in approximately the order we expect +%% to hit them, that's why the empty binary is half way through. + +parse_table(<>) -> + [{NameString, longstr, Value} | parse_table(Rest)]; + +?SIMPLE_PARSE_TABLE($I, Value:32/signed, signedint); +?SIMPLE_PARSE_TABLE($T, Value:64/unsigned, timestamp); + parse_table(<<>>) -> []; -parse_table(<>) -> - {Type, Value, Rest} = parse_field_value(ValueAndRest), - [{NameString, Type, Value} | parse_table(Rest)]. -parse_array(<<>>) -> - []; -parse_array(<>) -> - {Type, Value, Rest} = parse_field_value(ValueAndRest), - [{Type, Value} | parse_array(Rest)]. +?SIMPLE_PARSE_TABLE($b, Value:8/signed, byte); +?SIMPLE_PARSE_TABLE($d, Value:64/float, double); +?SIMPLE_PARSE_TABLE($f, Value:32/float, float); +?SIMPLE_PARSE_TABLE($l, Value:64/signed, long); +?SIMPLE_PARSE_TABLE($s, Value:16/signed, short); + +parse_table(<>) -> + [{NameString, bool, (Value /= 0)} | parse_table(Rest)]; + +parse_table(<>) -> + [{NameString, decimal, {Before, After}} | parse_table(Rest)]; -parse_field_value(<<$S, VLen:32/unsigned, V:VLen/binary, R/binary>>) -> - {longstr, V, R}; +parse_table(<>) -> + [{NameString, table, parse_table(Value)} | parse_table(Rest)]; -parse_field_value(<<$I, V:32/signed, R/binary>>) -> - {signedint, V, R}; +parse_table(<>) -> + [{NameString, array, parse_array(Value)} | parse_table(Rest)]; + +parse_table(<>) -> + [{NameString, binary, Value} | parse_table(Rest)]; + +parse_table(<>) -> + [{NameString, void, undefined} | parse_table(Rest)]. + +-define(SIMPLE_PARSE_ARRAY(BType, Pattern, RType), + parse_array(<>) -> + [{RType, Value} | parse_table(Rest)]). + +parse_array(<>) -> + [{NameString, longstr, Value} | parse_array(Rest)]; + +?SIMPLE_PARSE_ARRAY($I, Value:32/signed, signedint); +?SIMPLE_PARSE_ARRAY($T, Value:64/unsigned, timestamp); + +parse_array(<<>>) -> + []; -parse_field_value(<<$D, Before:8/unsigned, After:32/unsigned, R/binary>>) -> - {decimal, {Before, After}, R}; +?SIMPLE_PARSE_ARRAY($b, Value:8/signed, byte); +?SIMPLE_PARSE_ARRAY($d, Value:64/float, double); +?SIMPLE_PARSE_ARRAY($f, Value:32/float, float); +?SIMPLE_PARSE_ARRAY($l, Value:64/signed, long); +?SIMPLE_PARSE_ARRAY($s, Value:16/signed, short); -parse_field_value(<<$T, V:64/unsigned, R/binary>>) -> - {timestamp, V, R}; +parse_array(<<$t, Value:8/unsigned, Rest/binary>>) -> + [{bool, (Value /= 0)} | parse_array(Rest)]; -parse_field_value(<<$F, VLen:32/unsigned, Table:VLen/binary, R/binary>>) -> - {table, parse_table(Table), R}; +parse_array(<<$D, Before:8/unsigned, After:32/unsigned, Rest/binary>>) -> + [{decimal, {Before, After}} | parse_array(Rest)]; -parse_field_value(<<$A, VLen:32/unsigned, Array:VLen/binary, R/binary>>) -> - {array, parse_array(Array), R}; +parse_array(<<$F, VLen:32/unsigned, Value:VLen/binary, Rest/binary>>) -> + [{table, parse_table(Value)} | parse_array(Rest)]; -parse_field_value(<<$b, V:8/signed, R/binary>>) -> {byte, V, R}; -parse_field_value(<<$d, V:64/float, R/binary>>) -> {double, V, R}; -parse_field_value(<<$f, V:32/float, R/binary>>) -> {float, V, R}; -parse_field_value(<<$l, V:64/signed, R/binary>>) -> {long, V, R}; -parse_field_value(<<$s, V:16/signed, R/binary>>) -> {short, V, R}; -parse_field_value(<<$t, V:8/unsigned, R/binary>>) -> {bool, (V /= 0), R}; +parse_array(<<$A, VLen:32/unsigned, Value:VLen/binary, Rest/binary>>) -> + [{array, parse_array(Value)} | parse_array(Rest)]; -parse_field_value(<<$x, VLen:32/unsigned, V:VLen/binary, R/binary>>) -> - {binary, V, R}; +parse_array(<<$x, VLen:32/unsigned, Value:VLen/binary, Rest/binary>>) -> + [{binary, Value} | parse_array(Rest)]; -parse_field_value(<<$V, R/binary>>) -> - {void, undefined, R}. +parse_array(<<$V, Rest/binary>>) -> + [{void, undefined} | parse_array(Rest)]. ensure_content_decoded(Content = #content{properties = Props}) when Props =/= none -> -- cgit v1.2.1 From e947a3923bfe20d99d43c9341370b137a33607b7 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 31 Oct 2014 13:09:18 +0000 Subject: Oops --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1b66a306..c955a8fc 100644 --- a/Makefile +++ b/Makefile @@ -57,7 +57,7 @@ USE_PROPER_QC:=$(shell erl -noshell -eval 'io:format({module, proper} =:= code:e endif #other args: +native +"{hipe,[o3,verbose]}" -Ddebug=true +debug_info +no_strict_record_tests -ERLC_OPTS=-I $(INCLUDE_DIR) -Wall -v +debug_info +bin_opt_info $(call boolean_macro,$(USE_SPECS),use_specs) $(call boolean_macro,$(USE_PROPER_QC),use_proper_qc) +ERLC_OPTS=-I $(INCLUDE_DIR) -Wall -v +debug_info $(call boolean_macro,$(USE_SPECS),use_specs) $(call boolean_macro,$(USE_PROPER_QC),use_proper_qc) ifdef INSTRUMENT_FOR_QC ERLC_OPTS += -DINSTR_MOD=gm_qc -- cgit v1.2.1 From 4d7c548ddb5e8109fe18984e4dadb4f0cbb86078 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 3 Nov 2014 11:28:31 +0000 Subject: That shouldn't be a complete copy-paste of the parse_table/1 version. --- src/rabbit_binary_parser.erl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/rabbit_binary_parser.erl b/src/rabbit_binary_parser.erl index 8850d69e..6e277d35 100644 --- a/src/rabbit_binary_parser.erl +++ b/src/rabbit_binary_parser.erl @@ -93,9 +93,8 @@ parse_table(<>) -> [{RType, Value} | parse_table(Rest)]). -parse_array(<>) -> - [{NameString, longstr, Value} | parse_array(Rest)]; +parse_array(<<$S, VLen:32/unsigned, Value:VLen/binary, Rest/binary>>) -> + [{longstr, Value} | parse_array(Rest)]; ?SIMPLE_PARSE_ARRAY($I, Value:32/signed, signedint); ?SIMPLE_PARSE_ARRAY($T, Value:64/unsigned, timestamp); -- cgit v1.2.1 From 694943dc70c2466ee8d3cdcaf190081645a3b160 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 3 Nov 2014 13:19:43 +0000 Subject: ...and another stupid bug --- src/rabbit_binary_parser.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rabbit_binary_parser.erl b/src/rabbit_binary_parser.erl index 6e277d35..ee8147f4 100644 --- a/src/rabbit_binary_parser.erl +++ b/src/rabbit_binary_parser.erl @@ -91,7 +91,7 @@ parse_table(<>) -> - [{RType, Value} | parse_table(Rest)]). + [{RType, Value} | parse_array(Rest)]). parse_array(<<$S, VLen:32/unsigned, Value:VLen/binary, Rest/binary>>) -> [{longstr, Value} | parse_array(Rest)]; -- cgit v1.2.1 From c8c5c1504d8b39ceb956adf07f1cbba1147e6990 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 6 Nov 2014 11:39:25 +0000 Subject: Simple statistics about reads, writes and syncs through the FHC. --- src/file_handle_cache.erl | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 3a7a692c..cec4bccc 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -335,7 +335,7 @@ read(Ref, Count) -> fun ([#handle { is_read = false }]) -> {error, not_open_for_reading}; ([Handle = #handle { hdl = Hdl, offset = Offset }]) -> - case prim_file:read(Hdl, Count) of + case prim_file_read(Hdl, Count) of {ok, Data} = Obj -> Offset1 = Offset + iolist_size(Data), {Obj, [Handle #handle { offset = Offset1 }]}; @@ -355,7 +355,7 @@ append(Ref, Data) -> write_buffer_size_limit = 0, at_eof = true } = Handle1} -> Offset1 = Offset + iolist_size(Data), - {prim_file:write(Hdl, Data), + {prim_file_write(Hdl, Data), [Handle1 #handle { is_dirty = true, offset = Offset1 }]}; {{ok, _Offset}, #handle { write_buffer = WriteBuffer, write_buffer_size = Size, @@ -382,7 +382,7 @@ sync(Ref) -> ok; ([Handle = #handle { hdl = Hdl, is_dirty = true, write_buffer = [] }]) -> - case prim_file:sync(Hdl) of + case prim_file_sync(Hdl) of ok -> {ok, [Handle #handle { is_dirty = false }]}; Error -> {Error, [Handle]} end @@ -539,6 +539,17 @@ info(Items) -> gen_server2:call(?SERVER, {info, Items}, infinity). %% Internal functions %%---------------------------------------------------------------------------- +prim_file_read(Hdl, Size) -> + file_handle_cache_stats:update( + read, Size, fun() -> prim_file:read(Hdl, Size) end). + +prim_file_write(Hdl, Bytes) -> + file_handle_cache_stats:update( + write, iolist_size(Bytes), fun() -> prim_file:write(Hdl, Bytes) end). + +prim_file_sync(Hdl) -> + file_handle_cache_stats:update(sync, fun() -> prim_file:sync(Hdl) end). + is_reader(Mode) -> lists:member(read, Mode). is_writer(Mode) -> lists:member(write, Mode). @@ -742,7 +753,7 @@ soft_close(Handle) -> is_dirty = IsDirty, last_used_at = Then } = Handle1 } -> ok = case IsDirty of - true -> prim_file:sync(Hdl); + true -> prim_file_sync(Hdl); false -> ok end, ok = prim_file:close(Hdl), @@ -817,7 +828,7 @@ write_buffer(Handle = #handle { hdl = Hdl, offset = Offset, write_buffer = WriteBuffer, write_buffer_size = DataSize, at_eof = true }) -> - case prim_file:write(Hdl, lists:reverse(WriteBuffer)) of + case prim_file_write(Hdl, lists:reverse(WriteBuffer)) of ok -> Offset1 = Offset + DataSize, {ok, Handle #handle { offset = Offset1, is_dirty = true, @@ -843,6 +854,7 @@ used(#fhc_state{open_count = C1, %%---------------------------------------------------------------------------- init([AlarmSet, AlarmClear]) -> + file_handle_cache_stats:init(), Limit = case application:get_env(file_handles_high_watermark) of {ok, Watermark} when (is_integer(Watermark) andalso Watermark > 0) -> -- cgit v1.2.1 From 28274a6f2ed0a6b0a26e9ea0a80ef3a16e98274b Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 6 Nov 2014 12:49:44 +0000 Subject: Forgot to add that... --- src/file_handle_cache_stats.erl | 53 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 src/file_handle_cache_stats.erl diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl new file mode 100644 index 00000000..c8af20a2 --- /dev/null +++ b/src/file_handle_cache_stats.erl @@ -0,0 +1,53 @@ +%% The contents of this file are subject to the Mozilla Public License +%% Version 1.1 (the "License"); you may not use this file except in +%% compliance with the License. You may obtain a copy of the License +%% at http://www.mozilla.org/MPL/ +%% +%% Software distributed under the License is distributed on an "AS IS" +%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See +%% the License for the specific language governing rights and +%% limitations under the License. +%% +%% The Original Code is RabbitMQ. +%% +%% The Initial Developer of the Original Code is GoPivotal, Inc. +%% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. +%% + +-module(file_handle_cache_stats). + +%% stats about read / write operations that go through the fhc. + +-export([init/0, update/3, update/2, get/0]). + +-define(TABLE, ?MODULE). +-define(MICRO_TO_MILLI, 1000). + +init() -> + ets:new(?TABLE, [public, named_table]), + [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [read, write], + Counter <- [count, bytes, time]], + [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [sync], + Counter <- [count, time]]. + +update(Op, Bytes, Thunk) -> + {Time, Res} = timer:tc(Thunk), + ets:update_counter(?TABLE, {Op, count}, 1), + ets:update_counter(?TABLE, {Op, bytes}, Bytes), + ets:update_counter(?TABLE, {Op, time}, Time), + Res. + +update(Op, Thunk) -> + {Time, Res} = timer:tc(Thunk), + ets:update_counter(?TABLE, {Op, count}, 1), + ets:update_counter(?TABLE, {Op, time}, Time), + Res. + +get() -> + lists:sort([output(K, V) || {K, V} <- ets:tab2list(?TABLE)]). + +output({Op, time}, Val) -> {flatten_key(Op, time), Val / ?MICRO_TO_MILLI}; +output({Op, Ctr}, Val) -> {flatten_key(Op, Ctr), Val}. + +flatten_key(A, B) -> + list_to_atom("fhc_" ++ atom_to_list(A) ++ "_" ++ atom_to_list(B)). -- cgit v1.2.1 From 5957db9e3d058335f70d0c80acf8e28f14176bfc Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 6 Nov 2014 13:51:48 +0000 Subject: Move this formatting up to the agent. --- src/file_handle_cache_stats.erl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl index c8af20a2..d055d84a 100644 --- a/src/file_handle_cache_stats.erl +++ b/src/file_handle_cache_stats.erl @@ -21,7 +21,6 @@ -export([init/0, update/3, update/2, get/0]). -define(TABLE, ?MODULE). --define(MICRO_TO_MILLI, 1000). init() -> ets:new(?TABLE, [public, named_table]), @@ -44,10 +43,4 @@ update(Op, Thunk) -> Res. get() -> - lists:sort([output(K, V) || {K, V} <- ets:tab2list(?TABLE)]). - -output({Op, time}, Val) -> {flatten_key(Op, time), Val / ?MICRO_TO_MILLI}; -output({Op, Ctr}, Val) -> {flatten_key(Op, Ctr), Val}. - -flatten_key(A, B) -> - list_to_atom("fhc_" ++ atom_to_list(A) ++ "_" ++ atom_to_list(B)). + lists:sort(ets:tab2list(?TABLE)). -- cgit v1.2.1 From c2088028ff1cbf4a3d4d680f87054e9b5aecb345 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 10 Nov 2014 13:59:05 +0000 Subject: First pass at adding a read buffer to file_handle_cache. --- src/file_handle_cache.erl | 104 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 82 insertions(+), 22 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index cec4bccc..126b3f81 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -178,6 +178,9 @@ write_buffer_size, write_buffer_size_limit, write_buffer, + read_buffer, + read_buffer_size, + read_buffer_size_limit, at_eof, path, mode, @@ -334,13 +337,42 @@ read(Ref, Count) -> [Ref], fun ([#handle { is_read = false }]) -> {error, not_open_for_reading}; - ([Handle = #handle { hdl = Hdl, offset = Offset }]) -> - case prim_file_read(Hdl, Count) of - {ok, Data} = Obj -> Offset1 = Offset + iolist_size(Data), - {Obj, - [Handle #handle { offset = Offset1 }]}; - eof -> {eof, [Handle #handle { at_eof = true }]}; - Error -> {Error, [Handle]} + ([Handle = #handle{read_buffer = Buf, + read_buffer_size = BufSz, + offset = Offset}]) when BufSz >= Count -> + <> = Buf, + {{ok, Hd}, [Handle#handle{offset = Offset + Count, + read_buffer = Tl, + read_buffer_size = BufSz - Count}]}; + ([Handle = #handle{read_buffer = Buf, + read_buffer_size = BufSz, + read_buffer_size_limit = Limit, + hdl = Hdl, + offset = Offset}]) -> + WantedCount = Count - BufSz, + case prim_file_read(Hdl, Limit) of + {ok, Data} -> + ReadCount = size(Data), + case ReadCount < WantedCount of + true -> + OffSet1 = Offset + BufSz + ReadCount, + {{ok, <>}, + [Handle#handle{offset = OffSet1, + read_buffer = <<>>, + read_buffer_size = 0}]}; + false -> + <> = Data, + OffSet1 = Offset + BufSz + WantedCount, + BufSz1 = ReadCount - WantedCount, + {{ok, <>}, + [Handle#handle{offset = OffSet1, + read_buffer = Tl, + read_buffer_size = BufSz1}]} + end; + eof -> + {eof, [Handle #handle { at_eof = true }]}; + Error -> %% TODO correct or change handle? + {Error, [Handle]} end end). @@ -465,8 +497,10 @@ clear(Ref) -> fun ([#handle { at_eof = true, write_buffer_size = 0, offset = 0 }]) -> ok; ([Handle]) -> - case maybe_seek(bof, Handle #handle { write_buffer = [], - write_buffer_size = 0 }) of + case maybe_seek(bof, Handle #handle { write_buffer = [], + write_buffer_size = 0, + read_buffer = <<>>, + read_buffer_size = 0}) of {{ok, 0}, Handle1 = #handle { hdl = Hdl }} -> case prim_file:truncate(Hdl) of ok -> {ok, [Handle1 #handle { at_eof = true }]}; @@ -633,9 +667,11 @@ reopen([{Ref, NewOrReopen, Handle = #handle { hdl = closed, {ok, Hdl} -> Now = now(), {{ok, _Offset}, Handle1} = - maybe_seek(Offset, Handle #handle { hdl = Hdl, - offset = 0, - last_used_at = Now }), + maybe_seek(Offset, Handle #handle { hdl = Hdl, + offset = 0, + read_buffer = <<>>, + read_buffer_size = 0, + last_used_at = Now }), put({Ref, fhc_handle}, Handle1), reopen(RefNewOrReopenHdls, gb_trees:insert(Now, Ref, Tree), [{Ref, Handle1} | RefHdls]); @@ -727,6 +763,9 @@ new_closed_handle(Path, Mode, Options) -> write_buffer_size = 0, write_buffer_size_limit = WriteBufferSize, write_buffer = [], + read_buffer_size = 0, + read_buffer_size_limit = 1000000, %% TODO + read_buffer = <<>>, at_eof = false, path = Path, mode = Mode, @@ -787,17 +826,38 @@ hard_close(Handle) -> Result end. -maybe_seek(NewOffset, Handle = #handle { hdl = Hdl, offset = Offset, - at_eof = AtEoF }) -> +maybe_seek(NewOffset, Handle = #handle{hdl = Hdl, + offset = Offset, + read_buffer = Buf, + read_buffer_size = BufSz, + at_eof = AtEoF}) -> {AtEoF1, NeedsSeek} = needs_seek(AtEoF, Offset, NewOffset), - case (case NeedsSeek of - true -> prim_file:position(Hdl, NewOffset); - false -> {ok, Offset} - end) of - {ok, Offset1} = Result -> - {Result, Handle #handle { offset = Offset1, at_eof = AtEoF1 }}; - {error, _} = Error -> - {Error, Handle} + case NeedsSeek of + true -> + case not is_number(NewOffset) orelse + NewOffset < Offset orelse + NewOffset > BufSz + Offset of + true -> + case prim_file:position(Hdl, NewOffset) of + {ok, Offset1} = Result -> + {Result, Handle#handle{offset = Offset1, + at_eof = AtEoF1, + read_buffer = <<>>, + read_buffer_size = 0}}; + {error, _} = Error -> + {Error, Handle} + end; + false -> + Diff = NewOffset - Offset, + <<_:Diff/binary, Rest/binary>> = Buf, + {{ok, NewOffset}, + Handle#handle{offset = NewOffset, + at_eof = AtEoF1, + read_buffer = Rest, + read_buffer_size = BufSz - Diff}} + end; + false -> + {{ok, Offset}, Handle} end. needs_seek( AtEoF, _CurOffset, cur ) -> {AtEoF, false}; -- cgit v1.2.1 From 0e338d6fc8638f8eadd594ec72c1e766f3350958 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Tue, 11 Nov 2014 16:46:20 +0000 Subject: Also generate stats on seek. --- src/file_handle_cache.erl | 6 +++++- src/file_handle_cache_stats.erl | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index cec4bccc..3364c3ae 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -550,6 +550,10 @@ prim_file_write(Hdl, Bytes) -> prim_file_sync(Hdl) -> file_handle_cache_stats:update(sync, fun() -> prim_file:sync(Hdl) end). +prim_file_position(Hdl, NewOffset) -> + file_handle_cache_stats:update( + seek, fun() -> prim_file:position(Hdl, NewOffset) end). + is_reader(Mode) -> lists:member(read, Mode). is_writer(Mode) -> lists:member(write, Mode). @@ -791,7 +795,7 @@ maybe_seek(NewOffset, Handle = #handle { hdl = Hdl, offset = Offset, at_eof = AtEoF }) -> {AtEoF1, NeedsSeek} = needs_seek(AtEoF, Offset, NewOffset), case (case NeedsSeek of - true -> prim_file:position(Hdl, NewOffset); + true -> prim_file_position(Hdl, NewOffset); false -> {ok, Offset} end) of {ok, Offset1} = Result -> diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl index d055d84a..5a8d7b29 100644 --- a/src/file_handle_cache_stats.erl +++ b/src/file_handle_cache_stats.erl @@ -26,7 +26,7 @@ init() -> ets:new(?TABLE, [public, named_table]), [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [read, write], Counter <- [count, bytes, time]], - [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [sync], + [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [sync, seek], Counter <- [count, time]]. update(Op, Bytes, Thunk) -> -- cgit v1.2.1 From 54483ee88d6516caa56ced399e5a8cea7a72a1e3 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Tue, 11 Nov 2014 17:06:42 +0000 Subject: Emit stats on FHC recycling. --- src/file_handle_cache.erl | 12 +++++++----- src/file_handle_cache_stats.erl | 10 ++++++++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 3364c3ae..65f3f45b 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -626,14 +626,16 @@ reopen([], Tree, RefHdls) -> {ok, lists:reverse(RefHdls)}; reopen([{Ref, NewOrReopen, Handle = #handle { hdl = closed, path = Path, - mode = Mode, + mode = Mode0, offset = Offset, last_used_at = undefined }} | RefNewOrReopenHdls] = ToOpen, Tree, RefHdls) -> - case prim_file:open(Path, case NewOrReopen of - new -> Mode; - reopen -> [read | Mode] - end) of + Mode = case NewOrReopen of + new -> Mode0; + reopen -> file_handle_cache_stats:update(reopen), + [read | Mode0] + end, + case prim_file:open(Path, Mode) of {ok, Hdl} -> Now = now(), {{ok, _Offset}, Handle1} = diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl index 5a8d7b29..b1fbb3f4 100644 --- a/src/file_handle_cache_stats.erl +++ b/src/file_handle_cache_stats.erl @@ -18,7 +18,7 @@ %% stats about read / write operations that go through the fhc. --export([init/0, update/3, update/2, get/0]). +-export([init/0, update/3, update/2, update/1, get/0]). -define(TABLE, ?MODULE). @@ -27,7 +27,9 @@ init() -> [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [read, write], Counter <- [count, bytes, time]], [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [sync, seek], - Counter <- [count, time]]. + Counter <- [count, time]], + [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- [reopen], + Counter <- [count]]. update(Op, Bytes, Thunk) -> {Time, Res} = timer:tc(Thunk), @@ -42,5 +44,9 @@ update(Op, Thunk) -> ets:update_counter(?TABLE, {Op, time}, Time), Res. +update(Op) -> + ets:update_counter(?TABLE, {Op, count}, 1), + ok. + get() -> lists:sort(ets:tab2list(?TABLE)). -- cgit v1.2.1 From cab3f90898beac09f77d75219800f84b52f16c05 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 09:22:02 +0000 Subject: R13B03 compatibility. --- src/file_handle_cache_stats.erl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl index b1fbb3f4..832f0b3d 100644 --- a/src/file_handle_cache_stats.erl +++ b/src/file_handle_cache_stats.erl @@ -32,14 +32,14 @@ init() -> Counter <- [count]]. update(Op, Bytes, Thunk) -> - {Time, Res} = timer:tc(Thunk), + {Time, Res} = timer_tc(Thunk), ets:update_counter(?TABLE, {Op, count}, 1), ets:update_counter(?TABLE, {Op, bytes}, Bytes), ets:update_counter(?TABLE, {Op, time}, Time), Res. update(Op, Thunk) -> - {Time, Res} = timer:tc(Thunk), + {Time, Res} = timer_tc(Thunk), ets:update_counter(?TABLE, {Op, count}, 1), ets:update_counter(?TABLE, {Op, time}, Time), Res. @@ -50,3 +50,11 @@ update(Op) -> get() -> lists:sort(ets:tab2list(?TABLE)). + +%% TODO timer:tc/1 was introduced in R14B03; use that function once we +%% require that version. +timer_tc(Thunk) -> + T1 = os:timestamp(), + Res = Thunk(), + T2 = os:timestamp(), + {timer:now_diff(T2, T1), Res}. -- cgit v1.2.1 From 16b013458097e1a9a7d20f0138c24f705a8251f3 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 10:20:27 +0000 Subject: If they ask to read more than the buffer size, do so. --- src/file_handle_cache.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 519c596d..a7d5ce15 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -346,11 +346,11 @@ read(Ref, Count) -> read_buffer_size = BufSz - Count}]}; ([Handle = #handle{read_buffer = Buf, read_buffer_size = BufSz, - read_buffer_size_limit = Limit, + read_buffer_size_limit = BufSzLimit, hdl = Hdl, offset = Offset}]) -> WantedCount = Count - BufSz, - case prim_file_read(Hdl, Limit) of + case prim_file_read(Hdl, lists:max([BufSzLimit, WantedCount])) of {ok, Data} -> ReadCount = size(Data), case ReadCount < WantedCount of -- cgit v1.2.1 From b573d47f639fdc8f8b5ff869252543b00b1a8fda Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 10:56:33 +0000 Subject: Be a bit more systematic about reseting the read buffer. --- src/file_handle_cache.erl | 51 ++++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index a7d5ce15..fa896c67 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -334,7 +334,7 @@ close(Ref) -> read(Ref, Count) -> with_flushed_handles( - [Ref], + [Ref], keep, fun ([#handle { is_read = false }]) -> {error, not_open_for_reading}; ([Handle = #handle{read_buffer = Buf, @@ -357,9 +357,8 @@ read(Ref, Count) -> true -> OffSet1 = Offset + BufSz + ReadCount, {{ok, <>}, - [Handle#handle{offset = OffSet1, - read_buffer = <<>>, - read_buffer_size = 0}]}; + [reset_read_buffer( + Handle#handle{offset = OffSet1})]}; false -> <> = Data, OffSet1 = Offset + BufSz + WantedCount, @@ -409,7 +408,7 @@ append(Ref, Data) -> sync(Ref) -> with_flushed_handles( - [Ref], + [Ref], keep, fun ([#handle { is_dirty = false, write_buffer = [] }]) -> ok; ([Handle = #handle { hdl = Hdl, @@ -429,7 +428,7 @@ needs_sync(Ref) -> position(Ref, NewOffset) -> with_flushed_handles( - [Ref], + [Ref], keep, fun ([Handle]) -> {Result, Handle1} = maybe_seek(NewOffset, Handle), {Result, [Handle1]} end). @@ -497,10 +496,8 @@ clear(Ref) -> fun ([#handle { at_eof = true, write_buffer_size = 0, offset = 0 }]) -> ok; ([Handle]) -> - case maybe_seek(bof, Handle #handle { write_buffer = [], - write_buffer_size = 0, - read_buffer = <<>>, - read_buffer_size = 0}) of + case maybe_seek(bof, Handle#handle{write_buffer = [], + write_buffer_size = 0}) of {{ok, 0}, Handle1 = #handle { hdl = Hdl }} -> case prim_file:truncate(Hdl) of ok -> {ok, [Handle1 #handle { at_eof = true }]}; @@ -599,8 +596,15 @@ append_to_write(Mode) -> end. with_handles(Refs, Fun) -> + with_handles(Refs, reset, Fun). + +with_handles(Refs, ReadBuffer, Fun) -> case get_or_reopen([{Ref, reopen} || Ref <- Refs]) of - {ok, Handles} -> + {ok, Handles0} -> + Handles = case ReadBuffer of + reset -> [reset_read_buffer(H) || H <- Handles0]; + keep -> Handles0 + end, case Fun(Handles) of {Result, Handles1} when is_list(Handles1) -> lists:zipwith(fun put_handle/2, Refs, Handles1), @@ -613,8 +617,11 @@ with_handles(Refs, Fun) -> end. with_flushed_handles(Refs, Fun) -> + with_flushed_handles(Refs, reset, Fun). + +with_flushed_handles(Refs, ReadBuffer, Fun) -> with_handles( - Refs, + Refs, ReadBuffer, fun (Handles) -> case lists:foldl( fun (Handle, {ok, HandlesAcc}) -> @@ -673,11 +680,10 @@ reopen([{Ref, NewOrReopen, Handle = #handle { hdl = closed, {ok, Hdl} -> Now = now(), {{ok, _Offset}, Handle1} = - maybe_seek(Offset, Handle #handle { hdl = Hdl, - offset = 0, - read_buffer = <<>>, - read_buffer_size = 0, - last_used_at = Now }), + maybe_seek(Offset, reset_read_buffer( + Handle#handle{hdl = Hdl, + offset = 0, + last_used_at = Now})), put({Ref, fhc_handle}, Handle1), reopen(RefNewOrReopenHdls, gb_trees:insert(Now, Ref, Tree), [{Ref, Handle1} | RefHdls]); @@ -846,10 +852,9 @@ maybe_seek(NewOffset, Handle = #handle{hdl = Hdl, true -> case prim_file_position(Hdl, NewOffset) of {ok, Offset1} = Result -> - {Result, Handle#handle{offset = Offset1, - at_eof = AtEoF1, - read_buffer = <<>>, - read_buffer_size = 0}}; + {Result, reset_read_buffer( + Handle#handle{offset = Offset1, + at_eof = AtEoF1})}; {error, _} = Error -> {Error, Handle} end; @@ -903,6 +908,10 @@ write_buffer(Handle = #handle { hdl = Hdl, offset = Offset, {Error, Handle} end. +reset_read_buffer(Handle) -> + Handle#handle{read_buffer = <<>>, + read_buffer_size = 0}. + infos(Items, State) -> [{Item, i(Item, State)} || Item <- Items]. i(total_limit, #fhc_state{limit = Limit}) -> Limit; -- cgit v1.2.1 From 246e224e0fd4d49de6cccb9f76f365a729da0c61 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 11:10:14 +0000 Subject: Small refactor suggested by Matthias. --- src/file_handle_cache.erl | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index fa896c67..06a72aa4 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -845,27 +845,21 @@ maybe_seek(NewOffset, Handle = #handle{hdl = Hdl, at_eof = AtEoF}) -> {AtEoF1, NeedsSeek} = needs_seek(AtEoF, Offset, NewOffset), case NeedsSeek of + true when is_number(NewOffset) andalso + NewOffset >= Offset andalso NewOffset =< BufSz + Offset -> + Diff = NewOffset - Offset, + <<_:Diff/binary, Rest/binary>> = Buf, + {{ok, NewOffset}, Handle#handle{offset = NewOffset, + at_eof = AtEoF1, + read_buffer = Rest, + read_buffer_size = BufSz - Diff}}; true -> - case not is_number(NewOffset) orelse - NewOffset < Offset orelse - NewOffset > BufSz + Offset of - true -> - case prim_file_position(Hdl, NewOffset) of - {ok, Offset1} = Result -> - {Result, reset_read_buffer( - Handle#handle{offset = Offset1, - at_eof = AtEoF1})}; - {error, _} = Error -> - {Error, Handle} - end; - false -> - Diff = NewOffset - Offset, - <<_:Diff/binary, Rest/binary>> = Buf, - {{ok, NewOffset}, - Handle#handle{offset = NewOffset, - at_eof = AtEoF1, - read_buffer = Rest, - read_buffer_size = BufSz - Diff}} + case prim_file_position(Hdl, NewOffset) of + {ok, Offset1} = Result -> + {Result, reset_read_buffer(Handle#handle{offset = Offset1, + at_eof = AtEoF1})}; + {error, _} = Error -> + {Error, Handle} end; false -> {{ok, Offset}, Handle} -- cgit v1.2.1 From ae2b78b00a2c6844dfe0e0d68136ba104de359b2 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 11:57:32 +0000 Subject: Remove a couple of TODOs, make the read buffer size configurable, and don't use the read buffer for the QI or msg store transform since they already read in decent sized chunks. --- src/file_handle_cache.erl | 14 ++++++++++---- src/rabbit_msg_store.erl | 3 ++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 06a72aa4..2922e146 100644 --- a/src/file_handle_cache.erl +++ b/src/file_handle_cache.erl @@ -240,7 +240,8 @@ -spec(register_callback/3 :: (atom(), atom(), [any()]) -> 'ok'). -spec(open/3 :: (file:filename(), [any()], - [{'write_buffer', (non_neg_integer() | 'infinity' | 'unbuffered')}]) + [{'write_buffer', (non_neg_integer() | 'infinity' | 'unbuffered')} | + {'read_buffer', (non_neg_integer() | 'unbuffered')}]) -> val_or_error(ref())). -spec(close/1 :: (ref()) -> ok_or_error()). -spec(read/2 :: (ref(), non_neg_integer()) -> @@ -370,8 +371,8 @@ read(Ref, Count) -> end; eof -> {eof, [Handle #handle { at_eof = true }]}; - Error -> %% TODO correct or change handle? - {Error, [Handle]} + Error -> + {Error, [reset_read_buffer(Handle)]} end end). @@ -768,6 +769,11 @@ new_closed_handle(Path, Mode, Options) -> infinity -> infinity; N when is_integer(N) -> N end, + ReadBufferSize = + case proplists:get_value(read_buffer, Options, unbuffered) of + unbuffered -> 0; + N2 when is_integer(N2) -> N2 + end, Ref = make_ref(), put({Ref, fhc_handle}, #handle { hdl = closed, offset = 0, @@ -776,7 +782,7 @@ new_closed_handle(Path, Mode, Options) -> write_buffer_size_limit = WriteBufferSize, write_buffer = [], read_buffer_size = 0, - read_buffer_size_limit = 1000000, %% TODO + read_buffer_size_limit = ReadBufferSize, read_buffer = <<>>, at_eof = false, path = Path, diff --git a/src/rabbit_msg_store.erl b/src/rabbit_msg_store.erl index b829ae94..6c80ddcd 100644 --- a/src/rabbit_msg_store.erl +++ b/src/rabbit_msg_store.erl @@ -1299,7 +1299,8 @@ should_mask_action(CRef, MsgId, open_file(Dir, FileName, Mode) -> file_handle_cache:open(form_filename(Dir, FileName), ?BINARY_MODE ++ Mode, - [{write_buffer, ?HANDLE_CACHE_BUFFER_SIZE}]). + [{write_buffer, ?HANDLE_CACHE_BUFFER_SIZE}, + {read_buffer, ?HANDLE_CACHE_BUFFER_SIZE}]). close_handle(Key, CState = #client_msstate { file_handle_cache = FHC }) -> CState #client_msstate { file_handle_cache = close_handle(Key, FHC) }; -- cgit v1.2.1 From 5c02c826bf447c07cd6ecb7bb1070b9c5ea59b7f Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 13:01:35 +0000 Subject: Import changes from https://github.com/gotthardp/rabbitmq-server/tree/multi_authorization --- include/rabbit.hrl | 4 +- src/rabbit_access_control.erl | 95 +++++++++++++++++++++++------------- src/rabbit_auth_backend.erl | 11 +++-- src/rabbit_auth_backend_dummy.erl | 14 +++--- src/rabbit_auth_backend_internal.erl | 13 +++-- src/rabbit_channel.erl | 2 +- src/rabbit_direct.erl | 2 +- src/rabbit_reader.erl | 2 +- src/rabbit_types.erl | 8 +-- test/src/rabbit_tests.erl | 11 +++-- 10 files changed, 96 insertions(+), 66 deletions(-) diff --git a/include/rabbit.hrl b/include/rabbit.hrl index 74e165cd..627d0479 100644 --- a/include/rabbit.hrl +++ b/include/rabbit.hrl @@ -16,8 +16,8 @@ -record(user, {username, tags, - auth_backend, %% Module this user came from - impl %% Scratch space for that module + authN_backend, %% Authentication module this user came from + authZ_backends %% List of authorization modules }). -record(internal_user, {username, password_hash, tags}). diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index b0a9c0d8..dcec0ff5 100644 --- a/src/rabbit_access_control.erl +++ b/src/rabbit_access_control.erl @@ -19,7 +19,7 @@ -include("rabbit.hrl"). -export([check_user_pass_login/2, check_user_login/2, check_user_loopback/2, - check_vhost_access/2, check_resource_access/3]). + check_vhost_access/3, check_resource_access/3]). %%---------------------------------------------------------------------------- @@ -38,8 +38,8 @@ -spec(check_user_loopback/2 :: (rabbit_types:username(), rabbit_net:socket() | inet:ip_address()) -> 'ok' | 'not_allowed'). --spec(check_vhost_access/2 :: - (rabbit_types:user(), rabbit_types:vhost()) +-spec(check_vhost_access/3 :: + (rabbit_types:user(), rabbit_types:vhost(), rabbit_net:socket()) -> 'ok' | rabbit_types:channel_exit()). -spec(check_resource_access/3 :: (rabbit_types:user(), rabbit_types:r(atom()), permission_atom()) @@ -58,33 +58,52 @@ check_user_login(Username, AuthProps) -> fun ({ModN, ModZ}, {refused, _, _}) -> %% Different modules for authN vs authZ. So authenticate %% with authN module, then if that succeeds do - %% passwordless (i.e pre-authenticated) login with authZ - %% module, and use the #user{} the latter gives us. - case try_login(ModN, Username, AuthProps) of - {ok, _} -> try_login(ModZ, Username, []); + %% passwordless (i.e pre-authenticated) login with authZ. + case try_authenticate(ModN, Username, AuthProps) of + {ok, User, _AuthZ} -> try_authorize(ModZ, User, []); Else -> Else end; (Mod, {refused, _, _}) -> %% Same module for authN and authZ. Just take the result %% it gives us - try_login(Mod, Username, AuthProps); - (_, {ok, User}) -> + try_authenticate(Mod, Username, AuthProps); + (_, {ok, User, AuthZ}) -> %% We've successfully authenticated. Skip to the end... - {ok, User} + {ok, User, AuthZ} end, {refused, "No modules checked '~s'", [Username]}, Modules), - rabbit_event:notify(case R of - {ok, _User} -> user_authentication_success; - _ -> user_authentication_failure - end, [{name, Username}]), - R. -try_login(Module, Username, AuthProps) -> + case R of + {ok, RUser, RAuthZ} -> + rabbit_event:notify(user_authentication_success, [{name, Username}]), + %% Store the list of authorization backends + {ok, RUser#user{authZ_backends=RAuthZ}}; + _ -> + rabbit_event:notify(user_authentication_failure, [{name, Username}]), + R + end. + +try_authenticate(Module, Username, AuthProps) -> case Module:check_user_login(Username, AuthProps) of + {ok, User, AuthZ} -> {ok, User, [{Module, AuthZ}]}; {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", [Module, Username, E]}; Else -> Else end. +try_authorize(Modules, User, AuthZList) when is_list(Modules) -> + lists:foldr( + fun (Module, {ok, _User, AuthZList2}) -> try_authorize(Module, User, AuthZList2); + (_, {refused, _, _} = Error) -> Error + end, {ok, User, AuthZList}, Modules); + +try_authorize(Module, User = #user{username = Username}, AuthZList) -> + case Module:check_user_login(Username, []) of + {ok, _User, AuthZ} -> {ok, User, [{Module, AuthZ}|AuthZList]}; + {error, E} -> {refused, "~s failed authorizing ~s: ~p~n", + [Module, Username, E]}; + Else -> Else + end. + check_user_loopback(Username, SockOrAddr) -> {ok, Users} = application:get_env(rabbit, loopback_users), case rabbit_net:is_loopback(SockOrAddr) @@ -93,29 +112,39 @@ check_user_loopback(Username, SockOrAddr) -> false -> not_allowed end. -check_vhost_access(User = #user{ username = Username, - auth_backend = Module }, VHostPath) -> - check_access( - fun() -> - %% TODO this could be an andalso shortcut under >R13A - case rabbit_vhost:exists(VHostPath) of - false -> false; - true -> Module:check_vhost_access(User, VHostPath) - end - end, - Module, "access to vhost '~s' refused for user '~s'", - [VHostPath, Username]). +check_vhost_access(User = #user{ username = Username, + authZ_backends = Modules }, VHostPath, Sock) -> + lists:foldl( + fun({Module, Impl}, ok) -> + check_access( + fun() -> + %% TODO this could be an andalso shortcut under >R13A + case rabbit_vhost:exists(VHostPath) of + false -> false; + true -> Module:check_vhost_access(User, Impl, VHostPath, Sock) + end + end, + Module, "access to vhost '~s' refused for user '~s'", + [VHostPath, Username]); + + (_, Else) -> Else + end, ok, Modules). check_resource_access(User, R = #resource{kind = exchange, name = <<"">>}, Permission) -> check_resource_access(User, R#resource{name = <<"amq.default">>}, Permission); -check_resource_access(User = #user{username = Username, auth_backend = Module}, +check_resource_access(User = #user{username = Username, authZ_backends = Modules}, Resource, Permission) -> - check_access( - fun() -> Module:check_resource_access(User, Resource, Permission) end, - Module, "access to ~s refused for user '~s'", - [rabbit_misc:rs(Resource), Username]). + lists:foldl( + fun({Module, Impl}, ok) -> + check_access( + fun() -> Module:check_resource_access(User, Impl, Resource, Permission) end, + Module, "access to ~s refused for user '~s'", + [rabbit_misc:rs(Resource), Username]); + + (_, Else) -> Else + end, ok, Modules). check_access(Fun, Module, ErrStr, ErrArgs) -> Allow = case Fun() of diff --git a/src/rabbit_auth_backend.erl b/src/rabbit_auth_backend.erl index a7dd6494..99f291f1 100644 --- a/src/rabbit_auth_backend.erl +++ b/src/rabbit_auth_backend.erl @@ -33,7 +33,7 @@ %% {refused, Msg, Args} %% Client failed authentication. Log and die. -callback check_user_login(rabbit_types:username(), [term()]) -> - {'ok', rabbit_types:user()} | + {'ok', rabbit_types:user(), any()} | {'refused', string(), [any()]} | {'error', any()}. @@ -43,7 +43,8 @@ %% false %% {error, Error} %% Something went wrong. Log and die. --callback check_vhost_access(rabbit_types:user(), rabbit_types:vhost()) -> +-callback check_vhost_access(rabbit_types:user(), any(), + rabbit_types:vhost(), rabbit_net:socket()) -> boolean() | {'error', any()}. @@ -54,7 +55,7 @@ %% false %% {error, Error} %% Something went wrong. Log and die. --callback check_resource_access(rabbit_types:user(), +-callback check_resource_access(rabbit_types:user(), any(), rabbit_types:r(atom()), rabbit_access_control:permission_atom()) -> boolean() | {'error', any()}. @@ -64,8 +65,8 @@ -export([behaviour_info/1]). behaviour_info(callbacks) -> - [{description, 0}, {check_user_login, 2}, {check_vhost_access, 2}, - {check_resource_access, 3}]; + [{description, 0}, {check_user_login, 2}, {check_vhost_access, 4}, + {check_resource_access, 4}]; behaviour_info(_Other) -> undefined. diff --git a/src/rabbit_auth_backend_dummy.erl b/src/rabbit_auth_backend_dummy.erl index 5daca368..8e2d8269 100644 --- a/src/rabbit_auth_backend_dummy.erl +++ b/src/rabbit_auth_backend_dummy.erl @@ -21,7 +21,7 @@ -export([description/0]). -export([user/0]). --export([check_user_login/2, check_vhost_access/2, check_resource_access/3]). +-export([check_user_login/2, check_vhost_access/4, check_resource_access/4]). -ifdef(use_specs). @@ -31,10 +31,10 @@ %% A user to be used by the direct client when permission checks are %% not needed. This user can do anything AMQPish. -user() -> #user{username = <<"none">>, - tags = [], - auth_backend = ?MODULE, - impl = none}. +user() -> #user{username = <<"none">>, + tags = [], + authN_backend = ?MODULE, + authZ_backends = []}. %% Implementation of rabbit_auth_backend @@ -45,5 +45,5 @@ description() -> check_user_login(_, _) -> {refused, "cannot log in conventionally as dummy user", []}. -check_vhost_access(#user{}, _VHostPath) -> true. -check_resource_access(#user{}, #resource{}, _Permission) -> true. +check_vhost_access(#user{}, _Impl, _VHostPath, _Sock) -> true. +check_resource_access(#user{}, _Impl, #resource{}, _Permission) -> true. diff --git a/src/rabbit_auth_backend_internal.erl b/src/rabbit_auth_backend_internal.erl index fd1c4e8e..5cdff985 100644 --- a/src/rabbit_auth_backend_internal.erl +++ b/src/rabbit_auth_backend_internal.erl @@ -20,7 +20,7 @@ -behaviour(rabbit_auth_backend). -export([description/0]). --export([check_user_login/2, check_vhost_access/2, check_resource_access/3]). +-export([check_user_login/2, check_vhost_access/4, check_resource_access/4]). -export([add_user/2, delete_user/1, lookup_user/1, change_password/2, clear_password/1, @@ -98,17 +98,16 @@ internal_check_user_login(Username, Fun) -> case lookup_user(Username) of {ok, User = #internal_user{tags = Tags}} -> case Fun(User) of - true -> {ok, #user{username = Username, - tags = Tags, - auth_backend = ?MODULE, - impl = User}}; + true -> {ok, #user{username = Username, + tags = Tags, + authN_backend = ?MODULE}, User}; _ -> Refused end; {error, not_found} -> Refused end. -check_vhost_access(#user{username = Username}, VHostPath) -> +check_vhost_access(#user{username = Username}, _Impl, VHostPath, _Sock) -> case mnesia:dirty_read({rabbit_user_permission, #user_vhost{username = Username, virtual_host = VHostPath}}) of @@ -116,7 +115,7 @@ check_vhost_access(#user{username = Username}, VHostPath) -> [_R] -> true end. -check_resource_access(#user{username = Username}, +check_resource_access(#user{username = Username}, _Impl, #resource{virtual_host = VHostPath, name = Name}, Permission) -> case mnesia:dirty_read({rabbit_user_permission, diff --git a/src/rabbit_channel.erl b/src/rabbit_channel.erl index 8632e1b3..2f7e234d 100644 --- a/src/rabbit_channel.erl +++ b/src/rabbit_channel.erl @@ -581,7 +581,7 @@ check_user_id_header(#'P_basic'{user_id = Username}, #ch{user = #user{username = Username}}) -> ok; check_user_id_header( - #'P_basic'{}, #ch{user = #user{auth_backend = rabbit_auth_backend_dummy}}) -> + #'P_basic'{}, #ch{user = #user{authN_backend = rabbit_auth_backend_dummy}}) -> ok; check_user_id_header(#'P_basic'{user_id = Claimed}, #ch{user = #user{username = Actual, diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 749a67b1..f6140f09 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -92,7 +92,7 @@ connect0(AuthFun, VHost, Protocol, Pid, Infos) -> end. connect1(User, VHost, Protocol, Pid, Infos) -> - try rabbit_access_control:check_vhost_access(User, VHost) of + try rabbit_access_control:check_vhost_access(User, VHost, undefined) of ok -> ok = pg_local:join(rabbit_direct, Pid), rabbit_event:notify(connection_created, Infos), {ok, {User, rabbit_reader:server_properties(Protocol)}} diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index ca73006a..2033dd14 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -944,7 +944,7 @@ handle_method0(#'connection.open'{virtual_host = VHostPath}, helper_sup = SupPid, sock = Sock, throttle = Throttle}) -> - ok = rabbit_access_control:check_vhost_access(User, VHostPath), + ok = rabbit_access_control:check_vhost_access(User, VHostPath, Sock), NewConnection = Connection#connection{vhost = VHostPath}, ok = send_on_channel0(Sock, #'connection.open_ok'{}, Protocol), Conserve = rabbit_alarm:register(self(), {?MODULE, conserve_resources, []}), diff --git a/src/rabbit_types.erl b/src/rabbit_types.erl index ba48867a..b3158cc8 100644 --- a/src/rabbit_types.erl +++ b/src/rabbit_types.erl @@ -132,10 +132,10 @@ -type(protocol() :: rabbit_framing:protocol()). -type(user() :: - #user{username :: username(), - tags :: [atom()], - auth_backend :: atom(), - impl :: any()}). + #user{username :: username(), + tags :: [atom()], + authN_backend :: atom(), + authZ_backends :: [{atom(), any()}]}). -type(internal_user() :: #internal_user{username :: username(), diff --git a/test/src/rabbit_tests.erl b/test/src/rabbit_tests.erl index ef6b756b..a227f3d2 100644 --- a/test/src/rabbit_tests.erl +++ b/test/src/rabbit_tests.erl @@ -1292,11 +1292,12 @@ test_spawn_remote() -> end. user(Username) -> - #user{username = Username, - tags = [administrator], - auth_backend = rabbit_auth_backend_internal, - impl = #internal_user{username = Username, - tags = [administrator]}}. + #user{username = Username, + tags = [administrator], + authN_backend = rabbit_auth_backend_internal, + authZ_backends = [{rabbit_auth_backend_internal, + #internal_user{username = Username, + tags = [administrator]}}]}. test_confirms() -> {_Writer, Ch} = test_spawn(), -- cgit v1.2.1 From 592c9b0b2dd726a8dc7879f969793eef430eaf5b Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 14:36:48 +0000 Subject: Cosmetic. --- src/rabbit_access_control.erl | 61 +++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index dcec0ff5..a6d3cd6c 100644 --- a/src/rabbit_access_control.erl +++ b/src/rabbit_access_control.erl @@ -61,7 +61,7 @@ check_user_login(Username, AuthProps) -> %% passwordless (i.e pre-authenticated) login with authZ. case try_authenticate(ModN, Username, AuthProps) of {ok, User, _AuthZ} -> try_authorize(ModZ, User, []); - Else -> Else + Else -> Else end; (Mod, {refused, _, _}) -> %% Same module for authN and authZ. Just take the result @@ -71,30 +71,31 @@ check_user_login(Username, AuthProps) -> %% We've successfully authenticated. Skip to the end... {ok, User, AuthZ} end, {refused, "No modules checked '~s'", [Username]}, Modules), - case R of {ok, RUser, RAuthZ} -> - rabbit_event:notify(user_authentication_success, [{name, Username}]), + rabbit_event:notify(user_authentication_success, [{name,Username}]), %% Store the list of authorization backends {ok, RUser#user{authZ_backends=RAuthZ}}; _ -> - rabbit_event:notify(user_authentication_failure, [{name, Username}]), + rabbit_event:notify(user_authentication_failure, [{name,Username}]), R end. try_authenticate(Module, Username, AuthProps) -> case Module:check_user_login(Username, AuthProps) of {ok, User, AuthZ} -> {ok, User, [{Module, AuthZ}]}; - {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", - [Module, Username, E]}; - Else -> Else + {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", + [Module, Username, E]}; + {refused, F, A} -> {refused, F, A} end. try_authorize(Modules, User, AuthZList) when is_list(Modules) -> lists:foldr( - fun (Module, {ok, _User, AuthZList2}) -> try_authorize(Module, User, AuthZList2); - (_, {refused, _, _} = Error) -> Error - end, {ok, User, AuthZList}, Modules); + fun (Module, {ok, _User, AuthZList2}) -> + try_authorize(Module, User, AuthZList2); + (_, {refused, _, _} = Error) -> + Error + end, {ok, User, AuthZList}, Modules); try_authorize(Module, User = #user{username = Username}, AuthZList) -> case Module:check_user_login(Username, []) of @@ -112,37 +113,35 @@ check_user_loopback(Username, SockOrAddr) -> false -> not_allowed end. -check_vhost_access(User = #user{ username = Username, - authZ_backends = Modules }, VHostPath, Sock) -> +check_vhost_access(User = #user{username = Username, + authZ_backends = Modules}, VHostPath, Sock) -> lists:foldl( - fun({Module, Impl}, ok) -> - check_access( - fun() -> - %% TODO this could be an andalso shortcut under >R13A - case rabbit_vhost:exists(VHostPath) of - false -> false; - true -> Module:check_vhost_access(User, Impl, VHostPath, Sock) - end - end, - Module, "access to vhost '~s' refused for user '~s'", - [VHostPath, Username]); - - (_, Else) -> Else + fun({Mod, Impl}, ok) -> + check_access( + fun() -> + rabbit_vhost:exists(VHostPath) andalso + Mod:check_vhost_access(User, Impl, VHostPath, Sock) + end, + Mod, "access to vhost '~s' refused for user '~s'", + [VHostPath, Username]); + (_, Else) -> + Else end, ok, Modules). check_resource_access(User, R = #resource{kind = exchange, name = <<"">>}, Permission) -> check_resource_access(User, R#resource{name = <<"amq.default">>}, Permission); -check_resource_access(User = #user{username = Username, authZ_backends = Modules}, +check_resource_access(User = #user{username = Username, + authZ_backends = Modules}, Resource, Permission) -> lists:foldl( fun({Module, Impl}, ok) -> - check_access( - fun() -> Module:check_resource_access(User, Impl, Resource, Permission) end, - Module, "access to ~s refused for user '~s'", - [rabbit_misc:rs(Resource), Username]); - + check_access( + fun() -> Module:check_resource_access( + User, Impl, Resource, Permission) end, + Module, "access to ~s refused for user '~s'", + [rabbit_misc:rs(Resource), Username]); (_, Else) -> Else end, ok, Modules). -- cgit v1.2.1 From 606e3656008e1ca677e0cde9c3514e24f9ab1468 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 13 Nov 2014 16:18:20 +0000 Subject: Never pass the #user{} record into auth backends, they should only see their own stuff. Get rid of authN_backend, it has little reason for existing. Flatten case of authZ_backend. --- include/rabbit.hrl | 11 +++-- src/rabbit_access_control.erl | 87 ++++++++++++++++++++---------------- src/rabbit_auth_backend.erl | 20 ++++++--- src/rabbit_auth_backend_dummy.erl | 13 +++--- src/rabbit_auth_backend_internal.erl | 12 ++--- src/rabbit_channel.erl | 3 +- src/rabbit_types.erl | 3 +- 7 files changed, 87 insertions(+), 62 deletions(-) diff --git a/include/rabbit.hrl b/include/rabbit.hrl index 627d0479..86c30fc5 100644 --- a/include/rabbit.hrl +++ b/include/rabbit.hrl @@ -14,12 +14,17 @@ %% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. %% +%% Passed around most places -record(user, {username, tags, - authN_backend, %% Authentication module this user came from - authZ_backends %% List of authorization modules - }). + authz_backends}). %% List of {Module, AuthUser} pairs +%% Passed to auth backends +-record(auth_user, {username, + tags, + impl}). + +%% Implementation for the internal auth backend -record(internal_user, {username, password_hash, tags}). -record(permission, {configure, write, read}). -record(user_vhost, {username, virtual_host}). diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index a6d3cd6c..0ebd2fcf 100644 --- a/src/rabbit_access_control.erl +++ b/src/rabbit_access_control.erl @@ -55,55 +55,64 @@ check_user_pass_login(Username, Password) -> check_user_login(Username, AuthProps) -> {ok, Modules} = application:get_env(rabbit, auth_backends), R = lists:foldl( - fun ({ModN, ModZ}, {refused, _, _}) -> + fun ({ModN, ModZs0}, {refused, _, _}) -> + ModZs = case ModZs0 of + A when is_atom(A) -> [A]; + L when is_list(L) -> L + end, %% Different modules for authN vs authZ. So authenticate %% with authN module, then if that succeeds do %% passwordless (i.e pre-authenticated) login with authZ. case try_authenticate(ModN, Username, AuthProps) of - {ok, User, _AuthZ} -> try_authorize(ModZ, User, []); - Else -> Else + {ok, ModNUser = #auth_user{username = Username2}} -> + user(ModNUser, try_authorize(ModZs, Username2)); + Else -> + Else end; (Mod, {refused, _, _}) -> %% Same module for authN and authZ. Just take the result %% it gives us - try_authenticate(Mod, Username, AuthProps); - (_, {ok, User, AuthZ}) -> + case try_authenticate(Mod, Username, AuthProps) of + {ok, ModNUser} -> user(ModNUser, {ok, [{Mod, ModNUser}]}); + Else -> Else + end; + (_, {ok, User}) -> %% We've successfully authenticated. Skip to the end... - {ok, User, AuthZ} + {ok, User} end, {refused, "No modules checked '~s'", [Username]}, Modules), - case R of - {ok, RUser, RAuthZ} -> - rabbit_event:notify(user_authentication_success, [{name,Username}]), - %% Store the list of authorization backends - {ok, RUser#user{authZ_backends=RAuthZ}}; - _ -> - rabbit_event:notify(user_authentication_failure, [{name,Username}]), - R - end. + rabbit_event:notify(case R of + {ok, _User} -> user_authentication_success; + _ -> user_authentication_failure + end, [{name, Username}]), + R. try_authenticate(Module, Username, AuthProps) -> case Module:check_user_login(Username, AuthProps) of - {ok, User, AuthZ} -> {ok, User, [{Module, AuthZ}]}; - {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", - [Module, Username, E]}; - {refused, F, A} -> {refused, F, A} + {ok, AuthUser} -> {ok, AuthUser}; + {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", + [Module, Username, E]}; + {refused, F, A} -> {refused, F, A} end. -try_authorize(Modules, User, AuthZList) when is_list(Modules) -> +try_authorize(Modules, Username) -> lists:foldr( - fun (Module, {ok, _User, AuthZList2}) -> - try_authorize(Module, User, AuthZList2); + fun (Module, {ok, AUsers}) -> + case Module:check_user_login(Username, []) of + {ok, AUser} -> {ok, [{Module, AUser} | AUsers]}; + {error, E} -> {refused, "~s failed authorizing ~s: ~p~n", + [Module, Username, E]}; + {refused, F, A} -> {refused, F, A} + end; (_, {refused, _, _} = Error) -> Error - end, {ok, User, AuthZList}, Modules); - -try_authorize(Module, User = #user{username = Username}, AuthZList) -> - case Module:check_user_login(Username, []) of - {ok, _User, AuthZ} -> {ok, User, [{Module, AuthZ}|AuthZList]}; - {error, E} -> {refused, "~s failed authorizing ~s: ~p~n", - [Module, Username, E]}; - Else -> Else - end. + end, {ok, []}, Modules). + +user(#auth_user{username = Username, tags = Tags}, {ok, ModZUsers}) -> + {ok, #user{username = Username, + tags = Tags, + authz_backends = ModZUsers}}; +user(_AuthUser, Error) -> + Error. check_user_loopback(Username, SockOrAddr) -> {ok, Users} = application:get_env(rabbit, loopback_users), @@ -113,14 +122,14 @@ check_user_loopback(Username, SockOrAddr) -> false -> not_allowed end. -check_vhost_access(User = #user{username = Username, - authZ_backends = Modules}, VHostPath, Sock) -> +check_vhost_access(#user{username = Username, + authz_backends = Modules}, VHostPath, Sock) -> lists:foldl( - fun({Mod, Impl}, ok) -> + fun({Mod, AUser}, ok) -> check_access( fun() -> rabbit_vhost:exists(VHostPath) andalso - Mod:check_vhost_access(User, Impl, VHostPath, Sock) + Mod:check_vhost_access(AUser, VHostPath, Sock) end, Mod, "access to vhost '~s' refused for user '~s'", [VHostPath, Username]); @@ -132,14 +141,14 @@ check_resource_access(User, R = #resource{kind = exchange, name = <<"">>}, Permission) -> check_resource_access(User, R#resource{name = <<"amq.default">>}, Permission); -check_resource_access(User = #user{username = Username, - authZ_backends = Modules}, +check_resource_access(#user{username = Username, + authz_backends = Modules}, Resource, Permission) -> lists:foldl( - fun({Module, Impl}, ok) -> + fun({Module, AUser}, ok) -> check_access( fun() -> Module:check_resource_access( - User, Impl, Resource, Permission) end, + AUser, Resource, Permission) end, Module, "access to ~s refused for user '~s'", [rabbit_misc:rs(Resource), Username]); (_, Else) -> Else diff --git a/src/rabbit_auth_backend.erl b/src/rabbit_auth_backend.erl index 99f291f1..315d5719 100644 --- a/src/rabbit_auth_backend.erl +++ b/src/rabbit_auth_backend.erl @@ -16,8 +16,17 @@ -module(rabbit_auth_backend). +-include("rabbit.hrl"). + -ifdef(use_specs). +-export_type([auth_user/0]). + +-type(auth_user() :: + #auth_user{username :: rabbit_types:username(), + tags :: [atom()], + impl :: any()}). + %% A description proplist as with auth mechanisms, %% exchanges. Currently unused. -callback description() -> [proplists:property()]. @@ -33,7 +42,7 @@ %% {refused, Msg, Args} %% Client failed authentication. Log and die. -callback check_user_login(rabbit_types:username(), [term()]) -> - {'ok', rabbit_types:user(), any()} | + {'ok', auth_user()} | {'refused', string(), [any()]} | {'error', any()}. @@ -43,11 +52,10 @@ %% false %% {error, Error} %% Something went wrong. Log and die. --callback check_vhost_access(rabbit_types:user(), any(), +-callback check_vhost_access(auth_user(), rabbit_types:vhost(), rabbit_net:socket()) -> boolean() | {'error', any()}. - %% Given #user, resource and permission, can a user access a resource? %% %% Possible responses: @@ -55,7 +63,7 @@ %% false %% {error, Error} %% Something went wrong. Log and die. --callback check_resource_access(rabbit_types:user(), any(), +-callback check_resource_access(auth_user(), rabbit_types:r(atom()), rabbit_access_control:permission_atom()) -> boolean() | {'error', any()}. @@ -65,8 +73,8 @@ -export([behaviour_info/1]). behaviour_info(callbacks) -> - [{description, 0}, {check_user_login, 2}, {check_vhost_access, 4}, - {check_resource_access, 4}]; + [{description, 0}, {check_user_login, 2}, {check_vhost_access, 3}, + {check_resource_access, 3}]; behaviour_info(_Other) -> undefined. diff --git a/src/rabbit_auth_backend_dummy.erl b/src/rabbit_auth_backend_dummy.erl index 8e2d8269..e279e4bf 100644 --- a/src/rabbit_auth_backend_dummy.erl +++ b/src/rabbit_auth_backend_dummy.erl @@ -21,7 +21,7 @@ -export([description/0]). -export([user/0]). --export([check_user_login/2, check_vhost_access/4, check_resource_access/4]). +-export([check_user_login/2, check_vhost_access/3, check_resource_access/3]). -ifdef(use_specs). @@ -33,8 +33,11 @@ %% not needed. This user can do anything AMQPish. user() -> #user{username = <<"none">>, tags = [], - authN_backend = ?MODULE, - authZ_backends = []}. + authz_backends = [{?MODULE, buser()}]}. + +buser() -> #auth_user{username = <<"none">>, + tags = [], + impl = none}. %% Implementation of rabbit_auth_backend @@ -45,5 +48,5 @@ description() -> check_user_login(_, _) -> {refused, "cannot log in conventionally as dummy user", []}. -check_vhost_access(#user{}, _Impl, _VHostPath, _Sock) -> true. -check_resource_access(#user{}, _Impl, #resource{}, _Permission) -> true. +check_vhost_access(#auth_user{}, _VHostPath, _Sock) -> true. +check_resource_access(#auth_user{}, #resource{}, _Permission) -> true. diff --git a/src/rabbit_auth_backend_internal.erl b/src/rabbit_auth_backend_internal.erl index 5cdff985..c8f09be9 100644 --- a/src/rabbit_auth_backend_internal.erl +++ b/src/rabbit_auth_backend_internal.erl @@ -20,7 +20,7 @@ -behaviour(rabbit_auth_backend). -export([description/0]). --export([check_user_login/2, check_vhost_access/4, check_resource_access/4]). +-export([check_user_login/2, check_vhost_access/3, check_resource_access/3]). -export([add_user/2, delete_user/1, lookup_user/1, change_password/2, clear_password/1, @@ -98,16 +98,16 @@ internal_check_user_login(Username, Fun) -> case lookup_user(Username) of {ok, User = #internal_user{tags = Tags}} -> case Fun(User) of - true -> {ok, #user{username = Username, - tags = Tags, - authN_backend = ?MODULE}, User}; + true -> {ok, #auth_user{username = Username, + tags = Tags, + impl = none}}; _ -> Refused end; {error, not_found} -> Refused end. -check_vhost_access(#user{username = Username}, _Impl, VHostPath, _Sock) -> +check_vhost_access(#auth_user{username = Username}, VHostPath, _Sock) -> case mnesia:dirty_read({rabbit_user_permission, #user_vhost{username = Username, virtual_host = VHostPath}}) of @@ -115,7 +115,7 @@ check_vhost_access(#user{username = Username}, _Impl, VHostPath, _Sock) -> [_R] -> true end. -check_resource_access(#user{username = Username}, _Impl, +check_resource_access(#auth_user{username = Username}, #resource{virtual_host = VHostPath, name = Name}, Permission) -> case mnesia:dirty_read({rabbit_user_permission, diff --git a/src/rabbit_channel.erl b/src/rabbit_channel.erl index 2f7e234d..13cc925c 100644 --- a/src/rabbit_channel.erl +++ b/src/rabbit_channel.erl @@ -581,7 +581,8 @@ check_user_id_header(#'P_basic'{user_id = Username}, #ch{user = #user{username = Username}}) -> ok; check_user_id_header( - #'P_basic'{}, #ch{user = #user{authN_backend = rabbit_auth_backend_dummy}}) -> + #'P_basic'{}, #ch{user = #user{authz_backends = + [{rabbit_auth_backend_dummy, _}]}}) -> ok; check_user_id_header(#'P_basic'{user_id = Claimed}, #ch{user = #user{username = Actual, diff --git a/src/rabbit_types.erl b/src/rabbit_types.erl index b3158cc8..27fbae88 100644 --- a/src/rabbit_types.erl +++ b/src/rabbit_types.erl @@ -134,8 +134,7 @@ -type(user() :: #user{username :: username(), tags :: [atom()], - authN_backend :: atom(), - authZ_backends :: [{atom(), any()}]}). + authz_backends :: [{atom(), any()}]}). -type(internal_user() :: #internal_user{username :: username(), -- cgit v1.2.1 From aa1b10af8fcf7b95596f8db530ed3cd103afbe0a Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 14 Nov 2014 11:16:17 +0000 Subject: Fix tests. --- test/src/rabbit_tests.erl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/src/rabbit_tests.erl b/test/src/rabbit_tests.erl index a227f3d2..e614bfd7 100644 --- a/test/src/rabbit_tests.erl +++ b/test/src/rabbit_tests.erl @@ -1294,10 +1294,12 @@ test_spawn_remote() -> user(Username) -> #user{username = Username, tags = [administrator], - authN_backend = rabbit_auth_backend_internal, - authZ_backends = [{rabbit_auth_backend_internal, - #internal_user{username = Username, - tags = [administrator]}}]}. + authz_backends = [{rabbit_auth_backend_internal, auser(Username)}]}. + +auser(Username) -> + #auth_user{username = Username, + tags = [administrator], + impl = none}. test_confirms() -> {_Writer, Ch} = test_spawn(), -- cgit v1.2.1 From 627f4c0e15db86bdff439ccc01bcc81bb087a79f Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 14 Nov 2014 11:17:19 +0000 Subject: Rename. --- src/rabbit_auth_backend_dummy.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rabbit_auth_backend_dummy.erl b/src/rabbit_auth_backend_dummy.erl index e279e4bf..d2905334 100644 --- a/src/rabbit_auth_backend_dummy.erl +++ b/src/rabbit_auth_backend_dummy.erl @@ -33,9 +33,9 @@ %% not needed. This user can do anything AMQPish. user() -> #user{username = <<"none">>, tags = [], - authz_backends = [{?MODULE, buser()}]}. + authz_backends = [{?MODULE, auser()}]}. -buser() -> #auth_user{username = <<"none">>, +auser() -> #auth_user{username = <<"none">>, tags = [], impl = none}. -- cgit v1.2.1 From 8a6ad3517e031b8b7b85c63ab24d062d1d647b5b Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 14 Nov 2014 14:16:39 +0000 Subject: Tweak the APIs again, so that authz plugins aren't expected to create a #auth_user record in the first place, just whatever impl they want. Which necessitates seperate login functions for authz and authn, and if we're going to do that we might as well split the behaviours so that we have the possibility of making an authz-only plugin. --- include/rabbit.hrl | 2 +- src/rabbit_access_control.erl | 40 +++++++++++------- src/rabbit_auth_backend.erl | 81 ------------------------------------ src/rabbit_auth_backend_dummy.erl | 20 ++++----- src/rabbit_auth_backend_internal.erl | 23 +++++----- src/rabbit_authn_backend.erl | 49 ++++++++++++++++++++++ src/rabbit_authz_backend.erl | 74 ++++++++++++++++++++++++++++++++ src/rabbit_types.erl | 7 +++- test/src/rabbit_tests.erl | 7 +--- 9 files changed, 176 insertions(+), 127 deletions(-) delete mode 100644 src/rabbit_auth_backend.erl create mode 100644 src/rabbit_authn_backend.erl create mode 100644 src/rabbit_authz_backend.erl diff --git a/include/rabbit.hrl b/include/rabbit.hrl index 86c30fc5..9cbd978e 100644 --- a/include/rabbit.hrl +++ b/include/rabbit.hrl @@ -17,7 +17,7 @@ %% Passed around most places -record(user, {username, tags, - authz_backends}). %% List of {Module, AuthUser} pairs + authz_backends}). %% List of {Module, AuthUserImpl} pairs %% Passed to auth backends -record(auth_user, {username, diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index 0ebd2fcf..d1577432 100644 --- a/src/rabbit_access_control.erl +++ b/src/rabbit_access_control.erl @@ -73,8 +73,10 @@ check_user_login(Username, AuthProps) -> %% Same module for authN and authZ. Just take the result %% it gives us case try_authenticate(Mod, Username, AuthProps) of - {ok, ModNUser} -> user(ModNUser, {ok, [{Mod, ModNUser}]}); - Else -> Else + {ok, ModNUser = #auth_user{impl = Impl}} -> + user(ModNUser, {ok, [{Mod, Impl}]}); + Else -> + Else end; (_, {ok, User}) -> %% We've successfully authenticated. Skip to the end... @@ -87,7 +89,7 @@ check_user_login(Username, AuthProps) -> R. try_authenticate(Module, Username, AuthProps) -> - case Module:check_user_login(Username, AuthProps) of + case Module:user_login_authentication(Username, AuthProps) of {ok, AuthUser} -> {ok, AuthUser}; {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", [Module, Username, E]}; @@ -96,9 +98,9 @@ try_authenticate(Module, Username, AuthProps) -> try_authorize(Modules, Username) -> lists:foldr( - fun (Module, {ok, AUsers}) -> - case Module:check_user_login(Username, []) of - {ok, AUser} -> {ok, [{Module, AUser} | AUsers]}; + fun (Module, {ok, ModsImpls}) -> + case Module:user_login_authorization(Username) of + {ok, Impl} -> {ok, [{Module, Impl} | ModsImpls]}; {error, E} -> {refused, "~s failed authorizing ~s: ~p~n", [Module, Username, E]}; {refused, F, A} -> {refused, F, A} @@ -107,13 +109,18 @@ try_authorize(Modules, Username) -> Error end, {ok, []}, Modules). -user(#auth_user{username = Username, tags = Tags}, {ok, ModZUsers}) -> +user(#auth_user{username = Username, tags = Tags}, {ok, ModZImpls}) -> {ok, #user{username = Username, tags = Tags, - authz_backends = ModZUsers}}; + authz_backends = ModZImpls}}; user(_AuthUser, Error) -> Error. +auth_user(#user{username = Username, tags = Tags}, Impl) -> + #auth_user{username = Username, + tags = Tags, + impl = Impl}. + check_user_loopback(Username, SockOrAddr) -> {ok, Users} = application:get_env(rabbit, loopback_users), case rabbit_net:is_loopback(SockOrAddr) @@ -122,14 +129,15 @@ check_user_loopback(Username, SockOrAddr) -> false -> not_allowed end. -check_vhost_access(#user{username = Username, - authz_backends = Modules}, VHostPath, Sock) -> +check_vhost_access(User = #user{username = Username, + authz_backends = Modules}, VHostPath, Sock) -> lists:foldl( - fun({Mod, AUser}, ok) -> + fun({Mod, Impl}, ok) -> check_access( fun() -> rabbit_vhost:exists(VHostPath) andalso - Mod:check_vhost_access(AUser, VHostPath, Sock) + Mod:check_vhost_access( + auth_user(User, Impl), VHostPath, Sock) end, Mod, "access to vhost '~s' refused for user '~s'", [VHostPath, Username]); @@ -141,14 +149,14 @@ check_resource_access(User, R = #resource{kind = exchange, name = <<"">>}, Permission) -> check_resource_access(User, R#resource{name = <<"amq.default">>}, Permission); -check_resource_access(#user{username = Username, - authz_backends = Modules}, +check_resource_access(User = #user{username = Username, + authz_backends = Modules}, Resource, Permission) -> lists:foldl( - fun({Module, AUser}, ok) -> + fun({Module, Impl}, ok) -> check_access( fun() -> Module:check_resource_access( - AUser, Resource, Permission) end, + auth_user(User, Impl), Resource, Permission) end, Module, "access to ~s refused for user '~s'", [rabbit_misc:rs(Resource), Username]); (_, Else) -> Else diff --git a/src/rabbit_auth_backend.erl b/src/rabbit_auth_backend.erl deleted file mode 100644 index 315d5719..00000000 --- a/src/rabbit_auth_backend.erl +++ /dev/null @@ -1,81 +0,0 @@ -%% The contents of this file are subject to the Mozilla Public License -%% Version 1.1 (the "License"); you may not use this file except in -%% compliance with the License. You may obtain a copy of the License -%% at http://www.mozilla.org/MPL/ -%% -%% Software distributed under the License is distributed on an "AS IS" -%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See -%% the License for the specific language governing rights and -%% limitations under the License. -%% -%% The Original Code is RabbitMQ. -%% -%% The Initial Developer of the Original Code is GoPivotal, Inc. -%% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. -%% - --module(rabbit_auth_backend). - --include("rabbit.hrl"). - --ifdef(use_specs). - --export_type([auth_user/0]). - --type(auth_user() :: - #auth_user{username :: rabbit_types:username(), - tags :: [atom()], - impl :: any()}). - -%% A description proplist as with auth mechanisms, -%% exchanges. Currently unused. --callback description() -> [proplists:property()]. - -%% Check a user can log in, given a username and a proplist of -%% authentication information (e.g. [{password, Password}]). -%% -%% Possible responses: -%% {ok, User} -%% Authentication succeeded, and here's the user record. -%% {error, Error} -%% Something went wrong. Log and die. -%% {refused, Msg, Args} -%% Client failed authentication. Log and die. --callback check_user_login(rabbit_types:username(), [term()]) -> - {'ok', auth_user()} | - {'refused', string(), [any()]} | - {'error', any()}. - -%% Given #user and vhost, can a user log in to a vhost? -%% Possible responses: -%% true -%% false -%% {error, Error} -%% Something went wrong. Log and die. --callback check_vhost_access(auth_user(), - rabbit_types:vhost(), rabbit_net:socket()) -> - boolean() | {'error', any()}. - -%% Given #user, resource and permission, can a user access a resource? -%% -%% Possible responses: -%% true -%% false -%% {error, Error} -%% Something went wrong. Log and die. --callback check_resource_access(auth_user(), - rabbit_types:r(atom()), - rabbit_access_control:permission_atom()) -> - boolean() | {'error', any()}. - --else. - --export([behaviour_info/1]). - -behaviour_info(callbacks) -> - [{description, 0}, {check_user_login, 2}, {check_vhost_access, 3}, - {check_resource_access, 3}]; -behaviour_info(_Other) -> - undefined. - --endif. diff --git a/src/rabbit_auth_backend_dummy.erl b/src/rabbit_auth_backend_dummy.erl index d2905334..d2f07c1d 100644 --- a/src/rabbit_auth_backend_dummy.erl +++ b/src/rabbit_auth_backend_dummy.erl @@ -17,11 +17,12 @@ -module(rabbit_auth_backend_dummy). -include("rabbit.hrl"). --behaviour(rabbit_auth_backend). +-behaviour(rabbit_authn_backend). +-behaviour(rabbit_authz_backend). --export([description/0]). -export([user/0]). --export([check_user_login/2, check_vhost_access/3, check_resource_access/3]). +-export([user_login_authentication/2, user_login_authorization/1, + check_vhost_access/3, check_resource_access/3]). -ifdef(use_specs). @@ -33,19 +34,14 @@ %% not needed. This user can do anything AMQPish. user() -> #user{username = <<"none">>, tags = [], - authz_backends = [{?MODULE, auser()}]}. - -auser() -> #auth_user{username = <<"none">>, - tags = [], - impl = none}. + authz_backends = [{?MODULE, none}]}. %% Implementation of rabbit_auth_backend -description() -> - [{name, <<"Dummy">>}, - {description, <<"Database for the dummy user">>}]. +user_login_authentication(_, _) -> + {refused, "cannot log in conventionally as dummy user", []}. -check_user_login(_, _) -> +user_login_authorization(_) -> {refused, "cannot log in conventionally as dummy user", []}. check_vhost_access(#auth_user{}, _VHostPath, _Sock) -> true. diff --git a/src/rabbit_auth_backend_internal.erl b/src/rabbit_auth_backend_internal.erl index c8f09be9..20a5766d 100644 --- a/src/rabbit_auth_backend_internal.erl +++ b/src/rabbit_auth_backend_internal.erl @@ -17,10 +17,11 @@ -module(rabbit_auth_backend_internal). -include("rabbit.hrl"). --behaviour(rabbit_auth_backend). +-behaviour(rabbit_authn_backend). +-behaviour(rabbit_authz_backend). --export([description/0]). --export([check_user_login/2, check_vhost_access/3, check_resource_access/3]). +-export([user_login_authentication/2, user_login_authorization/1, + check_vhost_access/3, check_resource_access/3]). -export([add_user/2, delete_user/1, lookup_user/1, change_password/2, clear_password/1, @@ -76,13 +77,9 @@ %%---------------------------------------------------------------------------- %% Implementation of rabbit_auth_backend -description() -> - [{name, <<"Internal">>}, - {description, <<"Internal user / password database">>}]. - -check_user_login(Username, []) -> +user_login_authentication(Username, []) -> internal_check_user_login(Username, fun(_) -> true end); -check_user_login(Username, [{password, Cleartext}]) -> +user_login_authentication(Username, [{password, Cleartext}]) -> internal_check_user_login( Username, fun (#internal_user{password_hash = <>}) -> @@ -90,9 +87,15 @@ check_user_login(Username, [{password, Cleartext}]) -> (#internal_user{}) -> false end); -check_user_login(Username, AuthProps) -> +user_login_authentication(Username, AuthProps) -> exit({unknown_auth_props, Username, AuthProps}). +user_login_authorization(Username) -> + case user_login_authentication(Username, []) of + {ok, #auth_user{impl = Impl}} -> {ok, Impl}; + Else -> Else + end. + internal_check_user_login(Username, Fun) -> Refused = {refused, "user '~s' - invalid credentials", [Username]}, case lookup_user(Username) of diff --git a/src/rabbit_authn_backend.erl b/src/rabbit_authn_backend.erl new file mode 100644 index 00000000..cfc3f5db --- /dev/null +++ b/src/rabbit_authn_backend.erl @@ -0,0 +1,49 @@ +%% The contents of this file are subject to the Mozilla Public License +%% Version 1.1 (the "License"); you may not use this file except in +%% compliance with the License. You may obtain a copy of the License +%% at http://www.mozilla.org/MPL/ +%% +%% Software distributed under the License is distributed on an "AS IS" +%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See +%% the License for the specific language governing rights and +%% limitations under the License. +%% +%% The Original Code is RabbitMQ. +%% +%% The Initial Developer of the Original Code is GoPivotal, Inc. +%% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. +%% + +-module(rabbit_authn_backend). + +-include("rabbit.hrl"). + +-ifdef(use_specs). + +%% Check a user can log in, given a username and a proplist of +%% authentication information (e.g. [{password, Password}]). If your +%% backend is not to be used for authentication, this should always +%% refuse access. +%% +%% Possible responses: +%% {ok, User} +%% Authentication succeeded, and here's the user record. +%% {error, Error} +%% Something went wrong. Log and die. +%% {refused, Msg, Args} +%% Client failed authentication. Log and die. +-callback user_login_authentication(rabbit_types:username(), [term()]) -> + {'ok', rabbit_types:auth_user()} | + {'refused', string(), [any()]} | + {'error', any()}. + +-else. + +-export([behaviour_info/1]). + +behaviour_info(callbacks) -> + [{user_login_authentication, 2}]; +behaviour_info(_Other) -> + undefined. + +-endif. diff --git a/src/rabbit_authz_backend.erl b/src/rabbit_authz_backend.erl new file mode 100644 index 00000000..ff5f014e --- /dev/null +++ b/src/rabbit_authz_backend.erl @@ -0,0 +1,74 @@ +%% The contents of this file are subject to the Mozilla Public License +%% Version 1.1 (the "License"); you may not use this file except in +%% compliance with the License. You may obtain a copy of the License +%% at http://www.mozilla.org/MPL/ +%% +%% Software distributed under the License is distributed on an "AS IS" +%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See +%% the License for the specific language governing rights and +%% limitations under the License. +%% +%% The Original Code is RabbitMQ. +%% +%% The Initial Developer of the Original Code is GoPivotal, Inc. +%% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. +%% + +-module(rabbit_authz_backend). + +-include("rabbit.hrl"). + +-ifdef(use_specs). + +%% Check a user can log in, when this backend is being used for +%% authorisation only. Authentication has already taken place +%% successfully, but we need to check that the user exists in this +%% backend, and initialise any impl field we will want to have passed +%% back in future calls to check_vhost_access/3 and +%% check_resource_access/3. +%% +%% Possible responses: +%% {ok, Impl} +%% User authorisation succeeded, and here's the impl field. +%% {error, Error} +%% Something went wrong. Log and die. +%% {refused, Msg, Args} +%% User authorisation failed. Log and die. +-callback user_login_authorization(rabbit_types:username()) -> + {'ok', any()} | + {'refused', string(), [any()]} | + {'error', any()}. + +%% Given #auth_user and vhost, can a user log in to a vhost? +%% Possible responses: +%% true +%% false +%% {error, Error} +%% Something went wrong. Log and die. +-callback check_vhost_access(rabbit_types:auth_user(), + rabbit_types:vhost(), rabbit_net:socket()) -> + boolean() | {'error', any()}. + +%% Given #auth_user, resource and permission, can a user access a resource? +%% +%% Possible responses: +%% true +%% false +%% {error, Error} +%% Something went wrong. Log and die. +-callback check_resource_access(rabbit_types:auth_user(), + rabbit_types:r(atom()), + rabbit_access_control:permission_atom()) -> + boolean() | {'error', any()}. + +-else. + +-export([behaviour_info/1]). + +behaviour_info(callbacks) -> + [{user_login_authorization, 1}, + {check_vhost_access, 3}, {check_resource_access, 3}]; +behaviour_info(_Other) -> + undefined. + +-endif. diff --git a/src/rabbit_types.erl b/src/rabbit_types.erl index 27fbae88..039568df 100644 --- a/src/rabbit_types.erl +++ b/src/rabbit_types.erl @@ -27,7 +27,7 @@ vhost/0, ctag/0, amqp_error/0, r/1, r2/2, r3/3, listener/0, binding/0, binding_source/0, binding_destination/0, amqqueue/0, exchange/0, - connection/0, protocol/0, user/0, internal_user/0, + connection/0, protocol/0, auth_user/0, user/0, internal_user/0, username/0, password/0, password_hash/0, ok/1, error/1, ok_or_error/1, ok_or_error2/2, ok_pid_or_error/0, channel_exit/0, connection_exit/0, mfargs/0, proc_name/0, @@ -131,6 +131,11 @@ -type(protocol() :: rabbit_framing:protocol()). +-type(auth_user() :: + #auth_user{username :: username(), + tags :: [atom()], + impl :: any()}). + -type(user() :: #user{username :: username(), tags :: [atom()], diff --git a/test/src/rabbit_tests.erl b/test/src/rabbit_tests.erl index e614bfd7..dcbec8f6 100644 --- a/test/src/rabbit_tests.erl +++ b/test/src/rabbit_tests.erl @@ -1294,12 +1294,7 @@ test_spawn_remote() -> user(Username) -> #user{username = Username, tags = [administrator], - authz_backends = [{rabbit_auth_backend_internal, auser(Username)}]}. - -auser(Username) -> - #auth_user{username = Username, - tags = [administrator], - impl = none}. + authz_backends = [{rabbit_auth_backend_internal, none}]}. test_confirms() -> {_Writer, Ch} = test_spawn(), -- cgit v1.2.1 From a416ba84b97459952c2467cf0cdea0b76b6bf241 Mon Sep 17 00:00:00 2001 From: Jean-S?bastien P?dron Date: Thu, 20 Nov 2014 09:45:37 +0100 Subject: Remove support for the legacy 'cluster_nodes' values Before this change, a list of nodes without the node type was accepted. In this case, the node type was guessed and a warning suggesting how to update the configuration was logged. Now, the node type is mandatory and the RabbitMQ server refuses to start if the node type is unspecified. --- src/rabbit_mnesia.erl | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index fa51dd70..d3cacb17 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -116,18 +116,9 @@ init_from_config() -> {TryNodes, NodeType} = case application:get_env(rabbit, cluster_nodes) of {ok, Nodes} when is_list(Nodes) -> - Config = {Nodes -- [node()], case lists:member(node(), Nodes) of - true -> disc; - false -> ram - end}, - rabbit_log:warning( - "Converting legacy 'cluster_nodes' configuration~n ~w~n" - "to~n ~w.~n~n" - "Please update the configuration to the new format " - "{Nodes, NodeType}, where Nodes contains the nodes that the " - "node will try to cluster with, and NodeType is either " - "'disc' or 'ram'~n", [Nodes, Config]), - Config; + %% The legacy syntax (a nodes list without the node + %% type) is unsupported. + e(cluster_node_type_mandatory); {ok, Config} -> Config end, @@ -865,4 +856,7 @@ error_description(removing_node_from_offline_node) -> "To remove a node remotely from an offline node, the node you are removing " "from must be a disc node and all the other nodes must be offline."; error_description(no_running_cluster_nodes) -> - "You cannot leave a cluster if no online nodes are present.". + "You cannot leave a cluster if no online nodes are present."; +error_description(cluster_node_type_mandatory) -> + "The 'cluster_nodes' configuration key must indicate the node type: " + "either {[...], disc} or {[...], ram}". -- cgit v1.2.1 From 2b28700c0ac996ac37e106c5c1b7e58ce968f391 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Fri, 21 Nov 2014 13:06:12 +0100 Subject: Detect other invalid 'cluster_nodes' values In all cases, abort startup with an explanatory message. --- src/rabbit_mnesia.erl | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index d3cacb17..8fbacdae 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -113,14 +113,29 @@ init() -> ok. init_from_config() -> + FindBadNodeNames = fun + (Name, BadNames) when is_atom(Name) -> BadNames; + (Name, BadNames) -> [Name | BadNames] + end, {TryNodes, NodeType} = case application:get_env(rabbit, cluster_nodes) of + {ok, {Nodes, Type} = Config} + when is_list(Nodes) andalso (Type == disc orelse Type == ram) -> + case lists:foldl(FindBadNodeNames, [], Nodes) of + [] -> Config; + BadNames -> e({invalid_cluster_node_names, BadNames}) + end; + {ok, {_, BadType}} when BadType /= disc andalso BadType /= ram -> + e({invalid_cluster_node_type, BadType}); {ok, Nodes} when is_list(Nodes) -> %% The legacy syntax (a nodes list without the node %% type) is unsupported. - e(cluster_node_type_mandatory); - {ok, Config} -> - Config + case lists:foldl(FindBadNodeNames, [], Nodes) of + [] -> e(cluster_node_type_mandatory); + BadNames -> e(invalid_cluster_nodes_conf) + end; + {ok, _} -> + e(invalid_cluster_nodes_conf) end, case TryNodes of [] -> init_db_and_upgrade([node()], disc, false); @@ -829,6 +844,20 @@ nodes_excl_me(Nodes) -> Nodes -- [node()]. e(Tag) -> throw({error, {Tag, error_description(Tag)}}). +error_description({invalid_cluster_node_names, BadNames}) -> + "In the 'cluster_nodes' configuration key, the following node names " + "are invalid: " ++ lists:flatten(io_lib:format("~p", [BadNames])); +error_description({invalid_cluster_node_type, BadType}) -> + "In the 'cluster_nodes' configuration key, the node type is invalid " + "(expected 'disc' or 'ram'): " ++ + lists:flatten(io_lib:format("~p", [BadType])); +error_description(cluster_node_type_mandatory) -> + "The 'cluster_nodes' configuration key must indicate the node type: " + "either {[...], disc} or {[...], ram}"; +error_description(invalid_cluster_nodes_conf) -> + "The 'cluster_nodes' configuration key is invalid, it must be of the " + "form {[Nodes], Type}, where Nodes is a list of node names and " + "Type is either 'disc' or 'ram'"; error_description(clustering_only_disc_node) -> "You cannot cluster a node if it is the only disc node in its existing " " cluster. If new nodes joined while this node was offline, use " @@ -856,7 +885,4 @@ error_description(removing_node_from_offline_node) -> "To remove a node remotely from an offline node, the node you are removing " "from must be a disc node and all the other nodes must be offline."; error_description(no_running_cluster_nodes) -> - "You cannot leave a cluster if no online nodes are present."; -error_description(cluster_node_type_mandatory) -> - "The 'cluster_nodes' configuration key must indicate the node type: " - "either {[...], disc} or {[...], ram}". + "You cannot leave a cluster if no online nodes are present.". -- cgit v1.2.1 From dbc16de3e4b67c2e90ccb8b92b396a4c2862a738 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Mon, 24 Nov 2014 17:40:23 +0100 Subject: When reporting invalid cluster node names, keep configuration order While here, fix an unused variable warning. --- src/rabbit_mnesia.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index 8fbacdae..91a8b140 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -121,7 +121,7 @@ init_from_config() -> case application:get_env(rabbit, cluster_nodes) of {ok, {Nodes, Type} = Config} when is_list(Nodes) andalso (Type == disc orelse Type == ram) -> - case lists:foldl(FindBadNodeNames, [], Nodes) of + case lists:foldr(FindBadNodeNames, [], Nodes) of [] -> Config; BadNames -> e({invalid_cluster_node_names, BadNames}) end; @@ -130,9 +130,9 @@ init_from_config() -> {ok, Nodes} when is_list(Nodes) -> %% The legacy syntax (a nodes list without the node %% type) is unsupported. - case lists:foldl(FindBadNodeNames, [], Nodes) of - [] -> e(cluster_node_type_mandatory); - BadNames -> e(invalid_cluster_nodes_conf) + case lists:foldr(FindBadNodeNames, [], Nodes) of + [] -> e(cluster_node_type_mandatory); + _ -> e(invalid_cluster_nodes_conf) end; {ok, _} -> e(invalid_cluster_nodes_conf) -- cgit v1.2.1 From 6373af3300b850c1d4c74ca8e5f8453c3da3eb18 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 25 Nov 2014 19:21:27 +0100 Subject: Add more properties to the user_authentication_* notifications Until now, the only property was {name, Username}. The added properties are: o {connection_type, network | direct} o {error, Message} (only if the authentication failed) For network connections, the following informations are added as returned by rabbit_reader:infos/2: o auth_mechanism o host o name (the property is renamed to connection_name to avoid conflict with the username) o peer_cert_issuer o peer_cert_subject o peer_cert_validity o peer_host o peer_port o protocol o ssl o ssl_cipher o ssl_protocol o vhost The notification is sent by rabbit_reader:notify_auth_result/5 and rabbit_direct:notify_auth_result/4, not by rabbit_access_control:check_user_login/2 anymore. This fixes a bug where a "user_authentication_success" event would be sent by rabbit_access_control:check_user_login/2, even if rabbit_reader:auth_phase/2 rejects the user later because the connection isn't on the loopback interface. --- src/rabbit_access_control.erl | 31 ++++++++++++------------ src/rabbit_direct.erl | 20 ++++++++++++++-- src/rabbit_reader.erl | 55 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 83 insertions(+), 23 deletions(-) diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index d1577432..41c54b07 100644 --- a/src/rabbit_access_control.erl +++ b/src/rabbit_access_control.erl @@ -31,10 +31,12 @@ -spec(check_user_pass_login/2 :: (rabbit_types:username(), rabbit_types:password()) - -> {'ok', rabbit_types:user()} | {'refused', string(), [any()]}). + -> {'ok', rabbit_types:user()} | + {'refused', rabbit_types:username(), string(), [any()]}). -spec(check_user_login/2 :: (rabbit_types:username(), [{atom(), any()}]) - -> {'ok', rabbit_types:user()} | {'refused', string(), [any()]}). + -> {'ok', rabbit_types:user()} | + {'refused', rabbit_types:username(), string(), [any()]}). -spec(check_user_loopback/2 :: (rabbit_types:username(), rabbit_net:socket() | inet:ip_address()) -> 'ok' | 'not_allowed'). @@ -55,7 +57,7 @@ check_user_pass_login(Username, Password) -> check_user_login(Username, AuthProps) -> {ok, Modules} = application:get_env(rabbit, auth_backends), R = lists:foldl( - fun ({ModN, ModZs0}, {refused, _, _}) -> + fun ({ModN, ModZs0}, {refused, _, _, _}) -> ModZs = case ModZs0 of A when is_atom(A) -> [A]; L when is_list(L) -> L @@ -69,7 +71,7 @@ check_user_login(Username, AuthProps) -> Else -> Else end; - (Mod, {refused, _, _}) -> + (Mod, {refused, _, _, _}) -> %% Same module for authN and authZ. Just take the result %% it gives us case try_authenticate(Mod, Username, AuthProps) of @@ -81,19 +83,17 @@ check_user_login(Username, AuthProps) -> (_, {ok, User}) -> %% We've successfully authenticated. Skip to the end... {ok, User} - end, {refused, "No modules checked '~s'", [Username]}, Modules), - rabbit_event:notify(case R of - {ok, _User} -> user_authentication_success; - _ -> user_authentication_failure - end, [{name, Username}]), + end, + {refused, Username, "No modules checked '~s'", [Username]}, Modules), R. try_authenticate(Module, Username, AuthProps) -> case Module:user_login_authentication(Username, AuthProps) of {ok, AuthUser} -> {ok, AuthUser}; - {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", + {error, E} -> {refused, Username, + "~s failed authenticating ~s: ~p~n", [Module, Username, E]}; - {refused, F, A} -> {refused, F, A} + {refused, F, A} -> {refused, Username, F, A} end. try_authorize(Modules, Username) -> @@ -101,12 +101,13 @@ try_authorize(Modules, Username) -> fun (Module, {ok, ModsImpls}) -> case Module:user_login_authorization(Username) of {ok, Impl} -> {ok, [{Module, Impl} | ModsImpls]}; - {error, E} -> {refused, "~s failed authorizing ~s: ~p~n", + {error, E} -> {refused, Username, + "~s failed authorizing ~s: ~p~n", [Module, Username, E]}; - {refused, F, A} -> {refused, F, A} + {refused, F, A} -> {refused, Username, F, A} end; - (_, {refused, _, _} = Error) -> - Error + (_, {refused, F, A}) -> + {refused, Username, F, A} end, {ok, []}, Modules). user(#auth_user{username = Username, tags = Tags}, {ok, ModZImpls}) -> diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index f6140f09..9756dd49 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -83,14 +83,30 @@ connect({Username, Password}, VHost, Protocol, Pid, Infos) -> connect0(AuthFun, VHost, Protocol, Pid, Infos) -> case rabbit:is_running() of true -> case AuthFun() of - {ok, User} -> + {ok, User = #user{username = Username}} -> + notify_auth_result(Username, + user_authentication_success, "", []), connect1(User, VHost, Protocol, Pid, Infos); - {refused, _M, _A} -> + {refused, Username, Msg, Args} -> + notify_auth_result(Username, + user_authentication_failure, Msg, Args), {error, {auth_failure, "Refused"}} end; false -> {error, broker_not_found_on_node} end. +notify_auth_result(Username, AuthResult, Msg, Args) -> + EventProps0 = [{connection_type, direct}], + EventProps1 = case Username of + none -> [{name, ''} | EventProps0]; + _ -> [{name, Username} | EventProps0] + end, + EventProps = case Msg of + "" -> EventProps1; + _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps1] + end, + rabbit_event:notify(AuthResult, EventProps). + connect1(User, VHost, Protocol, Pid, Infos) -> try rabbit_access_control:check_vhost_access(User, VHost, undefined) of ok -> ok = pg_local:join(rabbit_direct, Pid), diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 2033dd14..a18d75d7 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1046,9 +1046,15 @@ auth_phase(Response, auth_state = AuthState}, sock = Sock}) -> case AuthMechanism:handle_response(Response, AuthState) of + {refused, Username, Msg, Args} -> + auth_fail(Username, Msg, Args, Name, State); {refused, Msg, Args} -> - auth_fail(Msg, Args, Name, State); + %% Older auth mechanisms didn't return the username, even if + %% they reach a stage where they know it. + auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> + notify_auth_result(none, user_authentication_failure, + Msg, Args, State), rabbit_misc:protocol_error(syntax_error, Msg, Args); {challenge, Challenge, AuthState1} -> Secure = #'connection.secure'{challenge = Challenge}, @@ -1057,9 +1063,12 @@ auth_phase(Response, auth_state = AuthState1}}; {ok, User = #user{username = Username}} -> case rabbit_access_control:check_user_loopback(Username, Sock) of - ok -> ok; - not_allowed -> auth_fail("user '~s' can only connect via " - "localhost", [Username], Name, State) + ok -> + notify_auth_result(Username, user_authentication_success, + "", [], State); + not_allowed -> + auth_fail(Username, "user '~s' can only connect via " + "localhost", [Username], Name, State) end, Tune = #'connection.tune'{frame_max = get_env(frame_max), channel_max = get_env(channel_max), @@ -1071,11 +1080,14 @@ auth_phase(Response, end. -ifdef(use_specs). --spec(auth_fail/4 :: (string(), [any()], binary(), #v1{}) -> no_return()). +-spec(auth_fail/5 :: + (rabbit_types:username() | none, string(), [any()], binary(), #v1{}) -> + no_return()). -endif. -auth_fail(Msg, Args, AuthName, +auth_fail(Username, Msg, Args, AuthName, State = #v1{connection = #connection{protocol = Protocol, capabilities = Capabilities}}) -> + notify_auth_result(Username, user_authentication_failure, Msg, Args, State), AmqpError = rabbit_misc:amqp_error( access_refused, "~s login refused: ~s", [AuthName, io_lib:format(Msg, Args)], none), @@ -1094,6 +1106,37 @@ auth_fail(Msg, Args, AuthName, end, rabbit_misc:protocol_error(AmqpError). +notify_auth_result(Username, AuthResult, Msg, Args, State) -> + EventProps0 = [{connection_type, network}], + EventProps1 = lists:foldl( + fun + (name, Acc) -> [{connection_name, i(name, State)} | Acc]; + (Item, Acc) -> [{Item, i(Item, State)} | Acc] + end, EventProps0, [ + peer_cert_validity, + peer_cert_subject, + peer_cert_issuer, + ssl_cipher, + ssl_protocol, + ssl, + auth_mechanism, + protocol, + peer_port, + peer_host, + name, + vhost, + host + ]), + EventProps2 = case Username of + none -> [{name, ''} | EventProps1]; + _ -> [{name, Username} | EventProps1] + end, + EventProps = case Msg of + "" -> EventProps2; + _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps2] + end, + rabbit_event:notify(AuthResult, EventProps). + %%-------------------------------------------------------------------------- infos(Items, State) -> [{Item, i(Item, State)} || Item <- Items]. -- cgit v1.2.1 From 5122b4cc7a74bea9fe0a00bf4ac3c818b1f4259c Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 12:35:49 +0100 Subject: During startup, log statistics after modules were hipe-compiled A similar message was already displayed on stdout. A warning was also logged when HiPE was desired but unavailable. To be consistent with the "HiPE enabled" case, the same warning is now displayed on stdout too. --- src/rabbit.erl | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/rabbit.erl b/src/rabbit.erl index 664da206..ca1d5ba8 100644 --- a/src/rabbit.erl +++ b/src/rabbit.erl @@ -243,15 +243,19 @@ maybe_hipe_compile() -> {ok, Want} = application:get_env(rabbit, hipe_compile), Can = code:which(hipe) =/= non_existing, case {Want, Can} of - {true, true} -> hipe_compile(), - true; + {true, true} -> hipe_compile(); {true, false} -> false; - {false, _} -> true + {false, _} -> {ok, disabled} end. -warn_if_hipe_compilation_failed(true) -> +warn_if_hipe_compilation_failed({ok, disabled}) -> ok; +warn_if_hipe_compilation_failed({ok, Count, Duration}) -> + rabbit_log:info( + "HiPE in use: compiled ~B modules in ~Bs.~n", [Count, Duration]); warn_if_hipe_compilation_failed(false) -> + io:format( + "~nNot HiPE compiling: HiPE not found in this Erlang installation.~n"), rabbit_log:warning( "Not HiPE compiling: HiPE not found in this Erlang installation.~n"). @@ -276,8 +280,9 @@ hipe_compile() -> {'DOWN', MRef, process, _, Reason} -> exit(Reason) end || {_Pid, MRef} <- PidMRefs], T2 = erlang:now(), - io:format("|~n~nCompiled ~B modules in ~Bs~n", - [Count, timer:now_diff(T2, T1) div 1000000]). + Duration = timer:now_diff(T2, T1) div 1000000, + io:format("|~n~nCompiled ~B modules in ~Bs~n", [Count, Duration]), + {ok, Count, Duration}. split(L, N) -> split0(L, [[] || _ <- lists:seq(1, N)]). -- cgit v1.2.1 -- cgit v1.2.1 -- cgit v1.2.1 From 943bff538149eb83c96ec44566958d82ee394356 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Wed, 26 Nov 2014 12:20:43 +0000 Subject: Rename this since it's not quite just success / failure any more. --- src/rabbit.erl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rabbit.erl b/src/rabbit.erl index ca1d5ba8..40f24efc 100644 --- a/src/rabbit.erl +++ b/src/rabbit.erl @@ -248,12 +248,12 @@ maybe_hipe_compile() -> {false, _} -> {ok, disabled} end. -warn_if_hipe_compilation_failed({ok, disabled}) -> +log_hipe_result({ok, disabled}) -> ok; -warn_if_hipe_compilation_failed({ok, Count, Duration}) -> +log_hipe_result({ok, Count, Duration}) -> rabbit_log:info( "HiPE in use: compiled ~B modules in ~Bs.~n", [Count, Duration]); -warn_if_hipe_compilation_failed(false) -> +log_hipe_result(false) -> io:format( "~nNot HiPE compiling: HiPE not found in this Erlang installation.~n"), rabbit_log:warning( @@ -312,9 +312,9 @@ start() -> boot() -> start_it(fun() -> ok = ensure_application_loaded(), - Success = maybe_hipe_compile(), + HipeResult = maybe_hipe_compile(), ok = ensure_working_log_handlers(), - warn_if_hipe_compilation_failed(Success), + log_hipe_result(HipeResult), rabbit_node_monitor:prepare_cluster_status_files(), ok = rabbit_upgrade:maybe_upgrade_mnesia(), %% It's important that the consistency check happens after -- cgit v1.2.1 From 9c53f5f1720a00b2166719830fabe05a025bcede Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Wed, 26 Nov 2014 14:12:13 +0000 Subject: First pass at "rabbitmqctl rename_current_node" --- src/rabbit_control_main.erl | 13 ++++- src/rabbit_mnesia_offline.erl | 129 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 src/rabbit_mnesia_offline.erl diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index a931eef0..b19971fc 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -19,7 +19,7 @@ -include("rabbit_cli.hrl"). -export([start/0, stop/0, parse_arguments/2, action/5, - sync_queue/1, cancel_sync_queue/1]). + sync_queue/1, cancel_sync_queue/1, become/1]). -import(rabbit_cli, [rpc_call/4]). @@ -40,6 +40,7 @@ change_cluster_node_type, update_cluster_nodes, {forget_cluster_node, [?OFFLINE_DEF]}, + rename_current_node, force_boot, cluster_status, {sync_queue, [?VHOST_DEF]}, @@ -104,8 +105,8 @@ -define(COMMANDS_NOT_REQUIRING_APP, [stop, stop_app, start_app, wait, reset, force_reset, rotate_logs, join_cluster, change_cluster_node_type, update_cluster_nodes, - forget_cluster_node, cluster_status, status, environment, eval, - force_boot]). + forget_cluster_node, rename_current_node, cluster_status, status, + environment, eval, force_boot]). %%---------------------------------------------------------------------------- @@ -234,6 +235,12 @@ action(forget_cluster_node, Node, [ClusterNodeS], Opts, Inform) -> [ClusterNode, false]) end; +action(rename_current_node, _Node, [FromNodeS, ToNodeS], _Opts, Inform) -> + FromNode = list_to_atom(FromNodeS), + ToNode = list_to_atom(ToNodeS), + Inform("Renaming local cluster node ~s to ~s", [FromNode, ToNode]), + rabbit_mnesia_offline:rename_local_node(FromNode, ToNode); + action(force_boot, Node, [], _Opts, Inform) -> Inform("Forcing boot for Mnesia dir ~s", [mnesia:system_info(directory)]), case rabbit:is_running(Node) of diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl new file mode 100644 index 00000000..a8608dfa --- /dev/null +++ b/src/rabbit_mnesia_offline.erl @@ -0,0 +1,129 @@ +%% The contents of this file are subject to the Mozilla Public License +%% Version 1.1 (the "License"); you may not use this file except in +%% compliance with the License. You may obtain a copy of the License +%% at http://www.mozilla.org/MPL/ +%% +%% Software distributed under the License is distributed on an "AS IS" +%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See +%% the License for the specific language governing rights and +%% limitations under the License. +%% +%% The Original Code is RabbitMQ. +%% +%% The Initial Developer of the Original Code is GoPivotal, Inc. +%% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. +%% + +-module(rabbit_mnesia_offline). + +-export([rename_local_node/2]). + +%%---------------------------------------------------------------------------- + +-ifdef(use_specs). + +-spec(rename_local_node/2 :: (node(), node()) -> 'ok'). + +-endif. + +%%---------------------------------------------------------------------------- + +rename_local_node(FromNode, ToNode) -> + try + rabbit_control_main:become(FromNode), + rabbit_log:info("Renaming node ~s to ~s~n", [FromNode, ToNode]), + start_mnesia(), + Nodes = rabbit_mnesia:cluster_nodes(all), + case {lists:member(FromNode, Nodes), lists:member(ToNode, Nodes)} of + {true, false} -> ok; + {false, _} -> exit({node_not_in_cluster, FromNode}); + {_, true} -> exit({node_already_in_cluster, ToNode}) + end, + rabbit_table:force_load(), + rabbit_table:wait_for_replicated(), + FromBackup = rabbit_mnesia:dir() ++ "/rename-backup-from", + ToBackup = rabbit_mnesia:dir() ++ "/rename-backup-to", + io:format(" * Backing up to '~s'~n", [FromBackup]), + ok = mnesia:backup(FromBackup), + stop_mnesia(), + rabbit_control_main:become(ToNode), + io:format(" * Converting backup '~s'~n", [ToBackup]), + convert_backup(FromNode, ToNode, FromBackup, ToBackup), + ok = mnesia:install_fallback(ToBackup), + io:format(" * Loading backup '~s'~n", [ToBackup]), + start_mnesia(), + io:format(" * Converting config files~n", []), + convert_config_file(FromNode, ToNode, + rabbit_node_monitor:running_nodes_filename()), + convert_config_file(FromNode, ToNode, + rabbit_node_monitor:cluster_status_filename()), + ok + after + stop_mnesia() + end. + +start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). +stop_mnesia() -> stopped = mnesia:stop(). + +convert_backup(FromNode, ToNode, FromBackup, ToBackup) -> + Switch = fun + (Node) when Node == FromNode -> + ToNode; + (Node) when Node == ToNode -> + throw({error, {already_exists, Node}}); + (Node) -> + Node + end, + Convert = + fun + %% TODO do we ever hit these three heads? + ({schema, db_nodes, Nodes}, Acc) -> + io:format(" +++ db_nodes ~p~n", [Nodes]), + {[{schema, db_nodes, lists:map(Switch,Nodes)}], Acc}; + ({schema, version, Version}, Acc) -> + io:format(" +++ version: ~p~n", [Version]), + {[{schema, version, Version}], Acc}; + ({schema, cookie, Cookie}, Acc) -> + io:format(" +++ cookie: ~p~n", [Cookie]), + {[{schema, cookie, Cookie}], Acc}; + ({schema, Tab, CreateList}, Acc) -> + %% io:format("~n * Checking table: '~p'~n", [Tab]), + %%io:format(" . Initial content: ~p~n", [CreateList]), + Keys = [ram_copies, disc_copies, disc_only_copies], + OptSwitch = + fun({Key, Val}) -> + case lists:member(Key, Keys) of + true -> + %%io:format(" + Checking key: '~p'~n", [Key]), + {Key, lists:map(Switch, Val)}; + false-> + {Key, Val} + end + end, + Res = {[{schema, Tab, lists:map(OptSwitch, CreateList)}], Acc}, + %%io:format(" . Resulting content: ~p~n", [Res]), + Res; + (Other, Acc) -> + case lists:member(element(1, Other), [rabbit_durable_queue]) of + true -> Other1 = update_term(FromNode, ToNode, Other), + io:format(" --- ~p~n +++ ~p~n", [Other, Other1]), + {[Other1], Acc}; + false -> {[Other], Acc} + end + end, + mnesia:traverse_backup(FromBackup, ToBackup, Convert, switched). + +convert_config_file(FromNode, ToNode, Path) -> + {ok, Term} = rabbit_file:read_term_file(Path), + ok = rabbit_file:write_term_file(Path, update_term(FromNode, ToNode, Term)). + +update_term(N1, N2, L) when is_list(L) -> + [update_term(N1, N2, I) || I <- L]; +update_term(N1, N2, T) when is_tuple(T) -> + list_to_tuple(update_term(N1, N2, tuple_to_list(T))); +update_term(N1, N2, N1) -> + N2; +update_term(N1, N2, Pid) when is_pid(Pid), node(Pid) == N1 -> + rabbit_misc:node_to_fake_pid(N2); +update_term(_N1, _N2, Term) -> + Term. -- cgit v1.2.1 From cc068dee6d8b8762b5e8f10df6244313feb47633 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Wed, 26 Nov 2014 15:47:22 +0000 Subject: Create a way to mutate pids and use that rather than creating "fake" pids. --- Makefile | 2 +- src/rabbit_misc.erl | 31 ++++++++++++++++++++++--------- src/rabbit_mnesia_offline.erl | 2 +- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index c955a8fc..51db8d4c 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ TMPDIR ?= /tmp RABBITMQ_NODENAME ?= rabbit RABBITMQ_SERVER_START_ARGS ?= -RABBITMQ_MNESIA_DIR ?= $(TMPDIR)/rabbitmq-$(RABBITMQ_NODENAME)-mnesia +RABBITMQ_MNESIA_DIR ?= $(TMPDIR)/rabbitmq-mnesia RABBITMQ_PLUGINS_EXPAND_DIR ?= $(TMPDIR)/rabbitmq-$(RABBITMQ_NODENAME)-plugins-scratch RABBITMQ_LOG_BASE ?= $(TMPDIR) diff --git a/src/rabbit_misc.erl b/src/rabbit_misc.erl index 3e2c88ee..20d7051c 100644 --- a/src/rabbit_misc.erl +++ b/src/rabbit_misc.erl @@ -44,7 +44,8 @@ -export([format/2, format_many/1, format_stderr/2]). -export([unfold/2, ceil/1, queue_fold/3]). -export([sort_field_table/1]). --export([pid_to_string/1, string_to_pid/1, node_to_fake_pid/1]). +-export([pid_to_string/1, string_to_pid/1, + pid_change_node/2, node_to_fake_pid/1]). -export([version_compare/2, version_compare/3]). -export([version_minor_equivalent/2]). -export([dict_cons/3, orddict_cons/3, gb_trees_cons/3]). @@ -196,6 +197,7 @@ (rabbit_framing:amqp_table()) -> rabbit_framing:amqp_table()). -spec(pid_to_string/1 :: (pid()) -> string()). -spec(string_to_pid/1 :: (string()) -> pid()). +-spec(pid_change_node/2 :: (pid(), node()) -> pid()). -spec(node_to_fake_pid/1 :: (atom()) -> pid()). -spec(version_compare/2 :: (string(), string()) -> 'lt' | 'eq' | 'gt'). -spec(version_compare/3 :: @@ -686,11 +688,7 @@ sort_field_table(Arguments) -> %% regardless of what node we are running on. The representation also %% permits easy identification of the pid's node. pid_to_string(Pid) when is_pid(Pid) -> - %% see http://erlang.org/doc/apps/erts/erl_ext_dist.html (8.10 and - %% 8.7) - <<131,103,100,NodeLen:16,NodeBin:NodeLen/binary,Id:32,Ser:32,Cre:8>> - = term_to_binary(Pid), - Node = binary_to_term(<<131,100,NodeLen:16,NodeBin:NodeLen/binary>>), + {Node, Cre, Id, Ser} = decompose_pid(Pid), format("<~s.~B.~B.~B>", [Node, Cre, Id, Ser]). %% inverse of above @@ -701,17 +699,32 @@ string_to_pid(Str) -> case re:run(Str, "^<(.*)\\.(\\d+)\\.(\\d+)\\.(\\d+)>\$", [{capture,all_but_first,list}]) of {match, [NodeStr, CreStr, IdStr, SerStr]} -> - <<131,NodeEnc/binary>> = term_to_binary(list_to_atom(NodeStr)), [Cre, Id, Ser] = lists:map(fun list_to_integer/1, [CreStr, IdStr, SerStr]), - binary_to_term(<<131,103,NodeEnc/binary,Id:32,Ser:32,Cre:8>>); + compose_pid(list_to_atom(NodeStr), Cre, Id, Ser); nomatch -> throw(Err) end. +pid_change_node(Pid, NewNode) -> + {_OldNode, Cre, Id, Ser} = decompose_pid(Pid), + compose_pid(NewNode, Cre, Id, Ser). + %% node(node_to_fake_pid(Node)) =:= Node. node_to_fake_pid(Node) -> - string_to_pid(format("<~s.0.0.0>", [Node])). + compose_pid(Node, 0, 0, 0). + +decompose_pid(Pid) when is_pid(Pid) -> + %% see http://erlang.org/doc/apps/erts/erl_ext_dist.html (8.10 and + %% 8.7) + <<131,103,100,NodeLen:16,NodeBin:NodeLen/binary,Id:32,Ser:32,Cre:8>> + = term_to_binary(Pid), + Node = binary_to_term(<<131,100,NodeLen:16,NodeBin:NodeLen/binary>>), + {Node, Cre, Id, Ser}. + +compose_pid(Node, Cre, Id, Ser) -> + <<131,NodeEnc/binary>> = term_to_binary(Node), + binary_to_term(<<131,103,NodeEnc/binary,Id:32,Ser:32,Cre:8>>). version_compare(A, B, lte) -> case version_compare(A, B) of diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl index a8608dfa..24f7709d 100644 --- a/src/rabbit_mnesia_offline.erl +++ b/src/rabbit_mnesia_offline.erl @@ -124,6 +124,6 @@ update_term(N1, N2, T) when is_tuple(T) -> update_term(N1, N2, N1) -> N2; update_term(N1, N2, Pid) when is_pid(Pid), node(Pid) == N1 -> - rabbit_misc:node_to_fake_pid(N2); + rabbit_misc:pid_change_node(Pid, N2); update_term(_N1, _N2, Term) -> Term. -- cgit v1.2.1 From e24b4f938793f763b3142da7b59110184b50f2b3 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Wed, 26 Nov 2014 15:48:32 +0000 Subject: Oops --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 51db8d4c..c955a8fc 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ TMPDIR ?= /tmp RABBITMQ_NODENAME ?= rabbit RABBITMQ_SERVER_START_ARGS ?= -RABBITMQ_MNESIA_DIR ?= $(TMPDIR)/rabbitmq-mnesia +RABBITMQ_MNESIA_DIR ?= $(TMPDIR)/rabbitmq-$(RABBITMQ_NODENAME)-mnesia RABBITMQ_PLUGINS_EXPAND_DIR ?= $(TMPDIR)/rabbitmq-$(RABBITMQ_NODENAME)-plugins-scratch RABBITMQ_LOG_BASE ?= $(TMPDIR) -- cgit v1.2.1 From 77651e4c3fce5a622b3bb74d006fe53bc86b5c01 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 16:51:47 +0100 Subject: Update rabbit_auth_mechanism:handle_response() spec to match the change --- src/rabbit_auth_mechanism.erl | 5 ++++- src/rabbit_reader.erl | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/rabbit_auth_mechanism.erl b/src/rabbit_auth_mechanism.erl index d11af095..4f771db2 100644 --- a/src/rabbit_auth_mechanism.erl +++ b/src/rabbit_auth_mechanism.erl @@ -37,12 +37,15 @@ %% {protocol_error, Msg, Args} %% Client got the protocol wrong. Log and die. %% {refused, Msg, Args} +%% (deprecated) Client failed authentication. Log and die. +%% {refused, Username, Msg, Args} %% Client failed authentication. Log and die. -callback handle_response(binary(), any()) -> {'ok', rabbit_types:user()} | {'challenge', binary(), any()} | {'protocol_error', string(), [any()]} | - {'refused', string(), [any()]}. + {'refused', string(), [any()]} | + {'refused', rabbit_types:username() | none, string(), [any()]}. -else. diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index a18d75d7..05ed3eda 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1049,8 +1049,8 @@ auth_phase(Response, {refused, Username, Msg, Args} -> auth_fail(Username, Msg, Args, Name, State); {refused, Msg, Args} -> - %% Older auth mechanisms didn't return the username, even if - %% they reach a stage where they know it. + %% Deprecated: older auth mechanisms didn't return the + %% username, even if they reach a stage where they know it. auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> notify_auth_result(none, user_authentication_failure, -- cgit v1.2.1 From 62dc4e1df7d259f9bee6059a8735c1551a9361be Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 17:01:45 +0100 Subject: Rephrase a comment to use the present tense --- src/rabbit_reader.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 05ed3eda..364dfc05 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1049,7 +1049,7 @@ auth_phase(Response, {refused, Username, Msg, Args} -> auth_fail(Username, Msg, Args, Name, State); {refused, Msg, Args} -> - %% Deprecated: older auth mechanisms didn't return the + %% Deprecated: older auth mechanisms don't return the %% username, even if they reach a stage where they know it. auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> -- cgit v1.2.1 From 8a25a1aa86eb707bb0ed68d3f78f18f4a25c0bc5 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 17:53:45 +0100 Subject: Drop {refuse, _, _} from rabbit_auth_mechanism:handle_response/2 return values --- src/rabbit_auth_mechanism.erl | 3 --- src/rabbit_reader.erl | 4 ---- 2 files changed, 7 deletions(-) diff --git a/src/rabbit_auth_mechanism.erl b/src/rabbit_auth_mechanism.erl index 4f771db2..c8e23a75 100644 --- a/src/rabbit_auth_mechanism.erl +++ b/src/rabbit_auth_mechanism.erl @@ -36,15 +36,12 @@ %% Another round is needed. Here's the state I want next time. %% {protocol_error, Msg, Args} %% Client got the protocol wrong. Log and die. -%% {refused, Msg, Args} -%% (deprecated) Client failed authentication. Log and die. %% {refused, Username, Msg, Args} %% Client failed authentication. Log and die. -callback handle_response(binary(), any()) -> {'ok', rabbit_types:user()} | {'challenge', binary(), any()} | {'protocol_error', string(), [any()]} | - {'refused', string(), [any()]} | {'refused', rabbit_types:username() | none, string(), [any()]}. -else. diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 364dfc05..713b1844 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1048,10 +1048,6 @@ auth_phase(Response, case AuthMechanism:handle_response(Response, AuthState) of {refused, Username, Msg, Args} -> auth_fail(Username, Msg, Args, Name, State); - {refused, Msg, Args} -> - %% Deprecated: older auth mechanisms don't return the - %% username, even if they reach a stage where they know it. - auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> notify_auth_result(none, user_authentication_failure, Msg, Args, State), -- cgit v1.2.1 From 8cf49c21fcf76ae01a914abb6f51fe9421481723 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Wed, 26 Nov 2014 18:28:30 +0000 Subject: WIP towards rabbitmqctl rename_other_node. --- src/rabbit_control_main.erl | 8 ++++++++ src/rabbit_mnesia_offline.erl | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index b19971fc..fc049da1 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -41,6 +41,7 @@ update_cluster_nodes, {forget_cluster_node, [?OFFLINE_DEF]}, rename_current_node, + rename_other_node, force_boot, cluster_status, {sync_queue, [?VHOST_DEF]}, @@ -241,6 +242,13 @@ action(rename_current_node, _Node, [FromNodeS, ToNodeS], _Opts, Inform) -> Inform("Renaming local cluster node ~s to ~s", [FromNode, ToNode]), rabbit_mnesia_offline:rename_local_node(FromNode, ToNode); +action(rename_other_node, Node, [FromNodeS, ToNodeS], _Opts, Inform) -> + FromNode = list_to_atom(FromNodeS), + ToNode = list_to_atom(ToNodeS), + Inform("Renaming remote cluster node ~s to ~s", [FromNode, ToNode]), + rpc_call(Node, rabbit_mnesia_offline, rename_remote_node, + [FromNode, ToNode]); + action(force_boot, Node, [], _Opts, Inform) -> Inform("Forcing boot for Mnesia dir ~s", [mnesia:system_info(directory)]), case rabbit:is_running(Node) of diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl index 24f7709d..3d9d8642 100644 --- a/src/rabbit_mnesia_offline.erl +++ b/src/rabbit_mnesia_offline.erl @@ -17,12 +17,14 @@ -module(rabbit_mnesia_offline). -export([rename_local_node/2]). +-export([rename_remote_node/2]). %%---------------------------------------------------------------------------- -ifdef(use_specs). -spec(rename_local_node/2 :: (node(), node()) -> 'ok'). +-spec(rename_remote_node/2 :: (node(), node()) -> 'ok'). -endif. @@ -49,7 +51,7 @@ rename_local_node(FromNode, ToNode) -> rabbit_control_main:become(ToNode), io:format(" * Converting backup '~s'~n", [ToBackup]), convert_backup(FromNode, ToNode, FromBackup, ToBackup), - ok = mnesia:install_fallback(ToBackup), + ok = mnesia:install_fallback(ToBackup, [{scope, local}]), io:format(" * Loading backup '~s'~n", [ToBackup]), start_mnesia(), io:format(" * Converting config files~n", []), @@ -127,3 +129,21 @@ update_term(N1, N2, Pid) when is_pid(Pid), node(Pid) == N1 -> rabbit_misc:pid_change_node(Pid, N2); update_term(_N1, _N2, Term) -> Term. + +%%---------------------------------------------------------------------------- + +rename_remote_node(FromNode, ToNode) -> + All = rabbit_mnesia:cluster_nodes(all), + Running = rabbit_mnesia:cluster_nodes(running), + case {lists:member(FromNode, All), + lists:member(FromNode, Running), + lists:member(ToNode, All)} of + {true, false, false} -> ok; + {false, _, _} -> exit({node_not_in_cluster, FromNode}); + {_, true, _} -> exit({node_running, FromNode}); + {_, _, true} -> exit({node_already_in_cluster, ToNode}) + end, + mnesia:del_table_copy(schema, FromNode), + mnesia:change_config(extra_db_nodes, [ToNode]), + mnesia:add_table_copy(schema, ToNode, ram_copies), + ok. -- cgit v1.2.1 From 79abe0891a9ce0e240c82d49bafe6d224482501a Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 27 Nov 2014 12:59:49 +0000 Subject: Rearrange things a bit. We now have something that seems to work. Merging a modified database back into a running cluster seems to be a bit of a non-starter; Mnesia blows up with "Failed to merge schema: Incompatible schema cookies." So instead we do something similar to if we were doing upgrades: we always create the modified backup, but only load it if we are the first node in the cluster to start; if not we reset our mnesia state, resync with the cluster, then tell the cluster to forget our old incarnation and update its queue records for the new one. This also has the advantage that we don't need two rename commands, we just have the local offline one. Which is hopefully nicer operationally. It does mean that we go to the trouble of taking the backup and modifying it only to throw it away if we turn out to be a secondary node, but it should not be too expensive. --- src/rabbit_control_main.erl | 8 ------ src/rabbit_mnesia_offline.erl | 62 +++++++++++++++++++++++++++++++++++-------- src/rabbit_upgrade.erl | 6 +++-- 3 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index fc049da1..b19971fc 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -41,7 +41,6 @@ update_cluster_nodes, {forget_cluster_node, [?OFFLINE_DEF]}, rename_current_node, - rename_other_node, force_boot, cluster_status, {sync_queue, [?VHOST_DEF]}, @@ -242,13 +241,6 @@ action(rename_current_node, _Node, [FromNodeS, ToNodeS], _Opts, Inform) -> Inform("Renaming local cluster node ~s to ~s", [FromNode, ToNode]), rabbit_mnesia_offline:rename_local_node(FromNode, ToNode); -action(rename_other_node, Node, [FromNodeS, ToNodeS], _Opts, Inform) -> - FromNode = list_to_atom(FromNodeS), - ToNode = list_to_atom(ToNodeS), - Inform("Renaming remote cluster node ~s to ~s", [FromNode, ToNode]), - rpc_call(Node, rabbit_mnesia_offline, rename_remote_node, - [FromNode, ToNode]); - action(force_boot, Node, [], _Opts, Inform) -> Inform("Forcing boot for Mnesia dir ~s", [mnesia:system_info(directory)]), case rabbit:is_running(Node) of diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl index 3d9d8642..ae72462e 100644 --- a/src/rabbit_mnesia_offline.erl +++ b/src/rabbit_mnesia_offline.erl @@ -15,9 +15,11 @@ %% -module(rabbit_mnesia_offline). +-include("rabbit.hrl"). -export([rename_local_node/2]). -export([rename_remote_node/2]). +-export([maybe_complete_rename/2]). %%---------------------------------------------------------------------------- @@ -43,17 +45,16 @@ rename_local_node(FromNode, ToNode) -> end, rabbit_table:force_load(), rabbit_table:wait_for_replicated(), - FromBackup = rabbit_mnesia:dir() ++ "/rename-backup-from", - ToBackup = rabbit_mnesia:dir() ++ "/rename-backup-to", + FromBackup = from_backup_name(), + ToBackup = to_backup_name(), io:format(" * Backing up to '~s'~n", [FromBackup]), ok = mnesia:backup(FromBackup), stop_mnesia(), rabbit_control_main:become(ToNode), io:format(" * Converting backup '~s'~n", [ToBackup]), convert_backup(FromNode, ToNode, FromBackup, ToBackup), - ok = mnesia:install_fallback(ToBackup, [{scope, local}]), - io:format(" * Loading backup '~s'~n", [ToBackup]), - start_mnesia(), + ok = rabbit_file:write_term_file(rename_config_name(), + [{FromNode, ToNode}]), io:format(" * Converting config files~n", []), convert_config_file(FromNode, ToNode, rabbit_node_monitor:running_nodes_filename()), @@ -64,6 +65,42 @@ rename_local_node(FromNode, ToNode) -> stop_mnesia() end. +maybe_complete_rename(primary, _AllNodes) -> + case rabbit_file:read_term_file(rename_config_name()) of + {ok, [{_FromNode, _ToNode}]} -> + %% We are alone, restore the backup we previously took + ToBackup = to_backup_name(), + io:format(" * Loading backup '~s'~n", [ToBackup]), + ok = mnesia:install_fallback(ToBackup, [{scope, local}]), + start_mnesia(), + stop_mnesia(), + rabbit_file:delete(rename_config_name()), + rabbit_file:delete(from_backup_name()), + rabbit_file:delete(to_backup_name()), + ok; + _ -> + ok + end; + +maybe_complete_rename(secondary, AllNodes) -> + case rabbit_file:read_term_file(rename_config_name()) of + {ok, [{FromNode, ToNode}]} -> + rabbit_upgrade:secondary_upgrade(AllNodes), + [Another | _] = rabbit_mnesia:cluster_nodes(running) -- [node()], + ok = rpc:call(Another, ?MODULE, rename_remote_node, + [FromNode, ToNode]), + rabbit_file:delete(rename_config_name()), + rabbit_file:delete(from_backup_name()), + rabbit_file:delete(to_backup_name()), + ok; + _ -> + ok + end. + +from_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-from". +to_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-to". +rename_config_name() -> rabbit_mnesia:dir() ++ "/rename-pending.config". + start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). stop_mnesia() -> stopped = mnesia:stop(). @@ -138,12 +175,15 @@ rename_remote_node(FromNode, ToNode) -> case {lists:member(FromNode, All), lists:member(FromNode, Running), lists:member(ToNode, All)} of - {true, false, false} -> ok; - {false, _, _} -> exit({node_not_in_cluster, FromNode}); - {_, true, _} -> exit({node_running, FromNode}); - {_, _, true} -> exit({node_already_in_cluster, ToNode}) + {true, false, true} -> ok; + {false, _, _} -> exit({old_node_not_in_cluster, FromNode}); + {_, true, _} -> exit({old_node_running, FromNode}); + {_, _, false} -> exit({new_node_not_in_cluster, ToNode}) end, mnesia:del_table_copy(schema, FromNode), - mnesia:change_config(extra_db_nodes, [ToNode]), - mnesia:add_table_copy(schema, ToNode, ram_copies), + {atomic, ok} = mnesia:transform_table( + rabbit_durable_queue, + fun (Q) -> update_term(FromNode, ToNode, Q) end, + record_info(fields, amqqueue)), ok. + diff --git a/src/rabbit_upgrade.erl b/src/rabbit_upgrade.erl index 72bf7855..420aa205 100644 --- a/src/rabbit_upgrade.erl +++ b/src/rabbit_upgrade.erl @@ -16,7 +16,7 @@ -module(rabbit_upgrade). --export([maybe_upgrade_mnesia/0, maybe_upgrade_local/0]). +-export([maybe_upgrade_mnesia/0, maybe_upgrade_local/0, secondary_upgrade/1]). -include("rabbit.hrl"). @@ -122,6 +122,8 @@ remove_backup() -> maybe_upgrade_mnesia() -> AllNodes = rabbit_mnesia:cluster_nodes(all), + Mode = upgrade_mode(AllNodes), + ok = rabbit_mnesia_offline:maybe_complete_rename(Mode, AllNodes), case rabbit_version:upgrades_required(mnesia) of {error, starting_from_scratch} -> ok; @@ -138,7 +140,7 @@ maybe_upgrade_mnesia() -> ok; {ok, Upgrades} -> ensure_backup_taken(), - ok = case upgrade_mode(AllNodes) of + ok = case Mode of primary -> primary_upgrade(Upgrades, AllNodes); secondary -> secondary_upgrade(AllNodes) end -- cgit v1.2.1 From 6e84950b23c2a23157a96146e8c5d36e5009848d Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 27 Nov 2014 13:52:34 +0000 Subject: Don't be so keep to call upgrade_mode/1, it can die in all sorts of ways. --- src/rabbit_mnesia_offline.erl | 57 +++++++++++++++++++++++-------------------- src/rabbit_upgrade.erl | 8 +++--- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl index ae72462e..86b5c370 100644 --- a/src/rabbit_mnesia_offline.erl +++ b/src/rabbit_mnesia_offline.erl @@ -19,7 +19,7 @@ -export([rename_local_node/2]). -export([rename_remote_node/2]). --export([maybe_complete_rename/2]). +-export([maybe_complete_rename/1]). %%---------------------------------------------------------------------------- @@ -65,38 +65,43 @@ rename_local_node(FromNode, ToNode) -> stop_mnesia() end. -maybe_complete_rename(primary, _AllNodes) -> - case rabbit_file:read_term_file(rename_config_name()) of - {ok, [{_FromNode, _ToNode}]} -> - %% We are alone, restore the backup we previously took - ToBackup = to_backup_name(), - io:format(" * Loading backup '~s'~n", [ToBackup]), - ok = mnesia:install_fallback(ToBackup, [{scope, local}]), - start_mnesia(), - stop_mnesia(), - rabbit_file:delete(rename_config_name()), - rabbit_file:delete(from_backup_name()), - rabbit_file:delete(to_backup_name()), - ok; - _ -> - ok - end; +nodes_running(Nodes) -> + [N || N <- Nodes, rabbit:is_running(N)]. + -maybe_complete_rename(secondary, AllNodes) -> +maybe_complete_rename(AllNodes) -> case rabbit_file:read_term_file(rename_config_name()) of {ok, [{FromNode, ToNode}]} -> - rabbit_upgrade:secondary_upgrade(AllNodes), - [Another | _] = rabbit_mnesia:cluster_nodes(running) -- [node()], - ok = rpc:call(Another, ?MODULE, rename_remote_node, - [FromNode, ToNode]), - rabbit_file:delete(rename_config_name()), - rabbit_file:delete(from_backup_name()), - rabbit_file:delete(to_backup_name()), - ok; + case rabbit_upgrade:nodes_running(AllNodes) of + [] -> complete_rename_primary(); + _ -> complete_rename_secondary(FromNode, ToNode, AllNodes) + end; _ -> ok end. +complete_rename_primary() -> + %% We are alone, restore the backup we previously took + ToBackup = to_backup_name(), + io:format(" * Loading backup '~s'~n", [ToBackup]), + ok = mnesia:install_fallback(ToBackup, [{scope, local}]), + start_mnesia(), + stop_mnesia(), + rabbit_file:delete(rename_config_name()), + rabbit_file:delete(from_backup_name()), + rabbit_file:delete(to_backup_name()), + ok. + +complete_rename_secondary(FromNode, ToNode, AllNodes) -> + rabbit_upgrade:secondary_upgrade(AllNodes), + [Another | _] = rabbit_mnesia:cluster_nodes(running) -- [node()], + ok = rpc:call(Another, ?MODULE, rename_remote_node, + [FromNode, ToNode]), + rabbit_file:delete(rename_config_name()), + rabbit_file:delete(from_backup_name()), + rabbit_file:delete(to_backup_name()), + ok. + from_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-from". to_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-to". rename_config_name() -> rabbit_mnesia:dir() ++ "/rename-pending.config". diff --git a/src/rabbit_upgrade.erl b/src/rabbit_upgrade.erl index 420aa205..a1e116a0 100644 --- a/src/rabbit_upgrade.erl +++ b/src/rabbit_upgrade.erl @@ -16,7 +16,8 @@ -module(rabbit_upgrade). --export([maybe_upgrade_mnesia/0, maybe_upgrade_local/0, secondary_upgrade/1]). +-export([maybe_upgrade_mnesia/0, maybe_upgrade_local/0, + nodes_running/1, secondary_upgrade/1]). -include("rabbit.hrl"). @@ -122,8 +123,7 @@ remove_backup() -> maybe_upgrade_mnesia() -> AllNodes = rabbit_mnesia:cluster_nodes(all), - Mode = upgrade_mode(AllNodes), - ok = rabbit_mnesia_offline:maybe_complete_rename(Mode, AllNodes), + ok = rabbit_mnesia_offline:maybe_complete_rename(AllNodes), case rabbit_version:upgrades_required(mnesia) of {error, starting_from_scratch} -> ok; @@ -140,7 +140,7 @@ maybe_upgrade_mnesia() -> ok; {ok, Upgrades} -> ensure_backup_taken(), - ok = case Mode of + ok = case upgrade_mode(AllNodes) of primary -> primary_upgrade(Upgrades, AllNodes); secondary -> secondary_upgrade(AllNodes) end -- cgit v1.2.1 From c88c4fbadf0cb1586386e1b4d2abf14427f4dc2c Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Thu, 27 Nov 2014 14:54:47 +0000 Subject: Support renaming other remote nodes at the same time, in case we want to do a big bang. --- src/rabbit_control_main.erl | 6 +- src/rabbit_mnesia_offline.erl | 131 +++++++++++++++++++++--------------------- 2 files changed, 69 insertions(+), 68 deletions(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index b19971fc..21c4e4c3 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -235,11 +235,13 @@ action(forget_cluster_node, Node, [ClusterNodeS], Opts, Inform) -> [ClusterNode, false]) end; -action(rename_current_node, _Node, [FromNodeS, ToNodeS], _Opts, Inform) -> +action(rename_current_node, _Node, [FromNodeS, ToNodeS | OthersS], + _Opts, Inform) -> + Others = [list_to_atom(N) || N <- OthersS], FromNode = list_to_atom(FromNodeS), ToNode = list_to_atom(ToNodeS), Inform("Renaming local cluster node ~s to ~s", [FromNode, ToNode]), - rabbit_mnesia_offline:rename_local_node(FromNode, ToNode); + rabbit_mnesia_offline:rename_local_node(FromNode, ToNode, Others); action(force_boot, Node, [], _Opts, Inform) -> Inform("Forcing boot for Mnesia dir ~s", [mnesia:system_info(directory)]), diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl index 86b5c370..eb22bc14 100644 --- a/src/rabbit_mnesia_offline.erl +++ b/src/rabbit_mnesia_offline.erl @@ -17,7 +17,7 @@ -module(rabbit_mnesia_offline). -include("rabbit.hrl"). --export([rename_local_node/2]). +-export([rename_local_node/3]). -export([rename_remote_node/2]). -export([maybe_complete_rename/1]). @@ -25,14 +25,15 @@ -ifdef(use_specs). --spec(rename_local_node/2 :: (node(), node()) -> 'ok'). +-spec(rename_local_node/3 :: (node(), node(), [node()]) -> 'ok'). -spec(rename_remote_node/2 :: (node(), node()) -> 'ok'). -endif. %%---------------------------------------------------------------------------- -rename_local_node(FromNode, ToNode) -> +rename_local_node(FromNode, ToNode, Others) -> + NodeMap = dict:from_list(split_others([FromNode, ToNode | Others])), try rabbit_control_main:become(FromNode), rabbit_log:info("Renaming node ~s to ~s~n", [FromNode, ToNode]), @@ -50,24 +51,23 @@ rename_local_node(FromNode, ToNode) -> io:format(" * Backing up to '~s'~n", [FromBackup]), ok = mnesia:backup(FromBackup), stop_mnesia(), - rabbit_control_main:become(ToNode), io:format(" * Converting backup '~s'~n", [ToBackup]), - convert_backup(FromNode, ToNode, FromBackup, ToBackup), + convert_backup(NodeMap, FromBackup, ToBackup), ok = rabbit_file:write_term_file(rename_config_name(), [{FromNode, ToNode}]), io:format(" * Converting config files~n", []), - convert_config_file(FromNode, ToNode, + convert_config_file(NodeMap, rabbit_node_monitor:running_nodes_filename()), - convert_config_file(FromNode, ToNode, + convert_config_file(NodeMap, rabbit_node_monitor:cluster_status_filename()), ok after stop_mnesia() end. -nodes_running(Nodes) -> - [N || N <- Nodes, rabbit:is_running(N)]. - +split_others([]) -> []; +split_others([_]) -> exit(even_list_needed); +split_others([A, B | T]) -> [{A, B} | split_others(T)]. maybe_complete_rename(AllNodes) -> case rabbit_file:read_term_file(rename_config_name()) of @@ -109,47 +109,42 @@ rename_config_name() -> rabbit_mnesia:dir() ++ "/rename-pending.config". start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). stop_mnesia() -> stopped = mnesia:stop(). -convert_backup(FromNode, ToNode, FromBackup, ToBackup) -> - Switch = fun - (Node) when Node == FromNode -> - ToNode; - (Node) when Node == ToNode -> - throw({error, {already_exists, Node}}); - (Node) -> - Node +convert_backup(NodeMap, FromBackup, ToBackup) -> + Switch = fun (OldNode) -> + lookup_node(OldNode, NodeMap) end, Convert = fun %% TODO do we ever hit these three heads? - ({schema, db_nodes, Nodes}, Acc) -> - io:format(" +++ db_nodes ~p~n", [Nodes]), - {[{schema, db_nodes, lists:map(Switch,Nodes)}], Acc}; - ({schema, version, Version}, Acc) -> - io:format(" +++ version: ~p~n", [Version]), - {[{schema, version, Version}], Acc}; - ({schema, cookie, Cookie}, Acc) -> - io:format(" +++ cookie: ~p~n", [Cookie]), - {[{schema, cookie, Cookie}], Acc}; - ({schema, Tab, CreateList}, Acc) -> - %% io:format("~n * Checking table: '~p'~n", [Tab]), - %%io:format(" . Initial content: ~p~n", [CreateList]), - Keys = [ram_copies, disc_copies, disc_only_copies], - OptSwitch = - fun({Key, Val}) -> - case lists:member(Key, Keys) of - true -> - %%io:format(" + Checking key: '~p'~n", [Key]), - {Key, lists:map(Switch, Val)}; - false-> - {Key, Val} - end - end, - Res = {[{schema, Tab, lists:map(OptSwitch, CreateList)}], Acc}, - %%io:format(" . Resulting content: ~p~n", [Res]), - Res; + %% ({schema, db_nodes, Nodes}, Acc) -> + %% io:format(" +++ db_nodes ~p~n", [Nodes]), + %% {[{schema, db_nodes, lists:map(Switch,Nodes)}], Acc}; + %% ({schema, version, Version}, Acc) -> + %% io:format(" +++ version: ~p~n", [Version]), + %% {[{schema, version, Version}], Acc}; + %% ({schema, cookie, Cookie}, Acc) -> + %% io:format(" +++ cookie: ~p~n", [Cookie]), + %% {[{schema, cookie, Cookie}], Acc}; + %% ({schema, Tab, CreateList}, Acc) -> + %% io:format("~n * Checking table: '~p'~n", [Tab]), + %% io:format(" . Initial content: ~p~n", [CreateList]), + %% Keys = [ram_copies, disc_copies, disc_only_copies], + %% OptSwitch = + %% fun({Key, Val}) -> + %% case lists:member(Key, Keys) of + %% true -> + %% %%io:format(" + Checking key: '~p'~n", [Key]), + %% {Key, lists:map(Switch, Val)}; + %% false-> + %% {Key, Val} + %% end + %% end, + %% Res = {[{schema, Tab, lists:map(OptSwitch, CreateList)}], Acc}, + %% io:format(" . Resulting content: ~p~n", [Res]), + %% Res; (Other, Acc) -> - case lists:member(element(1, Other), [rabbit_durable_queue]) of - true -> Other1 = update_term(FromNode, ToNode, Other), + case lists:member(element(1, Other), [schema, rabbit_durable_queue]) of + true -> Other1 = update_term(NodeMap, Other), io:format(" --- ~p~n +++ ~p~n", [Other, Other1]), {[Other1], Acc}; false -> {[Other], Acc} @@ -157,19 +152,25 @@ convert_backup(FromNode, ToNode, FromBackup, ToBackup) -> end, mnesia:traverse_backup(FromBackup, ToBackup, Convert, switched). -convert_config_file(FromNode, ToNode, Path) -> +convert_config_file(NodeMap, Path) -> {ok, Term} = rabbit_file:read_term_file(Path), - ok = rabbit_file:write_term_file(Path, update_term(FromNode, ToNode, Term)). - -update_term(N1, N2, L) when is_list(L) -> - [update_term(N1, N2, I) || I <- L]; -update_term(N1, N2, T) when is_tuple(T) -> - list_to_tuple(update_term(N1, N2, tuple_to_list(T))); -update_term(N1, N2, N1) -> - N2; -update_term(N1, N2, Pid) when is_pid(Pid), node(Pid) == N1 -> - rabbit_misc:pid_change_node(Pid, N2); -update_term(_N1, _N2, Term) -> + ok = rabbit_file:write_term_file(Path, update_term(NodeMap, Term)). + +lookup_node(OldNode, NodeMap) -> + case dict:find(OldNode, NodeMap) of + {ok, NewNode} -> NewNode; + error -> OldNode + end. + +update_term(NodeMap, L) when is_list(L) -> + [update_term(NodeMap, I) || I <- L]; +update_term(NodeMap, T) when is_tuple(T) -> + list_to_tuple(update_term(NodeMap, tuple_to_list(T))); +update_term(NodeMap, Node) when is_atom(Node) -> + lookup_node(Node, NodeMap); +update_term(NodeMap, Pid) when is_pid(Pid) -> + rabbit_misc:pid_change_node(Pid, lookup_node(node(Pid), NodeMap)); +update_term(_NodeMap, Term) -> Term. %%---------------------------------------------------------------------------- @@ -177,18 +178,16 @@ update_term(_N1, _N2, Term) -> rename_remote_node(FromNode, ToNode) -> All = rabbit_mnesia:cluster_nodes(all), Running = rabbit_mnesia:cluster_nodes(running), - case {lists:member(FromNode, All), - lists:member(FromNode, Running), - lists:member(ToNode, All)} of - {true, false, true} -> ok; - {false, _, _} -> exit({old_node_not_in_cluster, FromNode}); - {_, true, _} -> exit({old_node_running, FromNode}); - {_, _, false} -> exit({new_node_not_in_cluster, ToNode}) + case {lists:member(FromNode, Running), lists:member(ToNode, All)} of + {false, true} -> ok; + {true, _} -> exit({old_node_running, FromNode}); + {_, false} -> exit({new_node_not_in_cluster, ToNode}) end, mnesia:del_table_copy(schema, FromNode), + NodeMap = dict:from_list([{FromNode, ToNode}]), {atomic, ok} = mnesia:transform_table( rabbit_durable_queue, - fun (Q) -> update_term(FromNode, ToNode, Q) end, + fun (Q) -> update_term(NodeMap, Q) end, record_info(fields, amqqueue)), ok. -- cgit v1.2.1 From 16236cc49676cf8ddf1eff68f7d293ff4963233c Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 15:57:21 +0000 Subject: We are presumably the source of truth now, so make sure we boot. --- src/rabbit_mnesia_offline.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl index eb22bc14..efbfbbcc 100644 --- a/src/rabbit_mnesia_offline.erl +++ b/src/rabbit_mnesia_offline.erl @@ -90,6 +90,7 @@ complete_rename_primary() -> rabbit_file:delete(rename_config_name()), rabbit_file:delete(from_backup_name()), rabbit_file:delete(to_backup_name()), + rabbit_mnesia:force_load_next_boot(), ok. complete_rename_secondary(FromNode, ToNode, AllNodes) -> -- cgit v1.2.1 From cf05604306a8e36055746524cb2ece48e0434bb0 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 16:06:34 +0000 Subject: Remove commented out bit, remove pointless RPC (we're in the cluster by that point, we can make Mnesia changes ourselves). --- src/rabbit_mnesia_offline.erl | 60 ++++++++++--------------------------------- 1 file changed, 13 insertions(+), 47 deletions(-) diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl index efbfbbcc..7c610f17 100644 --- a/src/rabbit_mnesia_offline.erl +++ b/src/rabbit_mnesia_offline.erl @@ -18,15 +18,16 @@ -include("rabbit.hrl"). -export([rename_local_node/3]). --export([rename_remote_node/2]). -export([maybe_complete_rename/1]). +-define(CONVERT_TABLES, [schema, rabbit_durable_queue]). + %%---------------------------------------------------------------------------- -ifdef(use_specs). -spec(rename_local_node/3 :: (node(), node(), [node()]) -> 'ok'). --spec(rename_remote_node/2 :: (node(), node()) -> 'ok'). +-spec(maybe_complete_rename/1 :: ([node()]) -> 'ok'). -endif. @@ -95,9 +96,7 @@ complete_rename_primary() -> complete_rename_secondary(FromNode, ToNode, AllNodes) -> rabbit_upgrade:secondary_upgrade(AllNodes), - [Another | _] = rabbit_mnesia:cluster_nodes(running) -- [node()], - ok = rpc:call(Another, ?MODULE, rename_remote_node, - [FromNode, ToNode]), + rename_remote_node(FromNode, ToNode), rabbit_file:delete(rename_config_name()), rabbit_file:delete(from_backup_name()), rabbit_file:delete(to_backup_name()), @@ -111,47 +110,15 @@ start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). stop_mnesia() -> stopped = mnesia:stop(). convert_backup(NodeMap, FromBackup, ToBackup) -> - Switch = fun (OldNode) -> - lookup_node(OldNode, NodeMap) - end, - Convert = - fun - %% TODO do we ever hit these three heads? - %% ({schema, db_nodes, Nodes}, Acc) -> - %% io:format(" +++ db_nodes ~p~n", [Nodes]), - %% {[{schema, db_nodes, lists:map(Switch,Nodes)}], Acc}; - %% ({schema, version, Version}, Acc) -> - %% io:format(" +++ version: ~p~n", [Version]), - %% {[{schema, version, Version}], Acc}; - %% ({schema, cookie, Cookie}, Acc) -> - %% io:format(" +++ cookie: ~p~n", [Cookie]), - %% {[{schema, cookie, Cookie}], Acc}; - %% ({schema, Tab, CreateList}, Acc) -> - %% io:format("~n * Checking table: '~p'~n", [Tab]), - %% io:format(" . Initial content: ~p~n", [CreateList]), - %% Keys = [ram_copies, disc_copies, disc_only_copies], - %% OptSwitch = - %% fun({Key, Val}) -> - %% case lists:member(Key, Keys) of - %% true -> - %% %%io:format(" + Checking key: '~p'~n", [Key]), - %% {Key, lists:map(Switch, Val)}; - %% false-> - %% {Key, Val} - %% end - %% end, - %% Res = {[{schema, Tab, lists:map(OptSwitch, CreateList)}], Acc}, - %% io:format(" . Resulting content: ~p~n", [Res]), - %% Res; - (Other, Acc) -> - case lists:member(element(1, Other), [schema, rabbit_durable_queue]) of - true -> Other1 = update_term(NodeMap, Other), - io:format(" --- ~p~n +++ ~p~n", [Other, Other1]), - {[Other1], Acc}; - false -> {[Other], Acc} - end - end, - mnesia:traverse_backup(FromBackup, ToBackup, Convert, switched). + mnesia:traverse_backup( + FromBackup, ToBackup, + fun + (Row, Acc) -> + case lists:member(element(1, Row), ?CONVERT_TABLES) of + true -> {[update_term(NodeMap, Row)], Acc}; + false -> {[Row], Acc} + end + end, switched). convert_config_file(NodeMap, Path) -> {ok, Term} = rabbit_file:read_term_file(Path), @@ -191,4 +158,3 @@ rename_remote_node(FromNode, ToNode) -> fun (Q) -> update_term(NodeMap, Q) end, record_info(fields, amqqueue)), ok. - -- cgit v1.2.1 From 96514d234b7cf974be41eddfd8e1f25400b8dfe4 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 16:13:19 +0000 Subject: Rename things --- src/rabbit_control_main.erl | 9 ++- src/rabbit_mnesia_offline.erl | 160 ------------------------------------------ src/rabbit_mnesia_rename.erl | 160 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 165 deletions(-) delete mode 100644 src/rabbit_mnesia_offline.erl create mode 100644 src/rabbit_mnesia_rename.erl diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index 21c4e4c3..c00c694c 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -40,7 +40,7 @@ change_cluster_node_type, update_cluster_nodes, {forget_cluster_node, [?OFFLINE_DEF]}, - rename_current_node, + rename_node, force_boot, cluster_status, {sync_queue, [?VHOST_DEF]}, @@ -105,7 +105,7 @@ -define(COMMANDS_NOT_REQUIRING_APP, [stop, stop_app, start_app, wait, reset, force_reset, rotate_logs, join_cluster, change_cluster_node_type, update_cluster_nodes, - forget_cluster_node, rename_current_node, cluster_status, status, + forget_cluster_node, rename_node, cluster_status, status, environment, eval, force_boot]). %%---------------------------------------------------------------------------- @@ -235,13 +235,12 @@ action(forget_cluster_node, Node, [ClusterNodeS], Opts, Inform) -> [ClusterNode, false]) end; -action(rename_current_node, _Node, [FromNodeS, ToNodeS | OthersS], - _Opts, Inform) -> +action(rename_node, _Node, [FromNodeS, ToNodeS | OthersS], _Opts, Inform) -> Others = [list_to_atom(N) || N <- OthersS], FromNode = list_to_atom(FromNodeS), ToNode = list_to_atom(ToNodeS), Inform("Renaming local cluster node ~s to ~s", [FromNode, ToNode]), - rabbit_mnesia_offline:rename_local_node(FromNode, ToNode, Others); + rabbit_mnesia_rename:rename(FromNode, ToNode, Others); action(force_boot, Node, [], _Opts, Inform) -> Inform("Forcing boot for Mnesia dir ~s", [mnesia:system_info(directory)]), diff --git a/src/rabbit_mnesia_offline.erl b/src/rabbit_mnesia_offline.erl deleted file mode 100644 index 7c610f17..00000000 --- a/src/rabbit_mnesia_offline.erl +++ /dev/null @@ -1,160 +0,0 @@ -%% The contents of this file are subject to the Mozilla Public License -%% Version 1.1 (the "License"); you may not use this file except in -%% compliance with the License. You may obtain a copy of the License -%% at http://www.mozilla.org/MPL/ -%% -%% Software distributed under the License is distributed on an "AS IS" -%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See -%% the License for the specific language governing rights and -%% limitations under the License. -%% -%% The Original Code is RabbitMQ. -%% -%% The Initial Developer of the Original Code is GoPivotal, Inc. -%% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. -%% - --module(rabbit_mnesia_offline). --include("rabbit.hrl"). - --export([rename_local_node/3]). --export([maybe_complete_rename/1]). - --define(CONVERT_TABLES, [schema, rabbit_durable_queue]). - -%%---------------------------------------------------------------------------- - --ifdef(use_specs). - --spec(rename_local_node/3 :: (node(), node(), [node()]) -> 'ok'). --spec(maybe_complete_rename/1 :: ([node()]) -> 'ok'). - --endif. - -%%---------------------------------------------------------------------------- - -rename_local_node(FromNode, ToNode, Others) -> - NodeMap = dict:from_list(split_others([FromNode, ToNode | Others])), - try - rabbit_control_main:become(FromNode), - rabbit_log:info("Renaming node ~s to ~s~n", [FromNode, ToNode]), - start_mnesia(), - Nodes = rabbit_mnesia:cluster_nodes(all), - case {lists:member(FromNode, Nodes), lists:member(ToNode, Nodes)} of - {true, false} -> ok; - {false, _} -> exit({node_not_in_cluster, FromNode}); - {_, true} -> exit({node_already_in_cluster, ToNode}) - end, - rabbit_table:force_load(), - rabbit_table:wait_for_replicated(), - FromBackup = from_backup_name(), - ToBackup = to_backup_name(), - io:format(" * Backing up to '~s'~n", [FromBackup]), - ok = mnesia:backup(FromBackup), - stop_mnesia(), - io:format(" * Converting backup '~s'~n", [ToBackup]), - convert_backup(NodeMap, FromBackup, ToBackup), - ok = rabbit_file:write_term_file(rename_config_name(), - [{FromNode, ToNode}]), - io:format(" * Converting config files~n", []), - convert_config_file(NodeMap, - rabbit_node_monitor:running_nodes_filename()), - convert_config_file(NodeMap, - rabbit_node_monitor:cluster_status_filename()), - ok - after - stop_mnesia() - end. - -split_others([]) -> []; -split_others([_]) -> exit(even_list_needed); -split_others([A, B | T]) -> [{A, B} | split_others(T)]. - -maybe_complete_rename(AllNodes) -> - case rabbit_file:read_term_file(rename_config_name()) of - {ok, [{FromNode, ToNode}]} -> - case rabbit_upgrade:nodes_running(AllNodes) of - [] -> complete_rename_primary(); - _ -> complete_rename_secondary(FromNode, ToNode, AllNodes) - end; - _ -> - ok - end. - -complete_rename_primary() -> - %% We are alone, restore the backup we previously took - ToBackup = to_backup_name(), - io:format(" * Loading backup '~s'~n", [ToBackup]), - ok = mnesia:install_fallback(ToBackup, [{scope, local}]), - start_mnesia(), - stop_mnesia(), - rabbit_file:delete(rename_config_name()), - rabbit_file:delete(from_backup_name()), - rabbit_file:delete(to_backup_name()), - rabbit_mnesia:force_load_next_boot(), - ok. - -complete_rename_secondary(FromNode, ToNode, AllNodes) -> - rabbit_upgrade:secondary_upgrade(AllNodes), - rename_remote_node(FromNode, ToNode), - rabbit_file:delete(rename_config_name()), - rabbit_file:delete(from_backup_name()), - rabbit_file:delete(to_backup_name()), - ok. - -from_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-from". -to_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-to". -rename_config_name() -> rabbit_mnesia:dir() ++ "/rename-pending.config". - -start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). -stop_mnesia() -> stopped = mnesia:stop(). - -convert_backup(NodeMap, FromBackup, ToBackup) -> - mnesia:traverse_backup( - FromBackup, ToBackup, - fun - (Row, Acc) -> - case lists:member(element(1, Row), ?CONVERT_TABLES) of - true -> {[update_term(NodeMap, Row)], Acc}; - false -> {[Row], Acc} - end - end, switched). - -convert_config_file(NodeMap, Path) -> - {ok, Term} = rabbit_file:read_term_file(Path), - ok = rabbit_file:write_term_file(Path, update_term(NodeMap, Term)). - -lookup_node(OldNode, NodeMap) -> - case dict:find(OldNode, NodeMap) of - {ok, NewNode} -> NewNode; - error -> OldNode - end. - -update_term(NodeMap, L) when is_list(L) -> - [update_term(NodeMap, I) || I <- L]; -update_term(NodeMap, T) when is_tuple(T) -> - list_to_tuple(update_term(NodeMap, tuple_to_list(T))); -update_term(NodeMap, Node) when is_atom(Node) -> - lookup_node(Node, NodeMap); -update_term(NodeMap, Pid) when is_pid(Pid) -> - rabbit_misc:pid_change_node(Pid, lookup_node(node(Pid), NodeMap)); -update_term(_NodeMap, Term) -> - Term. - -%%---------------------------------------------------------------------------- - -rename_remote_node(FromNode, ToNode) -> - All = rabbit_mnesia:cluster_nodes(all), - Running = rabbit_mnesia:cluster_nodes(running), - case {lists:member(FromNode, Running), lists:member(ToNode, All)} of - {false, true} -> ok; - {true, _} -> exit({old_node_running, FromNode}); - {_, false} -> exit({new_node_not_in_cluster, ToNode}) - end, - mnesia:del_table_copy(schema, FromNode), - NodeMap = dict:from_list([{FromNode, ToNode}]), - {atomic, ok} = mnesia:transform_table( - rabbit_durable_queue, - fun (Q) -> update_term(NodeMap, Q) end, - record_info(fields, amqqueue)), - ok. diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl new file mode 100644 index 00000000..ef3854de --- /dev/null +++ b/src/rabbit_mnesia_rename.erl @@ -0,0 +1,160 @@ +%% The contents of this file are subject to the Mozilla Public License +%% Version 1.1 (the "License"); you may not use this file except in +%% compliance with the License. You may obtain a copy of the License +%% at http://www.mozilla.org/MPL/ +%% +%% Software distributed under the License is distributed on an "AS IS" +%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See +%% the License for the specific language governing rights and +%% limitations under the License. +%% +%% The Original Code is RabbitMQ. +%% +%% The Initial Developer of the Original Code is GoPivotal, Inc. +%% Copyright (c) 2007-2014 GoPivotal, Inc. All rights reserved. +%% + +-module(rabbit_mnesia_rename). +-include("rabbit.hrl"). + +-export([rename/3]). +-export([maybe_complete_rename/1]). + +-define(CONVERT_TABLES, [schema, rabbit_durable_queue]). + +%%---------------------------------------------------------------------------- + +-ifdef(use_specs). + +-spec(rename/3 :: (node(), node(), [node()]) -> 'ok'). +-spec(maybe_complete_rename/1 :: ([node()]) -> 'ok'). + +-endif. + +%%---------------------------------------------------------------------------- + +rename(FromNode, ToNode, Others) -> + NodeMap = dict:from_list(split_others([FromNode, ToNode | Others])), + try + rabbit_control_main:become(FromNode), + rabbit_log:info("Renaming node ~s to ~s~n", [FromNode, ToNode]), + start_mnesia(), + Nodes = rabbit_mnesia:cluster_nodes(all), + case {lists:member(FromNode, Nodes), lists:member(ToNode, Nodes)} of + {true, false} -> ok; + {false, _} -> exit({node_not_in_cluster, FromNode}); + {_, true} -> exit({node_already_in_cluster, ToNode}) + end, + rabbit_table:force_load(), + rabbit_table:wait_for_replicated(), + FromBackup = from_backup_name(), + ToBackup = to_backup_name(), + io:format(" * Backing up to '~s'~n", [FromBackup]), + ok = mnesia:backup(FromBackup), + stop_mnesia(), + io:format(" * Converting backup '~s'~n", [ToBackup]), + convert_backup(NodeMap, FromBackup, ToBackup), + ok = rabbit_file:write_term_file(rename_config_name(), + [{FromNode, ToNode}]), + io:format(" * Converting config files~n", []), + convert_config_file(NodeMap, + rabbit_node_monitor:running_nodes_filename()), + convert_config_file(NodeMap, + rabbit_node_monitor:cluster_status_filename()), + ok + after + stop_mnesia() + end. + +split_others([]) -> []; +split_others([_]) -> exit(even_list_needed); +split_others([A, B | T]) -> [{A, B} | split_others(T)]. + +maybe_complete_rename(AllNodes) -> + case rabbit_file:read_term_file(rename_config_name()) of + {ok, [{FromNode, ToNode}]} -> + case rabbit_upgrade:nodes_running(AllNodes) of + [] -> complete_rename_primary(); + _ -> complete_rename_secondary(FromNode, ToNode, AllNodes) + end; + _ -> + ok + end. + +complete_rename_primary() -> + %% We are alone, restore the backup we previously took + ToBackup = to_backup_name(), + io:format(" * Loading backup '~s'~n", [ToBackup]), + ok = mnesia:install_fallback(ToBackup, [{scope, local}]), + start_mnesia(), + stop_mnesia(), + rabbit_file:delete(rename_config_name()), + rabbit_file:delete(from_backup_name()), + rabbit_file:delete(to_backup_name()), + rabbit_mnesia:force_load_next_boot(), + ok. + +complete_rename_secondary(FromNode, ToNode, AllNodes) -> + rabbit_upgrade:secondary_upgrade(AllNodes), + rename_remote_node(FromNode, ToNode), + rabbit_file:delete(rename_config_name()), + rabbit_file:delete(from_backup_name()), + rabbit_file:delete(to_backup_name()), + ok. + +from_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-from". +to_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-to". +rename_config_name() -> rabbit_mnesia:dir() ++ "/rename-pending.config". + +start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). +stop_mnesia() -> stopped = mnesia:stop(). + +convert_backup(NodeMap, FromBackup, ToBackup) -> + mnesia:traverse_backup( + FromBackup, ToBackup, + fun + (Row, Acc) -> + case lists:member(element(1, Row), ?CONVERT_TABLES) of + true -> {[update_term(NodeMap, Row)], Acc}; + false -> {[Row], Acc} + end + end, switched). + +convert_config_file(NodeMap, Path) -> + {ok, Term} = rabbit_file:read_term_file(Path), + ok = rabbit_file:write_term_file(Path, update_term(NodeMap, Term)). + +lookup_node(OldNode, NodeMap) -> + case dict:find(OldNode, NodeMap) of + {ok, NewNode} -> NewNode; + error -> OldNode + end. + +update_term(NodeMap, L) when is_list(L) -> + [update_term(NodeMap, I) || I <- L]; +update_term(NodeMap, T) when is_tuple(T) -> + list_to_tuple(update_term(NodeMap, tuple_to_list(T))); +update_term(NodeMap, Node) when is_atom(Node) -> + lookup_node(Node, NodeMap); +update_term(NodeMap, Pid) when is_pid(Pid) -> + rabbit_misc:pid_change_node(Pid, lookup_node(node(Pid), NodeMap)); +update_term(_NodeMap, Term) -> + Term. + +%%---------------------------------------------------------------------------- + +rename_remote_node(FromNode, ToNode) -> + All = rabbit_mnesia:cluster_nodes(all), + Running = rabbit_mnesia:cluster_nodes(running), + case {lists:member(FromNode, Running), lists:member(ToNode, All)} of + {false, true} -> ok; + {true, _} -> exit({old_node_running, FromNode}); + {_, false} -> exit({new_node_not_in_cluster, ToNode}) + end, + mnesia:del_table_copy(schema, FromNode), + NodeMap = dict:from_list([{FromNode, ToNode}]), + {atomic, ok} = mnesia:transform_table( + rabbit_durable_queue, + fun (Q) -> update_term(NodeMap, Q) end, + record_info(fields, amqqueue)), + ok. -- cgit v1.2.1 From 1fdcb50ee3987fbfafc800a43469e3b3346a7b23 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 16:24:34 +0000 Subject: Oops --- src/rabbit_upgrade.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rabbit_upgrade.erl b/src/rabbit_upgrade.erl index a1e116a0..848d86c7 100644 --- a/src/rabbit_upgrade.erl +++ b/src/rabbit_upgrade.erl @@ -123,7 +123,7 @@ remove_backup() -> maybe_upgrade_mnesia() -> AllNodes = rabbit_mnesia:cluster_nodes(all), - ok = rabbit_mnesia_offline:maybe_complete_rename(AllNodes), + ok = rabbit_mnesia_rename:maybe_complete_rename(AllNodes), case rabbit_version:upgrades_required(mnesia) of {error, starting_from_scratch} -> ok; -- cgit v1.2.1 From 631b3783e66caa8cc47bfe2569d9af52b9ae9978 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 17:04:36 +0000 Subject: Improve the chance of not losing a bunch of data. --- src/rabbit_mnesia_rename.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index ef3854de..80efeee7 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -87,6 +87,7 @@ complete_rename_primary() -> io:format(" * Loading backup '~s'~n", [ToBackup]), ok = mnesia:install_fallback(ToBackup, [{scope, local}]), start_mnesia(), + rabbit_table:wait_for_replicated(), stop_mnesia(), rabbit_file:delete(rename_config_name()), rabbit_file:delete(from_backup_name()), -- cgit v1.2.1 From 48d9b9c3455f6be811cd64b769bfb193bb4f873e Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 17:04:46 +0000 Subject: Less io:format, more rabbit_log. --- src/rabbit_mnesia_rename.erl | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index 80efeee7..8dbf14a2 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -37,7 +37,6 @@ rename(FromNode, ToNode, Others) -> NodeMap = dict:from_list(split_others([FromNode, ToNode | Others])), try rabbit_control_main:become(FromNode), - rabbit_log:info("Renaming node ~s to ~s~n", [FromNode, ToNode]), start_mnesia(), Nodes = rabbit_mnesia:cluster_nodes(all), case {lists:member(FromNode, Nodes), lists:member(ToNode, Nodes)} of @@ -49,14 +48,11 @@ rename(FromNode, ToNode, Others) -> rabbit_table:wait_for_replicated(), FromBackup = from_backup_name(), ToBackup = to_backup_name(), - io:format(" * Backing up to '~s'~n", [FromBackup]), ok = mnesia:backup(FromBackup), stop_mnesia(), - io:format(" * Converting backup '~s'~n", [ToBackup]), convert_backup(NodeMap, FromBackup, ToBackup), ok = rabbit_file:write_term_file(rename_config_name(), [{FromNode, ToNode}]), - io:format(" * Converting config files~n", []), convert_config_file(NodeMap, rabbit_node_monitor:running_nodes_filename()), convert_config_file(NodeMap, @@ -74,17 +70,18 @@ maybe_complete_rename(AllNodes) -> case rabbit_file:read_term_file(rename_config_name()) of {ok, [{FromNode, ToNode}]} -> case rabbit_upgrade:nodes_running(AllNodes) of - [] -> complete_rename_primary(); + [] -> complete_rename_primary(FromNode, ToNode); _ -> complete_rename_secondary(FromNode, ToNode, AllNodes) end; _ -> ok end. -complete_rename_primary() -> +complete_rename_primary(FromNode, ToNode) -> %% We are alone, restore the backup we previously took ToBackup = to_backup_name(), - io:format(" * Loading backup '~s'~n", [ToBackup]), + rabbit_log:info("Restarting as primary after rename from ~s to ~s~n", + [FromNode, ToNode]), ok = mnesia:install_fallback(ToBackup, [{scope, local}]), start_mnesia(), rabbit_table:wait_for_replicated(), @@ -96,6 +93,8 @@ complete_rename_primary() -> ok. complete_rename_secondary(FromNode, ToNode, AllNodes) -> + rabbit_log:info("Restarting as secondary after rename from ~s to ~s~n", + [FromNode, ToNode]), rabbit_upgrade:secondary_upgrade(AllNodes), rename_remote_node(FromNode, ToNode), rabbit_file:delete(rename_config_name()), -- cgit v1.2.1 From d6001abb7c94e5d94eb54e096f40c76249268751 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 17:08:30 +0000 Subject: Detect starting as the old node name after a rename and roll back. Also rename a bit. --- src/rabbit_mnesia_rename.erl | 68 ++++++++++++++++++++++++++++---------------- src/rabbit_upgrade.erl | 2 +- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index 8dbf14a2..e7c163e1 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -18,7 +18,7 @@ -include("rabbit.hrl"). -export([rename/3]). --export([maybe_complete_rename/1]). +-export([maybe_finish/1]). -define(CONVERT_TABLES, [schema, rabbit_durable_queue]). @@ -27,7 +27,7 @@ -ifdef(use_specs). -spec(rename/3 :: (node(), node(), [node()]) -> 'ok'). --spec(maybe_complete_rename/1 :: ([node()]) -> 'ok'). +-spec(maybe_finish/1 :: ([node()]) -> 'ok'). -endif. @@ -53,10 +53,7 @@ rename(FromNode, ToNode, Others) -> convert_backup(NodeMap, FromBackup, ToBackup), ok = rabbit_file:write_term_file(rename_config_name(), [{FromNode, ToNode}]), - convert_config_file(NodeMap, - rabbit_node_monitor:running_nodes_filename()), - convert_config_file(NodeMap, - rabbit_node_monitor:cluster_status_filename()), + convert_config_files(NodeMap), ok after stop_mnesia() @@ -66,41 +63,58 @@ split_others([]) -> []; split_others([_]) -> exit(even_list_needed); split_others([A, B | T]) -> [{A, B} | split_others(T)]. -maybe_complete_rename(AllNodes) -> +maybe_finish(AllNodes) -> case rabbit_file:read_term_file(rename_config_name()) of - {ok, [{FromNode, ToNode}]} -> + {ok, [{FromNode, ToNode}]} -> finish(FromNode, ToNode, AllNodes); + _ -> ok + end. + +finish(FromNode, ToNode, AllNodes) -> + case node() of + ToNode -> case rabbit_upgrade:nodes_running(AllNodes) of - [] -> complete_rename_primary(FromNode, ToNode); - _ -> complete_rename_secondary(FromNode, ToNode, AllNodes) + [] -> finish_primary(FromNode, ToNode); + _ -> finish_secondary(FromNode, ToNode, AllNodes) end; + FromNode -> + rabbit_log:info( + "Abandoning rename from ~s to ~s since we are still ~s~n", + [FromNode, ToNode, FromNode]), + convert_config_files(mini_map(ToNode, FromNode)), + delete_rename_files(); _ -> - ok + %% Boot will almost certainly fail but we might as + %% well just log this + rabbit_log:info( + "Rename attempted from ~s to ~s but we are ~s - ignoring.~n", + [FromNode, ToNode, node()]) end. -complete_rename_primary(FromNode, ToNode) -> - %% We are alone, restore the backup we previously took - ToBackup = to_backup_name(), +finish_primary(FromNode, ToNode) -> rabbit_log:info("Restarting as primary after rename from ~s to ~s~n", [FromNode, ToNode]), + %% We are alone, restore the backup we previously took + ToBackup = to_backup_name(), ok = mnesia:install_fallback(ToBackup, [{scope, local}]), start_mnesia(), rabbit_table:wait_for_replicated(), stop_mnesia(), - rabbit_file:delete(rename_config_name()), - rabbit_file:delete(from_backup_name()), - rabbit_file:delete(to_backup_name()), + delete_rename_files(), rabbit_mnesia:force_load_next_boot(), ok. -complete_rename_secondary(FromNode, ToNode, AllNodes) -> +finish_secondary(FromNode, ToNode, AllNodes) -> rabbit_log:info("Restarting as secondary after rename from ~s to ~s~n", [FromNode, ToNode]), rabbit_upgrade:secondary_upgrade(AllNodes), rename_remote_node(FromNode, ToNode), + delete_rename_files(), + ok. + +delete_rename_files() -> rabbit_file:delete(rename_config_name()), rabbit_file:delete(from_backup_name()), - rabbit_file:delete(to_backup_name()), - ok. + rabbit_file:delete(to_backup_name()). from_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-from". to_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-to". @@ -120,6 +134,11 @@ convert_backup(NodeMap, FromBackup, ToBackup) -> end end, switched). +convert_config_files(NodeMap) -> + [convert_config_file(NodeMap, Path) || + Path <- [rabbit_node_monitor:running_nodes_filename(), + rabbit_node_monitor:cluster_status_filename()]]. + convert_config_file(NodeMap, Path) -> {ok, Term} = rabbit_file:read_term_file(Path), ok = rabbit_file:write_term_file(Path, update_term(NodeMap, Term)). @@ -130,6 +149,8 @@ lookup_node(OldNode, NodeMap) -> error -> OldNode end. +mini_map(FromNode, ToNode) -> dict:from_list([{FromNode, ToNode}]). + update_term(NodeMap, L) when is_list(L) -> [update_term(NodeMap, I) || I <- L]; update_term(NodeMap, T) when is_tuple(T) -> @@ -141,8 +162,6 @@ update_term(NodeMap, Pid) when is_pid(Pid) -> update_term(_NodeMap, Term) -> Term. -%%---------------------------------------------------------------------------- - rename_remote_node(FromNode, ToNode) -> All = rabbit_mnesia:cluster_nodes(all), Running = rabbit_mnesia:cluster_nodes(running), @@ -152,9 +171,8 @@ rename_remote_node(FromNode, ToNode) -> {_, false} -> exit({new_node_not_in_cluster, ToNode}) end, mnesia:del_table_copy(schema, FromNode), - NodeMap = dict:from_list([{FromNode, ToNode}]), + Map = mini_map(FromNode, ToNode), {atomic, ok} = mnesia:transform_table( - rabbit_durable_queue, - fun (Q) -> update_term(NodeMap, Q) end, + rabbit_durable_queue, fun (Q) -> update_term(Map, Q) end, record_info(fields, amqqueue)), ok. diff --git a/src/rabbit_upgrade.erl b/src/rabbit_upgrade.erl index 848d86c7..2ab65459 100644 --- a/src/rabbit_upgrade.erl +++ b/src/rabbit_upgrade.erl @@ -123,7 +123,7 @@ remove_backup() -> maybe_upgrade_mnesia() -> AllNodes = rabbit_mnesia:cluster_nodes(all), - ok = rabbit_mnesia_rename:maybe_complete_rename(AllNodes), + ok = rabbit_mnesia_rename:maybe_finish(AllNodes), case rabbit_version:upgrades_required(mnesia) of {error, starting_from_scratch} -> ok; -- cgit v1.2.1 From 492e7a6dccb50cad14eba9010313653064c787b5 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 17:22:37 +0000 Subject: And unbreak clusters. --- src/rabbit_mnesia_rename.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index e7c163e1..a8cf2991 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -97,6 +97,7 @@ finish_primary(FromNode, ToNode) -> ToBackup = to_backup_name(), ok = mnesia:install_fallback(ToBackup, [{scope, local}]), start_mnesia(), + rabbit_table:force_load(), rabbit_table:wait_for_replicated(), stop_mnesia(), delete_rename_files(), -- cgit v1.2.1 From 36b9eeb55f60eb8614ea3a0a258ba4c2076e5e6f Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Fri, 28 Nov 2014 17:56:15 +0000 Subject: Rename again --- src/rabbit_control_main.erl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index c00c694c..d9bae4c4 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -40,7 +40,7 @@ change_cluster_node_type, update_cluster_nodes, {forget_cluster_node, [?OFFLINE_DEF]}, - rename_node, + rename_cluster_node, force_boot, cluster_status, {sync_queue, [?VHOST_DEF]}, @@ -105,7 +105,7 @@ -define(COMMANDS_NOT_REQUIRING_APP, [stop, stop_app, start_app, wait, reset, force_reset, rotate_logs, join_cluster, change_cluster_node_type, update_cluster_nodes, - forget_cluster_node, rename_node, cluster_status, status, + forget_cluster_node, rename_cluster_node, cluster_status, status, environment, eval, force_boot]). %%---------------------------------------------------------------------------- @@ -235,7 +235,8 @@ action(forget_cluster_node, Node, [ClusterNodeS], Opts, Inform) -> [ClusterNode, false]) end; -action(rename_node, _Node, [FromNodeS, ToNodeS | OthersS], _Opts, Inform) -> +action(rename_cluster_node, _Node, [FromNodeS, ToNodeS | OthersS], _Opts, + Inform) -> Others = [list_to_atom(N) || N <- OthersS], FromNode = list_to_atom(FromNodeS), ToNode = list_to_atom(ToNodeS), -- cgit v1.2.1 From 22d9ab0a1bd2903856577177e1b7e750435844ce Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 1 Dec 2014 13:57:37 +0000 Subject: Allow argument-pairs to rename_cluster_node to go in any order (and thus allow remote-only renamings). Don't use mnesia:transform_table/3 since it's rather picky about which disk nodes are up. --- src/rabbit_control_main.erl | 15 ++++++------ src/rabbit_mnesia_rename.erl | 54 +++++++++++++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index d9bae4c4..e7e28890 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -235,13 +235,10 @@ action(forget_cluster_node, Node, [ClusterNodeS], Opts, Inform) -> [ClusterNode, false]) end; -action(rename_cluster_node, _Node, [FromNodeS, ToNodeS | OthersS], _Opts, - Inform) -> - Others = [list_to_atom(N) || N <- OthersS], - FromNode = list_to_atom(FromNodeS), - ToNode = list_to_atom(ToNodeS), - Inform("Renaming local cluster node ~s to ~s", [FromNode, ToNode]), - rabbit_mnesia_rename:rename(FromNode, ToNode, Others); +action(rename_cluster_node, Node, NodesS, _Opts, Inform) -> + Nodes = split_list([list_to_atom(N) || N <- NodesS]), + Inform("Renaming cluster nodes:~n ~p~n", [Nodes]), + rabbit_mnesia_rename:rename(Node, Nodes); action(force_boot, Node, [], _Opts, Inform) -> Inform("Forcing boot for Mnesia dir ~s", [mnesia:system_info(directory)]), @@ -729,3 +726,7 @@ prettify_typed_amqp_value(table, Value) -> prettify_amqp_table(Value); prettify_typed_amqp_value(array, Value) -> [prettify_typed_amqp_value(T, V) || {T, V} <- Value]; prettify_typed_amqp_value(_Type, Value) -> Value. + +split_list([]) -> []; +split_list([_]) -> exit(even_list_needed); +split_list([A, B | T]) -> [{A, B} | split_list(T)]. diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index a8cf2991..c505c896 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -17,7 +17,7 @@ -module(rabbit_mnesia_rename). -include("rabbit.hrl"). --export([rename/3]). +-export([rename/2]). -export([maybe_finish/1]). -define(CONVERT_TABLES, [schema, rabbit_durable_queue]). @@ -26,23 +26,37 @@ -ifdef(use_specs). --spec(rename/3 :: (node(), node(), [node()]) -> 'ok'). +-spec(rename/2 :: (node(), [{node(), node()}]) -> 'ok'). -spec(maybe_finish/1 :: ([node()]) -> 'ok'). -endif. %%---------------------------------------------------------------------------- -rename(FromNode, ToNode, Others) -> - NodeMap = dict:from_list(split_others([FromNode, ToNode | Others])), +rename(Node, NodeMapList) -> try + NodeMap = dict:from_list(NodeMapList), + {FromNodes, ToNodes} = lists:unzip(NodeMapList), + case length(FromNodes) - length(lists:usort(ToNodes)) of + 0 -> ok; + _ -> exit({duplicate_node, ToNodes}) + end, + FromNode = case [From || {From, To} <- NodeMapList, + To =:= Node] of + [N] -> N; + [] -> Node + end, + ToNode = case dict:find(FromNode, NodeMap) of + {ok, N2} -> N2; + error -> FromNode + end, rabbit_control_main:become(FromNode), start_mnesia(), Nodes = rabbit_mnesia:cluster_nodes(all), - case {lists:member(FromNode, Nodes), lists:member(ToNode, Nodes)} of - {true, false} -> ok; - {false, _} -> exit({node_not_in_cluster, FromNode}); - {_, true} -> exit({node_already_in_cluster, ToNode}) + case {FromNodes -- Nodes, ToNodes -- (ToNodes -- Nodes)} of + {[], []} -> ok; + {F, []} -> exit({nodes_not_in_cluster, F}); + {_, T} -> exit({nodes_already_in_cluster, T}) end, rabbit_table:force_load(), rabbit_table:wait_for_replicated(), @@ -59,10 +73,6 @@ rename(FromNode, ToNode, Others) -> stop_mnesia() end. -split_others([]) -> []; -split_others([_]) -> exit(even_list_needed); -split_others([A, B | T]) -> [{A, B} | split_others(T)]. - maybe_finish(AllNodes) -> case rabbit_file:read_term_file(rename_config_name()) of {ok, [{FromNode, ToNode}]} -> finish(FromNode, ToNode, AllNodes); @@ -171,9 +181,21 @@ rename_remote_node(FromNode, ToNode) -> {true, _} -> exit({old_node_running, FromNode}); {_, false} -> exit({new_node_not_in_cluster, ToNode}) end, - mnesia:del_table_copy(schema, FromNode), + {atomic, ok} = mnesia:del_table_copy(schema, FromNode), Map = mini_map(FromNode, ToNode), - {atomic, ok} = mnesia:transform_table( - rabbit_durable_queue, fun (Q) -> update_term(Map, Q) end, - record_info(fields, amqqueue)), + {atomic, _} = transform_table(rabbit_durable_queue, Map), ok. + +transform_table(Table, Map) -> + mnesia:sync_transaction( + fun () -> + mnesia:lock({table, Table}, write), + transform_table(Table, Map, mnesia:first(Table)) + end). + +transform_table(Table, Map, '$end_of_table') -> + ok; +transform_table(Table, Map, Key) -> + [Term] = mnesia:read(Table, Key, write), + ok = mnesia:write(Table, update_term(Map, Term), write), + transform_table(Table, Map, mnesia:next(Table, Key)). -- cgit v1.2.1 From 6af82ae5af12881be637d4998ec85526b8c0c56a Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 1 Dec 2014 14:55:38 +0000 Subject: Put all the rename-related temp files in a directory together, and keep copies of the rewritten configs so we can roll back without having to re-edit them. --- src/rabbit_mnesia_rename.erl | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index c505c896..fcd1a595 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -62,6 +62,7 @@ rename(Node, NodeMapList) -> rabbit_table:wait_for_replicated(), FromBackup = from_backup_name(), ToBackup = to_backup_name(), + ok = rabbit_file:ensure_dir(FromBackup), ok = mnesia:backup(FromBackup), stop_mnesia(), convert_backup(NodeMap, FromBackup, ToBackup), @@ -82,6 +83,7 @@ maybe_finish(AllNodes) -> finish(FromNode, ToNode, AllNodes) -> case node() of ToNode -> + [{ok, _} = file:copy(new_conf_path(F), F) || F <- config_files()], case rabbit_upgrade:nodes_running(AllNodes) of [] -> finish_primary(FromNode, ToNode); _ -> finish_secondary(FromNode, ToNode, AllNodes) @@ -90,7 +92,6 @@ finish(FromNode, ToNode, AllNodes) -> rabbit_log:info( "Abandoning rename from ~s to ~s since we are still ~s~n", [FromNode, ToNode, FromNode]), - convert_config_files(mini_map(ToNode, FromNode)), delete_rename_files(); _ -> %% Boot will almost certainly fail but we might as @@ -122,14 +123,13 @@ finish_secondary(FromNode, ToNode, AllNodes) -> delete_rename_files(), ok. -delete_rename_files() -> - rabbit_file:delete(rename_config_name()), - rabbit_file:delete(from_backup_name()), - rabbit_file:delete(to_backup_name()). +temp_dir_name() -> "rename". +dir() -> rabbit_mnesia:dir() ++ "/" ++ temp_dir_name(). +from_backup_name() -> dir() ++ "/backup-from". +to_backup_name() -> dir() ++ "/backup-to". +rename_config_name() -> dir() ++ "/pending.config". -from_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-from". -to_backup_name() -> rabbit_mnesia:dir() ++ "/rename-backup-to". -rename_config_name() -> rabbit_mnesia:dir() ++ "/rename-pending.config". +delete_rename_files() -> ok = rabbit_file:recursive_delete([dir()]). start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). stop_mnesia() -> stopped = mnesia:stop(). @@ -145,14 +145,22 @@ convert_backup(NodeMap, FromBackup, ToBackup) -> end end, switched). +config_files() -> + [rabbit_node_monitor:running_nodes_filename(), + rabbit_node_monitor:cluster_status_filename()]. + +new_conf_path(Path) -> + filename:join([filename:dirname(Path), + temp_dir_name(), + filename:basename(Path)]). + convert_config_files(NodeMap) -> - [convert_config_file(NodeMap, Path) || - Path <- [rabbit_node_monitor:running_nodes_filename(), - rabbit_node_monitor:cluster_status_filename()]]. + [convert_config_file(NodeMap, Path) || Path <- config_files()]. convert_config_file(NodeMap, Path) -> {ok, Term} = rabbit_file:read_term_file(Path), - ok = rabbit_file:write_term_file(Path, update_term(NodeMap, Term)). + ok = rabbit_file:write_term_file( + new_conf_path(Path), update_term(NodeMap, Term)). lookup_node(OldNode, NodeMap) -> case dict:find(OldNode, NodeMap) of @@ -193,7 +201,7 @@ transform_table(Table, Map) -> transform_table(Table, Map, mnesia:first(Table)) end). -transform_table(Table, Map, '$end_of_table') -> +transform_table(_Table, _Map, '$end_of_table') -> ok; transform_table(Table, Map, Key) -> [Term] = mnesia:read(Table, Key, write), -- cgit v1.2.1 From ddfb1f474480999b5b4daca8d7265de6b3a09884 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 1 Dec 2014 15:15:40 +0000 Subject: Swap location of backup and regular config files. A bit less logical, but we need them to be in the right place when starting for big-bang upgrades to work. --- src/rabbit_mnesia_rename.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index fcd1a595..786f9115 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -83,7 +83,6 @@ maybe_finish(AllNodes) -> finish(FromNode, ToNode, AllNodes) -> case node() of ToNode -> - [{ok, _} = file:copy(new_conf_path(F), F) || F <- config_files()], case rabbit_upgrade:nodes_running(AllNodes) of [] -> finish_primary(FromNode, ToNode); _ -> finish_secondary(FromNode, ToNode, AllNodes) @@ -92,6 +91,7 @@ finish(FromNode, ToNode, AllNodes) -> rabbit_log:info( "Abandoning rename from ~s to ~s since we are still ~s~n", [FromNode, ToNode, FromNode]), + [{ok, _} = file:copy(backup_of_conf(F), F) || F <- config_files()], delete_rename_files(); _ -> %% Boot will almost certainly fail but we might as @@ -149,7 +149,7 @@ config_files() -> [rabbit_node_monitor:running_nodes_filename(), rabbit_node_monitor:cluster_status_filename()]. -new_conf_path(Path) -> +backup_of_conf(Path) -> filename:join([filename:dirname(Path), temp_dir_name(), filename:basename(Path)]). @@ -159,8 +159,8 @@ convert_config_files(NodeMap) -> convert_config_file(NodeMap, Path) -> {ok, Term} = rabbit_file:read_term_file(Path), - ok = rabbit_file:write_term_file( - new_conf_path(Path), update_term(NodeMap, Term)). + {ok, _} = file:copy(Path, backup_of_conf(Path)), + ok = rabbit_file:write_term_file(Path, update_term(NodeMap, Term)). lookup_node(OldNode, NodeMap) -> case dict:find(OldNode, NodeMap) of -- cgit v1.2.1 From d7a33bd61756233e2c259ee450256e7ac107f17e Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 1 Dec 2014 15:30:59 +0000 Subject: Add a section to the manual page. --- docs/rabbitmqctl.1.xml | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/docs/rabbitmqctl.1.xml b/docs/rabbitmqctl.1.xml index 8d04f28a..047fd57f 100644 --- a/docs/rabbitmqctl.1.xml +++ b/docs/rabbitmqctl.1.xml @@ -425,6 +425,41 @@ + + rename_cluster_node oldnode1 newnode1 oldnode2 newnode2 ... + + + Supports renaming of the local cluster node. + + + This subcommand causes rabbitmqctl to temporarily become + the node in order to make the change. The local cluster + node must therefore be completely stopped; other nodes + can be online or offline. + + + This subcommand takes an even number of arguments, in + pairs representing the old and new names for nodes. You + must specify the old and new names for this node and for + any other nodes that are stopped and being renamed at + the same time. + + + It is possible to stop all nodes and rename them all + simultaneously (in which case old and new names for all + nodes must be given to every node) or stop and rename + nodes one at a time (in which case each node only needs + to be told how its own name is changing). + + For example: + rabbitmqctl rename_cluster_node rabbit@misshelpful rabbit@cordelia + + This command will rename the node + rabbit@misshelpful to the node + rabbit@cordelia. + + + update_cluster_nodes clusternode -- cgit v1.2.1 From 864e67f5e853bd9185406b6f9aedd0c91824ca29 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 1 Dec 2014 15:46:57 +0000 Subject: Maybe clearer? --- docs/rabbitmqctl.1.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rabbitmqctl.1.xml b/docs/rabbitmqctl.1.xml index 047fd57f..8d042670 100644 --- a/docs/rabbitmqctl.1.xml +++ b/docs/rabbitmqctl.1.xml @@ -429,7 +429,7 @@ rename_cluster_node oldnode1 newnode1 oldnode2 newnode2 ... - Supports renaming of the local cluster node. + Supports renaming of cluster nodes in the local database. This subcommand causes rabbitmqctl to temporarily become -- cgit v1.2.1 From 86e2af9c031b7c2791461f78a9924e104129b2af Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:14:30 +0100 Subject: Move the info keys list added to user_authentication_* to a -define() --- src/rabbit_reader.erl | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 713b1844..260fdad7 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -58,6 +58,11 @@ -define(INFO_KEYS, ?CREATION_EVENT_KEYS ++ ?STATISTICS_KEYS -- [pid]). +-define(AUTH_NOTIFICATION_INFO_KEYS, + [peer_cert_validity, peer_cert_subject, peer_cert_issuer, + ssl_cipher, ssl_protocol, ssl, auth_mechanism, protocol, + peer_port, peer_host, name, vhost, host]). + -define(IS_RUNNING(State), (State#v1.connection_state =:= running orelse State#v1.connection_state =:= blocking orelse @@ -1108,21 +1113,7 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> fun (name, Acc) -> [{connection_name, i(name, State)} | Acc]; (Item, Acc) -> [{Item, i(Item, State)} | Acc] - end, EventProps0, [ - peer_cert_validity, - peer_cert_subject, - peer_cert_issuer, - ssl_cipher, - ssl_protocol, - ssl, - auth_mechanism, - protocol, - peer_port, - peer_host, - name, - vhost, - host - ]), + end, EventProps0, ?AUTH_NOTIFICATION_INFO_KEYS), EventProps2 = case Username of none -> [{name, ''} | EventProps1]; _ -> [{name, Username} | EventProps1] -- cgit v1.2.1 From 5d3858e788d1114608541dc1e830206534fd89ab Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:17:07 +0100 Subject: Use rabbit_misc:format/2 instead of lists:flatten/1 + io_lib:format/2 --- src/rabbit_direct.erl | 2 +- src/rabbit_reader.erl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 9756dd49..ddd8d4e5 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -103,7 +103,7 @@ notify_auth_result(Username, AuthResult, Msg, Args) -> end, EventProps = case Msg of "" -> EventProps1; - _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps1] + _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps1] end, rabbit_event:notify(AuthResult, EventProps). diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 260fdad7..88bbc367 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1120,7 +1120,7 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> end, EventProps = case Msg of "" -> EventProps2; - _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps2] + _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps2] end, rabbit_event:notify(AuthResult, EventProps). -- cgit v1.2.1 From 2c4bdf1d191d97d55461209f9f185a044ab84433 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:38:12 +0100 Subject: Use list comprehension instead of lists:foldl/3 To keep a somewhat logical order in the list of user_authentication_* properties, reverse the order of the AUTH_NOTIFICATION_INFO_KEYS list. This list was previously reversed by lists:foldl/3. --- src/rabbit_reader.erl | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 88bbc367..60246fbe 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -59,9 +59,9 @@ -define(INFO_KEYS, ?CREATION_EVENT_KEYS ++ ?STATISTICS_KEYS -- [pid]). -define(AUTH_NOTIFICATION_INFO_KEYS, - [peer_cert_validity, peer_cert_subject, peer_cert_issuer, - ssl_cipher, ssl_protocol, ssl, auth_mechanism, protocol, - peer_port, peer_host, name, vhost, host]). + [host, vhost, name, peer_host, peer_port, protocol, auth_mechanism, + ssl, ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, + peer_cert_validity]). -define(IS_RUNNING(State), (State#v1.connection_state =:= running orelse @@ -1109,11 +1109,11 @@ auth_fail(Username, Msg, Args, AuthName, notify_auth_result(Username, AuthResult, Msg, Args, State) -> EventProps0 = [{connection_type, network}], - EventProps1 = lists:foldl( - fun - (name, Acc) -> [{connection_name, i(name, State)} | Acc]; - (Item, Acc) -> [{Item, i(Item, State)} | Acc] - end, EventProps0, ?AUTH_NOTIFICATION_INFO_KEYS), + EventProps1 = EventProps0 ++ [ + case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS], EventProps2 = case Username of none -> [{name, ''} | EventProps1]; _ -> [{name, Username} | EventProps1] -- cgit v1.2.1 From 6aa27b4694b043423d2d992602643420f955f91d Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:45:27 +0100 Subject: Only include ssl/certificate informations when the connection is over SSL This gives lighter notifications for plain TCP connections. --- src/rabbit_reader.erl | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 60246fbe..1ec8a150 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -60,7 +60,10 @@ -define(AUTH_NOTIFICATION_INFO_KEYS, [host, vhost, name, peer_host, peer_port, protocol, auth_mechanism, - ssl, ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, + ssl]). + +-define(AUTH_NOTIFICATION_SSL_INFO_KEYS, + [ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, peer_cert_validity]). -define(IS_RUNNING(State), @@ -1114,13 +1117,21 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> name -> {connection_name, i(name, State)}; _ -> {Item, i(Item, State)} end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS], - EventProps2 = case Username of - none -> [{name, ''} | EventProps1]; - _ -> [{name, Username} | EventProps1] + EventProps2 = case i(ssl, State) of + false -> EventProps1; + true -> EventProps1 ++ [ + case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] + end, + EventProps3 = case Username of + none -> [{name, ''} | EventProps2]; + _ -> [{name, Username} | EventProps2] end, EventProps = case Msg of - "" -> EventProps2; - _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps2] + "" -> EventProps3; + _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps3] end, rabbit_event:notify(AuthResult, EventProps). -- cgit v1.2.1 From e74bd8fe71260fe669793bcdad245591a61c56ab Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:55:23 +0100 Subject: Pass "extra properties" to notify_auth_result/{3,4} instead of a message This avoids to passs an empty message in the case of successful authentication. --- src/rabbit_direct.erl | 12 +++++------- src/rabbit_reader.erl | 15 +++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index ddd8d4e5..34eff61f 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -85,26 +85,24 @@ connect0(AuthFun, VHost, Protocol, Pid, Infos) -> true -> case AuthFun() of {ok, User = #user{username = Username}} -> notify_auth_result(Username, - user_authentication_success, "", []), + user_authentication_success, []), connect1(User, VHost, Protocol, Pid, Infos); {refused, Username, Msg, Args} -> notify_auth_result(Username, - user_authentication_failure, Msg, Args), + user_authentication_failure, + [{error, rabbit_misc:format(Msg, Args)}]), {error, {auth_failure, "Refused"}} end; false -> {error, broker_not_found_on_node} end. -notify_auth_result(Username, AuthResult, Msg, Args) -> +notify_auth_result(Username, AuthResult, ExtraProps) -> EventProps0 = [{connection_type, direct}], EventProps1 = case Username of none -> [{name, ''} | EventProps0]; _ -> [{name, Username} | EventProps0] end, - EventProps = case Msg of - "" -> EventProps1; - _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps1] - end, + EventProps = EventProps1 ++ ExtraProps, rabbit_event:notify(AuthResult, EventProps). connect1(User, VHost, Protocol, Pid, Infos) -> diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 1ec8a150..968e6a4d 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1058,7 +1058,8 @@ auth_phase(Response, auth_fail(Username, Msg, Args, Name, State); {protocol_error, Msg, Args} -> notify_auth_result(none, user_authentication_failure, - Msg, Args, State), + [{error, rabbit_misc:format(Msg, Args)}], + State), rabbit_misc:protocol_error(syntax_error, Msg, Args); {challenge, Challenge, AuthState1} -> Secure = #'connection.secure'{challenge = Challenge}, @@ -1069,7 +1070,7 @@ auth_phase(Response, case rabbit_access_control:check_user_loopback(Username, Sock) of ok -> notify_auth_result(Username, user_authentication_success, - "", [], State); + [], State); not_allowed -> auth_fail(Username, "user '~s' can only connect via " "localhost", [Username], Name, State) @@ -1091,7 +1092,8 @@ auth_phase(Response, auth_fail(Username, Msg, Args, AuthName, State = #v1{connection = #connection{protocol = Protocol, capabilities = Capabilities}}) -> - notify_auth_result(Username, user_authentication_failure, Msg, Args, State), + notify_auth_result(Username, user_authentication_failure, + [{error, rabbit_misc:format(Msg, Args)}], State), AmqpError = rabbit_misc:amqp_error( access_refused, "~s login refused: ~s", [AuthName, io_lib:format(Msg, Args)], none), @@ -1110,7 +1112,7 @@ auth_fail(Username, Msg, Args, AuthName, end, rabbit_misc:protocol_error(AmqpError). -notify_auth_result(Username, AuthResult, Msg, Args, State) -> +notify_auth_result(Username, AuthResult, ExtraProps, State) -> EventProps0 = [{connection_type, network}], EventProps1 = EventProps0 ++ [ case Item of @@ -1129,10 +1131,7 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> none -> [{name, ''} | EventProps2]; _ -> [{name, Username} | EventProps2] end, - EventProps = case Msg of - "" -> EventProps3; - _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps3] - end, + EventProps = EventProps3 ++ ExtraProps, rabbit_event:notify(AuthResult, EventProps). %%-------------------------------------------------------------------------- -- cgit v1.2.1 From 7378afd5fed81839813307fc9080a60af61aa91d Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 3 Dec 2014 14:05:03 +0100 Subject: Style fix: Use ++ to construct EventProps, not multiple temporary variables --- src/rabbit_direct.erl | 9 +++------ src/rabbit_reader.erl | 33 ++++++++++++++------------------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 34eff61f..931e1e4d 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -97,12 +97,9 @@ connect0(AuthFun, VHost, Protocol, Pid, Infos) -> end. notify_auth_result(Username, AuthResult, ExtraProps) -> - EventProps0 = [{connection_type, direct}], - EventProps1 = case Username of - none -> [{name, ''} | EventProps0]; - _ -> [{name, Username} | EventProps0] - end, - EventProps = EventProps1 ++ ExtraProps, + EventProps = [{connection_type, direct}] ++ + [{name, case Username of none -> ''; _ -> Username end}] ++ + ExtraProps, rabbit_event:notify(AuthResult, EventProps). connect1(User, VHost, Protocol, Pid, Infos) -> diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 968e6a4d..31e60e9a 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1113,25 +1113,20 @@ auth_fail(Username, Msg, Args, AuthName, rabbit_misc:protocol_error(AmqpError). notify_auth_result(Username, AuthResult, ExtraProps, State) -> - EventProps0 = [{connection_type, network}], - EventProps1 = EventProps0 ++ [ - case Item of - name -> {connection_name, i(name, State)}; - _ -> {Item, i(Item, State)} - end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS], - EventProps2 = case i(ssl, State) of - false -> EventProps1; - true -> EventProps1 ++ [ - case Item of - name -> {connection_name, i(name, State)}; - _ -> {Item, i(Item, State)} - end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] - end, - EventProps3 = case Username of - none -> [{name, ''} | EventProps2]; - _ -> [{name, Username} | EventProps2] - end, - EventProps = EventProps3 ++ ExtraProps, + EventProps = [{connection_type, network}] ++ + [case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS] ++ + case i(ssl, State) of + false -> []; + true -> [case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] + end ++ + [{name, case Username of none -> ''; _ -> Username end}] ++ + ExtraProps, rabbit_event:notify(AuthResult, EventProps). %%-------------------------------------------------------------------------- -- cgit v1.2.1 From f59084f398c8d7f73209c5ea60dac28352c027b6 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 3 Dec 2014 14:10:35 +0100 Subject: Filter out auth notification properties with no value --- src/rabbit_direct.erl | 2 +- src/rabbit_reader.erl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 931e1e4d..79c7a195 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -100,7 +100,7 @@ notify_auth_result(Username, AuthResult, ExtraProps) -> EventProps = [{connection_type, direct}] ++ [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, - rabbit_event:notify(AuthResult, EventProps). + rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). connect1(User, VHost, Protocol, Pid, Infos) -> try rabbit_access_control:check_vhost_access(User, VHost, undefined) of diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 31e60e9a..0c5f0232 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1127,7 +1127,7 @@ notify_auth_result(Username, AuthResult, ExtraProps, State) -> end ++ [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, - rabbit_event:notify(AuthResult, EventProps). + rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). %%-------------------------------------------------------------------------- -- cgit v1.2.1 From e76617eacf49f459700f29fa66cbb377465e085e Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Mon, 8 Dec 2014 16:57:36 +0000 Subject: WIP clustererer compatibility: install the fallback and "upgrade" mnesia at rename_cluster_node time, instead of during next boot. This means we need to faff around a bit making sure we do not contact other nodes during the upgrade, but also that on-disk data structures are left in a consistent state for the clusterererer to find them. --- src/rabbit_mnesia_rename.erl | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index 786f9115..d93f8ea3 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -69,6 +69,14 @@ rename(Node, NodeMapList) -> ok = rabbit_file:write_term_file(rename_config_name(), [{FromNode, ToNode}]), convert_config_files(NodeMap), + rabbit_control_main:become(ToNode), + ok = mnesia:install_fallback(ToBackup, [{scope, local}]), + application:set_env(kernel, dist_auto_connect, never), + start_mnesia(), + rabbit_table:force_load(), + rabbit_table:wait_for_replicated(), + stop_mnesia(), + rabbit_mnesia:force_load_next_boot(), ok after stop_mnesia() @@ -88,11 +96,12 @@ finish(FromNode, ToNode, AllNodes) -> _ -> finish_secondary(FromNode, ToNode, AllNodes) end; FromNode -> - rabbit_log:info( - "Abandoning rename from ~s to ~s since we are still ~s~n", - [FromNode, ToNode, FromNode]), - [{ok, _} = file:copy(backup_of_conf(F), F) || F <- config_files()], - delete_rename_files(); + exit(todo_fix_this_case); + %% rabbit_log:info( + %% "Abandoning rename from ~s to ~s since we are still ~s~n", + %% [FromNode, ToNode, FromNode]), + %% [{ok, _} = file:copy(backup_of_conf(F), F) || F <- config_files()], + %% delete_rename_files(); _ -> %% Boot will almost certainly fail but we might as %% well just log this @@ -104,15 +113,7 @@ finish(FromNode, ToNode, AllNodes) -> finish_primary(FromNode, ToNode) -> rabbit_log:info("Restarting as primary after rename from ~s to ~s~n", [FromNode, ToNode]), - %% We are alone, restore the backup we previously took - ToBackup = to_backup_name(), - ok = mnesia:install_fallback(ToBackup, [{scope, local}]), - start_mnesia(), - rabbit_table:force_load(), - rabbit_table:wait_for_replicated(), - stop_mnesia(), delete_rename_files(), - rabbit_mnesia:force_load_next_boot(), ok. finish_secondary(FromNode, ToNode, AllNodes) -> -- cgit v1.2.1 From 90d553d02219327a54bb90a6a2e03356cecdedb5 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Tue, 9 Dec 2014 10:31:42 +0000 Subject: Ensure become(node()) works rather than failing with {node_running, ...}. --- src/rabbit_control_main.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index e7e28890..2d5094a6 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -597,11 +597,11 @@ start_distribution() -> {ok, _} = net_kernel:start([list_to_atom(CtlNodeName), name_type()]). become(BecomeNode) -> + error_logger:tty(false), + ok = net_kernel:stop(), case net_adm:ping(BecomeNode) of pong -> exit({node_running, BecomeNode}); pang -> io:format(" * Impersonating node: ~s...", [BecomeNode]), - error_logger:tty(false), - ok = net_kernel:stop(), {ok, _} = net_kernel:start([BecomeNode, name_type()]), io:format(" done~n", []), Dir = mnesia:system_info(directory), -- cgit v1.2.1 From 26cb34870e9681e7cf5addea5942228d80c0d977 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Tue, 9 Dec 2014 10:32:11 +0000 Subject: Fail clearly if the node specified with '-n' is not a member of the cluster before or after. --- src/rabbit_mnesia_rename.erl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index d93f8ea3..9ef8aedd 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -53,10 +53,12 @@ rename(Node, NodeMapList) -> rabbit_control_main:become(FromNode), start_mnesia(), Nodes = rabbit_mnesia:cluster_nodes(all), - case {FromNodes -- Nodes, ToNodes -- (ToNodes -- Nodes)} of - {[], []} -> ok; - {F, []} -> exit({nodes_not_in_cluster, F}); - {_, T} -> exit({nodes_already_in_cluster, T}) + case {FromNodes -- Nodes, ToNodes -- (ToNodes -- Nodes), + lists:member(Node, Nodes ++ ToNodes)} of + {[], [], true} -> ok; + {[], [], false} -> exit({i_am_not_involved, Node}); + {F, [], _} -> exit({nodes_not_in_cluster, F}); + {_, T, _} -> exit({nodes_already_in_cluster, T}) end, rabbit_table:force_load(), rabbit_table:wait_for_replicated(), -- cgit v1.2.1 From 12e1354e311d9abb8b4bcc600a6528e243566f50 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Tue, 9 Dec 2014 11:49:31 +0000 Subject: Various fixes / improvements: with the new scheme multiple renamings will not waork, so check for them. Fix abortive renames (with a copy of the mnesia dir). Ensure we don't talk to any other nodes while backing up / restoring since that inevitably goes wrong. Split up rename/2 a bit and rename a few other things for more clarity. --- src/rabbit_mnesia_rename.erl | 71 ++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index 9ef8aedd..4c366c1a 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -35,6 +35,11 @@ rename(Node, NodeMapList) -> try + case rabbit_file:is_dir(dir()) of + true -> exit({rename_in_progress, + "Restart node under old name to roll back"}); + false -> ok = rabbit_file:ensure_dir(mnesia_copy_dir()) + end, NodeMap = dict:from_list(NodeMapList), {FromNodes, ToNodes} = lists:unzip(NodeMapList), case length(FromNodes) - length(lists:usort(ToNodes)) of @@ -50,8 +55,9 @@ rename(Node, NodeMapList) -> {ok, N2} -> N2; error -> FromNode end, + application:set_env(kernel, dist_auto_connect, never), rabbit_control_main:become(FromNode), - start_mnesia(), + ok = rabbit_mnesia:copy_db(mnesia_copy_dir()), Nodes = rabbit_mnesia:cluster_nodes(all), case {FromNodes -- Nodes, ToNodes -- (ToNodes -- Nodes), lists:member(Node, Nodes ++ ToNodes)} of @@ -60,30 +66,29 @@ rename(Node, NodeMapList) -> {F, [], _} -> exit({nodes_not_in_cluster, F}); {_, T, _} -> exit({nodes_already_in_cluster, T}) end, - rabbit_table:force_load(), - rabbit_table:wait_for_replicated(), - FromBackup = from_backup_name(), - ToBackup = to_backup_name(), - ok = rabbit_file:ensure_dir(FromBackup), - ok = mnesia:backup(FromBackup), - stop_mnesia(), - convert_backup(NodeMap, FromBackup, ToBackup), + take_backup(before_backup_name()), + convert_backup(NodeMap, before_backup_name(), after_backup_name()), ok = rabbit_file:write_term_file(rename_config_name(), [{FromNode, ToNode}]), convert_config_files(NodeMap), rabbit_control_main:become(ToNode), - ok = mnesia:install_fallback(ToBackup, [{scope, local}]), - application:set_env(kernel, dist_auto_connect, never), - start_mnesia(), - rabbit_table:force_load(), - rabbit_table:wait_for_replicated(), - stop_mnesia(), - rabbit_mnesia:force_load_next_boot(), + restore_backup(after_backup_name()), ok after stop_mnesia() end. +take_backup(Backup) -> + start_mnesia(), + ok = mnesia:backup(Backup), + stop_mnesia(). + +restore_backup(Backup) -> + ok = mnesia:install_fallback(Backup, [{scope, local}]), + start_mnesia(), + stop_mnesia(), + rabbit_mnesia:force_load_next_boot(). + maybe_finish(AllNodes) -> case rabbit_file:read_term_file(rename_config_name()) of {ok, [{FromNode, ToNode}]} -> finish(FromNode, ToNode, AllNodes); @@ -98,12 +103,14 @@ finish(FromNode, ToNode, AllNodes) -> _ -> finish_secondary(FromNode, ToNode, AllNodes) end; FromNode -> - exit(todo_fix_this_case); - %% rabbit_log:info( - %% "Abandoning rename from ~s to ~s since we are still ~s~n", - %% [FromNode, ToNode, FromNode]), - %% [{ok, _} = file:copy(backup_of_conf(F), F) || F <- config_files()], - %% delete_rename_files(); + rabbit_log:info( + "Abandoning rename from ~s to ~s since we are still ~s~n", + [FromNode, ToNode, FromNode]), + [{ok, _} = file:copy(backup_of_conf(F), F) || F <- config_files()], + ok = rabbit_file:recursive_delete([rabbit_mnesia:dir()]), + ok = rabbit_file:recursive_copy( + mnesia_copy_dir(), rabbit_mnesia:dir()), + delete_rename_files(); _ -> %% Boot will almost certainly fail but we might as %% well just log this @@ -122,19 +129,21 @@ finish_secondary(FromNode, ToNode, AllNodes) -> rabbit_log:info("Restarting as secondary after rename from ~s to ~s~n", [FromNode, ToNode]), rabbit_upgrade:secondary_upgrade(AllNodes), - rename_remote_node(FromNode, ToNode), + rename_in_running_mnesia(FromNode, ToNode), delete_rename_files(), ok. -temp_dir_name() -> "rename". -dir() -> rabbit_mnesia:dir() ++ "/" ++ temp_dir_name(). -from_backup_name() -> dir() ++ "/backup-from". -to_backup_name() -> dir() ++ "/backup-to". +dir() -> rabbit_mnesia:dir() ++ "-rename". +before_backup_name() -> dir() ++ "/backup-before". +after_backup_name() -> dir() ++ "/backup-after". rename_config_name() -> dir() ++ "/pending.config". +mnesia_copy_dir() -> dir() ++ "/mnesia-copy". delete_rename_files() -> ok = rabbit_file:recursive_delete([dir()]). -start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia). +start_mnesia() -> rabbit_misc:ensure_ok(mnesia:start(), cannot_start_mnesia), + rabbit_table:force_load(), + rabbit_table:wait_for_replicated(). stop_mnesia() -> stopped = mnesia:stop(). convert_backup(NodeMap, FromBackup, ToBackup) -> @@ -153,9 +162,7 @@ config_files() -> rabbit_node_monitor:cluster_status_filename()]. backup_of_conf(Path) -> - filename:join([filename:dirname(Path), - temp_dir_name(), - filename:basename(Path)]). + filename:join([dir(), filename:basename(Path)]). convert_config_files(NodeMap) -> [convert_config_file(NodeMap, Path) || Path <- config_files()]. @@ -184,7 +191,7 @@ update_term(NodeMap, Pid) when is_pid(Pid) -> update_term(_NodeMap, Term) -> Term. -rename_remote_node(FromNode, ToNode) -> +rename_in_running_mnesia(FromNode, ToNode) -> All = rabbit_mnesia:cluster_nodes(all), Running = rabbit_mnesia:cluster_nodes(running), case {lists:member(FromNode, Running), lists:member(ToNode, All)} of -- cgit v1.2.1 From 84a05d5c45e782e2659c01afc9d1ed01fe875904 Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Tue, 9 Dec 2014 12:15:03 +0000 Subject: Split out preparation from rename/2, and add comments. --- src/rabbit_mnesia_rename.erl | 106 +++++++++++++++++++++++++++++++------------ 1 file changed, 77 insertions(+), 29 deletions(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index 4c366c1a..2787cb74 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -22,6 +22,25 @@ -define(CONVERT_TABLES, [schema, rabbit_durable_queue]). +%% Supports renaming the nodes in the Mnesia database. In order to do +%% this, we take a backup of the database, traverse the backup +%% changing node names and pids as we go, then restore it. +%% +%% That's enough for a standalone node, for clusters the story is more +%% complex. We can take pairs of nodes From and To, but backing up and +%% restoring the database changes schema cookies, so if we just do +%% this on all nodes the cluster will refuse to re-form with +%% "Incompatible schema cookies.". Therefore we do something similar +%% to what we do for upgrades - the first node in the cluster to +%% restart becomes the authority, and other nodes wipe their own +%% Mnesia state and rejoin. They also need to tell Mnesia the old node +%% is not coming back. +%% +%% If we are renaming nodes one at a time then the running cluster +%% might not be aware that a rename has taken place, so after we wipe +%% and rejoin we then update any tables (in practice just +%% rabbit_durable_queue) which should be aware that we have changed. + %%---------------------------------------------------------------------------- -ifdef(use_specs). @@ -35,37 +54,27 @@ rename(Node, NodeMapList) -> try - case rabbit_file:is_dir(dir()) of - true -> exit({rename_in_progress, - "Restart node under old name to roll back"}); - false -> ok = rabbit_file:ensure_dir(mnesia_copy_dir()) - end, - NodeMap = dict:from_list(NodeMapList), - {FromNodes, ToNodes} = lists:unzip(NodeMapList), - case length(FromNodes) - length(lists:usort(ToNodes)) of - 0 -> ok; - _ -> exit({duplicate_node, ToNodes}) - end, - FromNode = case [From || {From, To} <- NodeMapList, - To =:= Node] of - [N] -> N; - [] -> Node - end, - ToNode = case dict:find(FromNode, NodeMap) of - {ok, N2} -> N2; - error -> FromNode - end, + %% Check everything is correct and figure out what we are + %% changing from and to. + {FromNode, ToNode, NodeMap} = prepare(Node, NodeMapList), + + %% We backup and restore Mnesia even if other nodes are + %% running at the time, and defer the final decision about + %% whether to use our mutated copy or rejoin the cluster until + %% we restart. That means we might be mutating our copy of the + %% database while the cluster is running. *Do not* contact the + %% cluster while this is happening, we are likely to get + %% confused. application:set_env(kernel, dist_auto_connect, never), - rabbit_control_main:become(FromNode), + + %% Take a copy we can restore from if we abandon the + %% rename. We don't restore from the "backup" since restoring + %% that changes schema cookies and might stop us rejoining the + %% cluster. ok = rabbit_mnesia:copy_db(mnesia_copy_dir()), - Nodes = rabbit_mnesia:cluster_nodes(all), - case {FromNodes -- Nodes, ToNodes -- (ToNodes -- Nodes), - lists:member(Node, Nodes ++ ToNodes)} of - {[], [], true} -> ok; - {[], [], false} -> exit({i_am_not_involved, Node}); - {F, [], _} -> exit({nodes_not_in_cluster, F}); - {_, T, _} -> exit({nodes_already_in_cluster, T}) - end, + + %% And make the actual changes + rabbit_control_main:become(FromNode), take_backup(before_backup_name()), convert_backup(NodeMap, before_backup_name(), after_backup_name()), ok = rabbit_file:write_term_file(rename_config_name(), @@ -78,6 +87,45 @@ rename(Node, NodeMapList) -> stop_mnesia() end. +prepare(Node, NodeMapList) -> + %% If we have a previous rename and haven't started since, give up. + case rabbit_file:is_dir(dir()) of + true -> exit({rename_in_progress, + "Restart node under old name to roll back"}); + false -> ok = rabbit_file:ensure_dir(mnesia_copy_dir()) + end, + + %% Check we don't have two nodes mapped to the same node + {FromNodes, ToNodes} = lists:unzip(NodeMapList), + case length(FromNodes) - length(lists:usort(ToNodes)) of + 0 -> ok; + _ -> exit({duplicate_node, ToNodes}) + end, + + %% Figure out which node we are before and after the change + FromNode = case [From || {From, To} <- NodeMapList, + To =:= Node] of + [N] -> N; + [] -> Node + end, + NodeMap = dict:from_list(NodeMapList), + ToNode = case dict:find(FromNode, NodeMap) of + {ok, N2} -> N2; + error -> FromNode + end, + + %% Check that we are in the cluster, all old nodes are in the + %% cluster, and no new nodes are. + Nodes = rabbit_mnesia:cluster_nodes(all), + case {FromNodes -- Nodes, ToNodes -- (ToNodes -- Nodes), + lists:member(Node, Nodes ++ ToNodes)} of + {[], [], true} -> ok; + {[], [], false} -> exit({i_am_not_involved, Node}); + {F, [], _} -> exit({nodes_not_in_cluster, F}); + {_, T, _} -> exit({nodes_already_in_cluster, T}) + end, + {FromNode, ToNode, NodeMap}. + take_backup(Backup) -> start_mnesia(), ok = mnesia:backup(Backup), -- cgit v1.2.1 From 37393c66a7f24ce774c261456dfe11705338386a Mon Sep 17 00:00:00 2001 From: Simon MacMullen Date: Tue, 9 Dec 2014 12:21:05 +0000 Subject: Nicer feedback. --- src/rabbit_control_main.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index 2d5094a6..bca740c6 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -237,7 +237,9 @@ action(forget_cluster_node, Node, [ClusterNodeS], Opts, Inform) -> action(rename_cluster_node, Node, NodesS, _Opts, Inform) -> Nodes = split_list([list_to_atom(N) || N <- NodesS]), - Inform("Renaming cluster nodes:~n ~p~n", [Nodes]), + Inform("Renaming cluster nodes:~n~s~n", + [lists:flatten([rabbit_misc:format(" ~s -> ~s~n", [F, T]) || + {F, T} <- Nodes])]), rabbit_mnesia_rename:rename(Node, Nodes); action(force_boot, Node, [], _Opts, Inform) -> -- cgit v1.2.1 From 3e4d1f7b0d46ca99f7ecc05e6ec29c1d17ff8d2c Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 10 Dec 2014 11:25:59 +0100 Subject: Now that empty keys are dropped, we can always add ssl_* keys If the connection isn't over SSL, ssl_* keys will be empty and dropped anyway. --- src/rabbit_reader.erl | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 0c5f0232..e1ffb704 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -60,10 +60,7 @@ -define(AUTH_NOTIFICATION_INFO_KEYS, [host, vhost, name, peer_host, peer_port, protocol, auth_mechanism, - ssl]). - --define(AUTH_NOTIFICATION_SSL_INFO_KEYS, - [ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, + ssl, ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, peer_cert_validity]). -define(IS_RUNNING(State), @@ -1118,13 +1115,6 @@ notify_auth_result(Username, AuthResult, ExtraProps, State) -> name -> {connection_name, i(name, State)}; _ -> {Item, i(Item, State)} end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS] ++ - case i(ssl, State) of - false -> []; - true -> [case Item of - name -> {connection_name, i(name, State)}; - _ -> {Item, i(Item, State)} - end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] - end ++ [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). -- cgit v1.2.1 From 643d598dfc6452b301a9ceeceb20f68d6a820336 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 10 Dec 2014 11:42:13 +0100 Subject: No need to concatenate hard-coded lists... --- src/rabbit_direct.erl | 4 ++-- src/rabbit_reader.erl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 79c7a195..11233e7e 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -97,8 +97,8 @@ connect0(AuthFun, VHost, Protocol, Pid, Infos) -> end. notify_auth_result(Username, AuthResult, ExtraProps) -> - EventProps = [{connection_type, direct}] ++ - [{name, case Username of none -> ''; _ -> Username end}] ++ + EventProps = [{connection_type, direct}, + {name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index e1ffb704..c92eaf7f 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1110,12 +1110,12 @@ auth_fail(Username, Msg, Args, AuthName, rabbit_misc:protocol_error(AmqpError). notify_auth_result(Username, AuthResult, ExtraProps, State) -> - EventProps = [{connection_type, network}] ++ + EventProps = [{connection_type, network}, + {name, case Username of none -> ''; _ -> Username end}] ++ [case Item of name -> {connection_name, i(name, State)}; _ -> {Item, i(Item, State)} end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS] ++ - [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). -- cgit v1.2.1