From 64c9e7cbc1212fd7527ecf77674081983446ce91 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 --- src/rabbit_binary_parser.erl | 103 ++++++++++++++++++++++++++++++------------- 1 file changed, 73 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/rabbit_binary_parser.erl b/src/rabbit_binary_parser.erl index 3ab82cad78..8850d69e38 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 51c02274f37cd756962020c700a83160f80b9cde 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(-) (limited to 'src') diff --git a/src/rabbit_binary_parser.erl b/src/rabbit_binary_parser.erl index 8850d69e38..6e277d3572 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 c2f204b93cf7dee57509f700a27a61f85204e27d 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(-) (limited to 'src') diff --git a/src/rabbit_binary_parser.erl b/src/rabbit_binary_parser.erl index 6e277d3572..ee8147f45f 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 d0e3516158df5543d7fb086dc53331f215b1ce32 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 3a7a692c5c..cec4bccc95 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 ee14d15b0a4dc605cb41f7e2624a8de3904925d4 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 (limited to 'src') diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl new file mode 100644 index 0000000000..c8af20a28f --- /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 56f30a96e9ce6035f8870faf12a13fe7b7f87d61 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(-) (limited to 'src') diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl index c8af20a28f..d055d84a7b 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 6feef553568bbb1ed35915763163dd3c464facbc 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index cec4bccc95..126b3f811d 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 a765a61c445aa3e44682b280559f39c6671a36e3 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index cec4bccc95..3364c3aecd 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 d055d84a7b..5a8d7b2943 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 3c68f656981fac66162902ddb259c4ff5fc43c12 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 3364c3aecd..65f3f45b1f 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 5a8d7b2943..b1fbb3f4ed 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 9f4627448c446becf8a71700cca295e090694446 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(-) (limited to 'src') diff --git a/src/file_handle_cache_stats.erl b/src/file_handle_cache_stats.erl index b1fbb3f4ed..832f0b3ddf 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 b838aa5423d99cae683bd9c344c325a7aa3a08b0 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 519c596d1d..a7d5ce1596 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 f285a3880bc5a8b7c77e6f03265cb57fae5278be 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index a7d5ce1596..fa896c67f3 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 abebb6517d14610bb0ee31f28ee3f10efaeb4154 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index fa896c67f3..06a72aa45d 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 830c24a4928b0ef968e205195aa2b0128d1e662c 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(-) (limited to 'src') diff --git a/src/file_handle_cache.erl b/src/file_handle_cache.erl index 06a72aa45d..2922e146a8 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 b829ae9476..6c80ddcd2c 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 75924c47056260b6f7533c9dce01d8dada072800 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 --- 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 +-- 8 files changed, 88 insertions(+), 59 deletions(-) (limited to 'src') diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index b0a9c0d807..dcec0ff51b 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 a7dd6494b1..99f291f174 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 5daca368eb..8e2d826930 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 fd1c4e8ee6..5cdff9857f 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 8632e1b3f7..2f7e234d89 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 749a67b1d5..f6140f09c2 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 ca73006aed..2033dd14ac 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 ba48867ad0..b3158cc821 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(), -- cgit v1.2.1 From 1f1aeb21df22abb740483dd011ccec1a25c299b7 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(-) (limited to 'src') diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index dcec0ff51b..a6d3cd6cca 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 751b440de2846a87814a2d926043958968a33a7e 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. --- 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 +- 6 files changed, 79 insertions(+), 59 deletions(-) (limited to 'src') diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index a6d3cd6cca..0ebd2fcf88 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 99f291f174..315d5719a1 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 8e2d826930..e279e4bf29 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 5cdff9857f..c8f09be9b1 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 2f7e234d89..13cc925cd0 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 b3158cc821..27fbae883f 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 17dc4acde6bd15f3943cf69729964190a157e279 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(-) (limited to 'src') diff --git a/src/rabbit_auth_backend_dummy.erl b/src/rabbit_auth_backend_dummy.erl index e279e4bf29..d2905334c3 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 feb6d85ccb7d18e9762680e69a3b94c0439ebc9e 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. --- 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 +++- 7 files changed, 174 insertions(+), 120 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 (limited to 'src') diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index 0ebd2fcf88..d1577432af 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 315d5719a1..0000000000 --- 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 d2905334c3..d2f07c1d57 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 c8f09be9b1..20a5766d20 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 0000000000..cfc3f5db51 --- /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 0000000000..ff5f014e37 --- /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 27fbae883f..039568df8e 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()], -- cgit v1.2.1 From 2325ed570a7743fd85d44a54370a2228246d7195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= 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(-) (limited to 'src') diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index fa51dd70de..d3cacb1757 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 e7267bf3d34667836d52a133098382adda66362e 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(-) (limited to 'src') diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index d3cacb1757..8fbacdae92 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 a29e4c5187e6d99e05f3f5f9a04dd2d7d793732d 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(-) (limited to 'src') diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index 8fbacdae92..91a8b14062 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 9f180a6ae468c249484d5dc671e94763e280f311 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(-) (limited to 'src') diff --git a/src/rabbit.erl b/src/rabbit.erl index 664da20688..ca1d5ba8ed 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 From 5b15ac058dd9cfeea49f6dc70032bdc4faf3cdc2 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(-) (limited to 'src') diff --git a/src/rabbit.erl b/src/rabbit.erl index ca1d5ba8ed..40f24efc38 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