From 9adb6cd23905ce01d987ec3dfdef515768b4d208 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Fri, 22 Jul 2016 05:33:09 +0300 Subject: [PATCH 01/29] Keep track of connections, introduce per-vhost limits Fixes #500, #627. Squashed commit of the following: commit 88036dccbb28828ceed39d793b13a2d3d3b99b80 Author: Michael Klishin Date: Thu Jul 21 03:31:25 2016 +0300 Refactor commit fc84b7a23735352da4cf95726b430fad984b837d Merge: df745e2 df28c63 Author: Michael Klishin Date: Wed Jul 20 18:30:19 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit df745e2544824b882d174b99d1d4470d05ac78c8 Author: Michael Klishin Date: Wed Jul 20 18:04:59 2016 +0300 Force close connections when vhost is deleted Fixes #627, related to #500. commit 2167f8ffebe9473af482816822bb30a0694a1f3e Author: Michael Klishin Date: Wed Jul 20 16:00:35 2016 +0300 Add tests for per-vhost connection limits commit 2a032a3ac9cc3b01b07692456590e213e5d28806 Author: Michael Klishin Date: Wed Jul 20 01:53:07 2016 +0300 Rename a few tests commit 86ce592db1516bb216d6dc45326ea80f55d14a30 Author: Michael Klishin Date: Wed Jul 20 01:44:10 2016 +0300 Tests for connection re-registration idempotency commit a774c7bebe0d91c18af1a2035697c431dca28d89 Author: Michael Klishin Date: Tue Jul 19 04:05:20 2016 +0300 Ask nodes that come back to re-register their connections Depending on the partition handling mode used there may or may not be any clients still connected. We make sure that registration and deregistration functions are idempotent and assume there may be connections on the node that has come back. Point of improvement: when a node comes back up, N-1 nodes will tell it to re-register connections. It could be fewer than N-1, ideally just 1. commit 24e4c0e690f192d138e70000ada6335671275f0b Author: Michael Klishin Date: Mon Jul 18 17:05:17 2016 +0300 Fix boot step commit 62da3c6a73b96d4a07e69a9b1f5519ae708817a6 Author: Michael Klishin Date: Mon Jul 18 11:16:21 2016 +0300 Compile commit b656f9e96bc4a55072c2092289c52e972c44f754 Merge: f2831e1 492406e Author: Michael Klishin Date: Thu Jul 14 15:25:49 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit f2831e14cd12fc242b5280326ce1cf28a1fc9766 Merge: e5858e9 7b10a4e Author: Michael Klishin Date: Thu Jul 7 13:45:31 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit e5858e971825d50032e41794f68692c3d9ffa381 Author: Michael Klishin Date: Wed Jul 6 12:32:56 2016 +0300 Towards working connection re-registration after (inter-node) network splits commit 548df732f17d4e30268cef4c1b2046b8c03613ef Author: Michael Klishin Date: Wed Jul 6 12:32:07 2016 +0300 Make network split simulation work as expected commit 4028c660b96b31123f3a932d17d7b8a23b08cfb6 Author: Michael Klishin Date: Tue Jul 5 14:43:37 2016 +0300 Close connections using rabbit_ct_client_helpers Per discussion with @dumbbell. commit 26fecc97aa6c4368ecc7dba4464cca8f9ea08cfa Author: Michael Klishin Date: Tue Jul 5 04:17:52 2016 +0300 Extract connection limit partition tests into a separate suite commit 8a466f1b61e3cb07ba639d5f09d260223c0ff0a4 Author: Michael Klishin Date: Tue Jul 5 04:17:41 2016 +0300 Better logging commit b06de9b26ee13742290f13b32e293a990c3d5192 Author: Michael Klishin Date: Mon Jul 4 02:54:54 2016 +0300 Modify a test so that it (expectedly) fails commit 078a78ae00a88566e2b7068a63457e58e149e09e Author: Michael Klishin Date: Mon Jul 4 02:44:58 2016 +0300 Towards covering node termination/unavailability in connection tracking commit ab99361041fd58371ddd0c1a76ab2a37a3c47142 Author: Michael Klishin Date: Sun Jul 3 15:25:10 2016 +0300 These are moved to rabbit_ct_broker_helpers commit 520b6ef2b268e263fe4a0d33fbc578343c7ebf83 Author: Michael Klishin Date: Sun Jul 3 03:54:52 2016 +0300 {allow,block}_traffic_between/2 are moved to rabbit_ct_broker_helpers commit b842eaa616d55ca813012d6afc3e3d9d85acb46f Merge: 26eb1fa d4f031e Author: Michael Klishin Date: Sun Jul 3 03:14:27 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit 26eb1fa0ede083f67f7f7177064f0274ebcd8530 Author: Michael Klishin Date: Sun Jul 3 02:39:09 2016 +0300 dist_proxy helpers moved to rabbit_ct_broker_helpers commit 3d741f445be053222eaa73c973257114c17aea1c Author: Michael Klishin Date: Sun Jul 3 01:28:44 2016 +0300 Cluster node shutdown test commit 57c7129edf583120d3f20702ea68a8c2a73cf136 Author: Michael Klishin Date: Sat Jul 2 23:01:46 2016 +0300 Refactor commit b736b30724828027d77a34cddf9f4bcb17b1773d Author: Michael Klishin Date: Sat Jul 2 22:49:42 2016 +0300 More tests commit dc1cb5f0797cba5840fc1fa5e98a0d688383c713 Author: Michael Klishin Date: Sat Jul 2 22:27:16 2016 +0300 More tests commit e94edfed7a19add20545f40cd9dc78562546e163 Author: Michael Klishin Date: Sat Jul 2 17:08:34 2016 +0300 Initial per-vhost connection limit tests commit 15b7b4e271eedea9ce02bcbbd766609fa9fe970d Author: Michael Klishin Date: Sat Jul 2 15:04:57 2016 +0300 Adapt to master, compile commit dc7f3337a8a0a268c42857becdd92640deddb1a4 Merge: e4884ff bb1fa55 Author: Michael Klishin Date: Sat Jul 2 02:44:18 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit e4884ffb29452fcdbd11ace1ac7b4d1b7d506b03 Merge: 71e2710 f0f43f8 Author: Michael Klishin Date: Wed Jun 29 14:27:40 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit 71e2710948d3a531c5426b18d63367fddf98ff55 Merge: b1ec9f3 704a2b5 Author: Michael Klishin Date: Thu Mar 31 01:55:29 2016 +0300 Merge branch 'master' into rabbitmq-server-500 Conflicts: src/rabbit_control_main.erl src/rabbit_types.erl commit b1ec9f30c4b896f227d3e8800d60b2934996f39e Author: Michael Klishin Date: Mon Feb 15 13:51:37 2016 +0300 Stub out event handlers for #627 and #628 commit f3cfb57e2e83436e51d7d065edd04cc9197b6539 Author: Michael Klishin Date: Sat Feb 13 01:33:50 2016 +0300 Use a counter column to track number of connections per vhost Limit query time is now 50-70 microseconds for 50M connections. commit e9132f11253972ff6b0cc09529e9cd39ba67c140 Author: Michael Klishin Date: Fri Feb 12 06:23:51 2016 +0300 Ignore ./debug commit 976e3ae10ba7e81c3ead1d2bd1fc852b0f9e3004 Author: Michael Klishin Date: Fri Feb 12 06:20:01 2016 +0300 Switch to ets:select_count/2 commit ec23cf15ad840da46c321e965081815173989197 Author: Michael Klishin Date: Thu Feb 11 05:11:08 2016 +0300 Enforce max connection limit Also introduce `rabbitmqctl clear_vhost_limits` and fix rabbitmqctl(1). commit ba20887832170f9c9b88e19cad06e0995f887b5b Merge: 49a1886 8974581 Author: Michael Klishin Date: Thu Feb 11 02:16:24 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit 49a18867faf6e38c08ad12635e716d7ff53b0529 Author: Michael Klishin Date: Wed Feb 10 16:45:00 2016 +0300 Spelling commit 723e6e4e412b81e47f3a9db5451a1342debcf79e Author: Michael Klishin Date: Wed Feb 10 16:31:34 2016 +0300 Create secondary indices on rabbit_tracked_connection.vhost and username commit b468c0fa05ce51cebff19098b500ccc217542f9f Merge: 6940d05 0120438 Author: Michael Klishin Date: Wed Feb 10 12:23:38 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit 6940d059cf3f844b2aab9cfe00652f23f9a6ad06 Author: Michael Klishin Date: Mon Feb 8 01:30:03 2016 +0300 Spam commit 032c2a67f59889dbec6a3ca6c5af23920a0d7cb2 Merge: 46da39c 2374ae8 Author: Michael Klishin Date: Fri Feb 5 23:48:38 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit 46da39c5f5e801939a61b476e577636d30eb6e54 Merge: 655e351 05361e6 Author: Michael Klishin Date: Wed Feb 3 11:20:08 2016 +0300 Merge branch 'master' into rabbitmq-server-500 commit 655e3512a031c5f57c8c09c5432a5d7671acc6af Author: Michael Klishin Date: Wed Feb 3 11:19:23 2016 +0300 Store and delete tracked connections in a table commit 4e849cf936f414a11bf5da3135e7a79c75413642 Author: Michael Klishin Date: Tue Jan 19 17:56:14 2016 +0300 Compile commit 504adde27cfbdb4f81ddd0e4e25abda23b5f138e Author: Michael Klishin Date: Tue Jan 19 17:55:08 2016 +0300 Switch to a handler for connection tracking (WIP) commit 3e1d2b4f65608e2c853ad7be0580050ee38fa4e7 Author: Michael Klishin Date: Tue Jan 19 14:46:09 2016 +0300 Migrations for virtual host limits and tracked connections commit 7499020af6fa8f2685cbee37ad77f189af6ee1e6 Author: Michael Klishin Date: Fri Jan 8 19:40:36 2016 +0300 Compile commit f3a11012f05d64d6d62fd5ec45a38fafaef47c49 Author: Michael Klishin Date: Fri Jan 8 19:14:12 2016 +0300 Switch rabbitmqctl set_vhost_limits to use JSON payload values Just like policies do. commit 7fc5f1a074ab5a34c33792a8ba25aa107eb0d993 Author: Michael Klishin Date: Wed Jan 6 19:07:50 2016 +0300 Stub out set_vhost_limits in ctl --- docs/rabbitmqctl.1.xml | 55 +- include/rabbit_cli.hrl | 1 - src/rabbit.erl | 5 + src/rabbit_connection_tracker.erl | 99 +++ src/rabbit_connection_tracking.erl | 229 ++++++ src/rabbit_connection_tracking_handler.erl | 101 +++ src/rabbit_control_main.erl | 15 + src/rabbit_mnesia.erl | 4 +- src/rabbit_node_monitor.erl | 4 +- src/rabbit_table.erl | 32 +- src/rabbit_upgrade_functions.erl | 39 ++ src/rabbit_vhost.erl | 31 +- src/rabbit_vhost_limit.erl | 98 +++ test/per_vhost_connection_limit_SUITE.erl | 660 ++++++++++++++++++ ...host_connection_limit_partitions_SUITE.erl | 164 +++++ 15 files changed, 1526 insertions(+), 11 deletions(-) create mode 100644 src/rabbit_connection_tracker.erl create mode 100644 src/rabbit_connection_tracking.erl create mode 100644 src/rabbit_connection_tracking_handler.erl create mode 100644 src/rabbit_vhost_limit.erl create mode 100644 test/per_vhost_connection_limit_SUITE.erl create mode 100644 test/per_vhost_connection_limit_partitions_SUITE.erl diff --git a/docs/rabbitmqctl.1.xml b/docs/rabbitmqctl.1.xml index 363748e046d7..9cd1198a28c0 100644 --- a/docs/rabbitmqctl.1.xml +++ b/docs/rabbitmqctl.1.xml @@ -1214,6 +1214,55 @@ + + Virtual Host Limits + + It is possible to enforce certain limits on virtual hosts. + + + + set_vhost_limits -p vhostpath definition + + + Sets virtual host limits + + + + definition + + The definition of the limits, as a + JSON term. In most shells you are very likely to + need to quote this. + + Recognised limits: max-connections (0 means "no limit"). + + + + For example: + rabbitmqctl set_vhost_limits -p qa_env '{"max-connections": 1024}' + + This command limits the max number of concurrent connections in vhost qa_env + to 1024. + + + + + + clear_vhost_limits -p vhostpath + + + Clears virtual host limits + + For example: + rabbitmqctl clear_vhost_limits -p qa_env + + This command clears vhost limits in vhost qa_env. + + + + + + Server Status @@ -2036,9 +2085,9 @@ fraction - Limit relative to the total amount available RAM - as a non-negative floating point number. - Values lower than 1.0 can be dangerous and + Limit relative to the total amount available RAM + as a non-negative floating point number. + Values lower than 1.0 can be dangerous and should be used carefully. diff --git a/include/rabbit_cli.hrl b/include/rabbit_cli.hrl index a0d1ecfdd519..0e338855cd00 100644 --- a/include/rabbit_cli.hrl +++ b/include/rabbit_cli.hrl @@ -30,7 +30,6 @@ -define(OFFLINE_OPT, "--offline"). -define(ONLINE_OPT, "--online"). - -define(NODE_DEF(Node), {?NODE_OPT, {option, Node}}). -define(QUIET_DEF, {?QUIET_OPT, flag}). -define(VHOST_DEF, {?VHOST_OPT, {option, "/"}}). diff --git a/src/rabbit.erl b/src/rabbit.erl index 2fa4cdee71ef..59ede3c802de 100644 --- a/src/rabbit.erl +++ b/src/rabbit.erl @@ -178,6 +178,11 @@ {mfa, {rabbit_direct, boot, []}}, {requires, log_relay}]}). +-rabbit_boot_step({connection_tracker, + [{description, "helps track node-local connections"}, + {mfa, {rabbit_connection_tracker, boot, []}}, + {requires, log_relay}]}). + -rabbit_boot_step({networking, [{mfa, {rabbit_networking, boot, []}}, {requires, log_relay}]}). diff --git a/src/rabbit_connection_tracker.erl b/src/rabbit_connection_tracker.erl new file mode 100644 index 000000000000..0177627d8316 --- /dev/null +++ b/src/rabbit_connection_tracker.erl @@ -0,0 +1,99 @@ +%% 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-2016 Pivotal Software, Inc. All rights reserved. +%% + +-module(rabbit_connection_tracker). + +%% Abstracts away how tracked connection records are stored +%% and queried. +%% +%% See also: +%% +%% * rabbit_connection_tracking_handler +%% * rabbit_reader +%% * rabbit_event + +-behaviour(gen_server2). + +%% API +-export([boot/0, start_link/0, reregister/1]). + +%% gen_fsm callbacks +-export([init/1, + handle_call/3, + handle_cast/2, + handle_info/2, + terminate/2, + code_change/3]). + +-define(SERVER, ?MODULE). + + +%%%=================================================================== +%%% API +%%%=================================================================== + +boot() -> + {ok, _} = start_link(), + ok. + +start_link() -> + gen_server2:start_link({local, ?SERVER}, ?MODULE, [], []). + +reregister(Node) -> + rabbit_log:info("Telling node ~p to re-register tracked connections", [Node]), + gen_server2:cast({?SERVER, Node}, reregister). + +%%%=================================================================== +%%% gen_server callbacks +%%%=================================================================== + +init([]) -> + {ok, {}}. + +handle_call(_Req, _From, State) -> + {noreply, State}. + +handle_cast(reregister, State) -> + Cs = rabbit_networking:connections_local(), + rabbit_log:info("Connection tracker: asked to re-register ~p client connections", [length(Cs)]), + case Cs of + [] -> ok; + Cs -> + [reregister_connection(C) || C <- Cs], + ok + end, + rabbit_log:info("Done re-registering client connections"), + {noreply, State}. + +handle_info(_Req, State) -> + {noreply, State}. + +terminate(_Reason, _State) -> + ok. + +code_change(_OldVsn, State, _Extra) -> + {ok, State}. + +%%%=================================================================== +%%% Internal functions +%%%=================================================================== + +reregister_connection(Conn) -> + try + Conn ! reregister + catch _:Error -> + rabbit_log:error("Failed to re-register connection ~p after a network split: ~p", [Conn, Error]) + end. diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl new file mode 100644 index 000000000000..c65023a2a850 --- /dev/null +++ b/src/rabbit_connection_tracking.erl @@ -0,0 +1,229 @@ +%% 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-2016 Pivotal Software, Inc. All rights reserved. +%% + +-module(rabbit_connection_tracking). + +%% Abstracts away how tracked connection records are stored +%% and queried. +%% +%% See also: +%% +%% * rabbit_connection_tracking_handler +%% * rabbit_reader +%% * rabbit_event + +-export([register_connection/1, unregister_connection/1, + list/0, list/1, list_on_node/1, + tracked_connection_from_connection_created/1, + tracked_connection_from_connection_state/1, + is_over_connection_limit/1, count_connections_in/1, + on_node_down/1, on_node_up/1]). + +-include_lib("rabbit.hrl"). + +-define(TABLE, rabbit_tracked_connection). +-define(PER_VHOST_COUNTER_TABLE, rabbit_tracked_connection_per_vhost). +-define(SERVER, ?MODULE). + +%% +%% API +%% + +-spec register_connection(rabbit_types:tracked_connection()) -> ok. + +register_connection(#tracked_connection{vhost = VHost, id = ConnId} = Conn) -> + rabbit_misc:execute_mnesia_transaction( + fun() -> + %% upsert + case mnesia:dirty_read(?TABLE, ConnId) of + [] -> + mnesia:write(?TABLE, Conn, write), + mnesia:dirty_update_counter( + rabbit_tracked_connection_per_vhost, VHost, 1); + [_Row] -> + ok + end, + ok + end). + +-spec unregister_connection(rabbit_types:connection_name()) -> ok. + +unregister_connection(ConnId = {_Node, _Name}) -> + rabbit_misc:execute_mnesia_transaction( + fun() -> + case mnesia:dirty_read(?TABLE, ConnId) of + [] -> ok; + [Row] -> + mnesia:dirty_update_counter( + ?PER_VHOST_COUNTER_TABLE, + Row#tracked_connection.vhost, -1), + mnesia:delete({?TABLE, ConnId}) + end + end). + + +-spec list() -> [rabbit_types:tracked_connection()]. + +list() -> + mnesia:dirty_match_object(?TABLE, #tracked_connection{_ = '_'}). + + +-spec list(rabbit_types:vhost()) -> [rabbit_types:tracked_connection()]. + +list(VHost) -> + mnesia:dirty_match_object(?TABLE, #tracked_connection{vhost = VHost, _ = '_'}). + + +-spec list_on_node(node()) -> [rabbit_types:tracked_connection()]. + +list_on_node(Node) -> + mnesia:dirty_match_object(?TABLE, #tracked_connection{node = Node, _ = '_'}). + + +-spec on_node_down(node()) -> ok. + +on_node_down(Node) -> + case lists:member(Node, nodes()) of + false -> + Cs = list_on_node(Node), + rabbit_log:info( + "Node ~p is down, unregistering ~p client connections~n", + [Node, length(Cs)]), + [unregister_connection(Id) || #tracked_connection{id = Id} <- Cs], + ok; + true -> rabbit_log:info( + "Keeping ~s connections: the node is already back~n", [Node]) + end. + +-spec on_node_up(node()) -> ok. +on_node_up(Node) -> + rabbit_connection_tracker:reregister(Node), + ok. + +-spec is_over_connection_limit(rabbit_types:vhost()) -> boolean(). + +is_over_connection_limit(VirtualHost) -> + ConnectionCount = count_connections_in(VirtualHost), + case rabbit_vhost_limit:connection_limit(VirtualHost) of + undefined -> false; + {ok, Limit} -> case {ConnectionCount, ConnectionCount >= Limit} of + %% 0 = no limit + {0, _} -> false; + %% the limit hasn't been reached + {_, false} -> false; + {_N, true} -> {true, Limit} + end + end. + + +-spec count_connections_in(rabbit_types:vhost()) -> non_neg_integer(). + +count_connections_in(VirtualHost) -> + try + case mnesia:transaction( + fun() -> + case mnesia:dirty_read( + {?PER_VHOST_COUNTER_TABLE, + VirtualHost}) of + [] -> 0; + [Val] -> + Val#tracked_connection_per_vhost.connection_count + end + end) of + {atomic, Val} -> Val; + {aborted, _Reason} -> 0 + end + catch + _:Err -> + rabbit_log:error( + "Failed to fetch number of connections in vhost ~p:~n~p~n", + [VirtualHost, Err]), + 0 + end. + +%% Returns a #tracked_connection from connection_created +%% event details. +%% +%% @see rabbit_connection_tracking_handler. +tracked_connection_from_connection_created(EventDetails) -> + %% Example event: + %% + %% [{type,network}, + %% {pid,<0.329.0>}, + %% {name,<<"127.0.0.1:60998 -> 127.0.0.1:5672">>}, + %% {port,5672}, + %% {peer_port,60998}, + %% {host,{0,0,0,0,0,65535,32512,1}}, + %% {peer_host,{0,0,0,0,0,65535,32512,1}}, + %% {ssl,false}, + %% {peer_cert_subject,''}, + %% {peer_cert_issuer,''}, + %% {peer_cert_validity,''}, + %% {auth_mechanism,<<"PLAIN">>}, + %% {ssl_protocol,''}, + %% {ssl_key_exchange,''}, + %% {ssl_cipher,''}, + %% {ssl_hash,''}, + %% {protocol,{0,9,1}}, + %% {user,<<"guest">>}, + %% {vhost,<<"/">>}, + %% {timeout,14}, + %% {frame_max,131072}, + %% {channel_max,65535}, + %% {client_properties, + %% [{<<"capabilities">>,table, + %% [{<<"publisher_confirms">>,bool,true}, + %% {<<"consumer_cancel_notify">>,bool,true}, + %% {<<"exchange_exchange_bindings">>,bool,true}, + %% {<<"basic.nack">>,bool,true}, + %% {<<"connection.blocked">>,bool,true}, + %% {<<"authentication_failure_close">>,bool,true}]}, + %% {<<"product">>,longstr,<<"Bunny">>}, + %% {<<"platform">>,longstr, + %% <<"ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin15]">>}, + %% {<<"version">>,longstr,<<"2.3.0.pre">>}, + %% {<<"information">>,longstr, + %% <<"http://rubybunny.info">>}]}, + %% {connected_at,1453214290847}] + Name = proplists:get_value(name, EventDetails), + Node = proplists:get_value(node, EventDetails), + #tracked_connection{id = {Node, Name}, + name = Name, + node = Node, + vhost = proplists:get_value(vhost, EventDetails), + username = proplists:get_value(user, EventDetails), + connected_at = proplists:get_value(connected_at, EventDetails), + pid = proplists:get_value(pid, EventDetails), + peer_host = proplists:get_value(peer_host, EventDetails), + peer_port = proplists:get_value(peer_port, EventDetails)}. + +tracked_connection_from_connection_state(#connection{ + vhost = VHost, + connected_at = Ts, + peer_host = PeerHost, + peer_port = PeerPort, + user = Username, + name = Name + }) -> + tracked_connection_from_connection_created( + [{name, Name}, + {node, node()}, + {vhost, VHost}, + {user, Username}, + {connected_at, Ts}, + {pid, self()}, + {peer_port, PeerPort}, + {peer_host, PeerHost}]). diff --git a/src/rabbit_connection_tracking_handler.erl b/src/rabbit_connection_tracking_handler.erl new file mode 100644 index 000000000000..deaf9bc7f666 --- /dev/null +++ b/src/rabbit_connection_tracking_handler.erl @@ -0,0 +1,101 @@ +%% 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-2016 Pivotal Software, Inc. All rights reserved. +%% + +-module(rabbit_connection_tracking_handler). + +%% This module keeps track of connection creation and termination events +%% on its local node. The primary goal here is to decouple connection +%% tracking from rabbit_reader in rabbit_common. +%% +%% Events from other nodes are ignored. + +%% This module keeps track of connection creation and termination events +%% on its local node. The primary goal here is to decouple connection +%% tracking from rabbit_reader in rabbit_common. +%% +%% Events from other nodes are ignored. + +-behaviour(gen_event). + +-export([init/1, handle_call/2, handle_event/2, handle_info/2, + terminate/2, code_change/3]). + +-include_lib("rabbit.hrl"). + +-rabbit_boot_step({?MODULE, + [{description, "connection tracking event handler"}, + {mfa, {gen_event, add_handler, + [rabbit_event, ?MODULE, []]}}, + {cleanup, {gen_event, delete_handler, + [rabbit_event, ?MODULE, []]}}, + {requires, [rabbit_event, rabbit_node_monitor]}, + {enables, recovery}]}). + + +%% +%% API +%% + +init([]) -> + {ok, []}. + +handle_event(#event{type = connection_created, props = Details}, State) -> + rabbit_connection_tracking:register_connection( + rabbit_connection_tracking:tracked_connection_from_connection_created(Details) + ), + {ok, State}; +%% see rabbit_reader +handle_event(#event{type = connection_reregistered, props = [{state, ConnState}]}, State) -> + rabbit_connection_tracking:register_connection( + rabbit_connection_tracking:tracked_connection_from_connection_state(ConnState) + ), + {ok, State}; +handle_event(#event{type = connection_closed, props = Details}, State) -> + %% [{name,<<"127.0.0.1:64078 -> 127.0.0.1:5672">>}, + %% {pid,<0.1774.0>}, + %% {node, rabbit@hostname}] + rabbit_connection_tracking:unregister_connection( + {proplists:get_value(node, Details), + proplists:get_value(name, Details)}), + {ok, State}; +handle_event(#event{type = vhost_deleted, props = Details}, State) -> + VHost = proplists:get_value(name, Details), + rabbit_log_connection:info("Closing all connections in vhost '~s' because it's being deleted", [VHost]), + case rabbit_connection_tracking:list(VHost) of + [] -> {ok, State}; + Cs -> + [rabbit_networking:close_connection(Pid, rabbit_misc:format("vhost '~s' is deleted", [VHost])) || #tracked_connection{pid = Pid} <- Cs], + {ok, State} + end; +handle_event(#event{type = user_deleted, props = Details}, State) -> + _Username = proplists:get_value(name, Details), + %% TODO: force close and unregister connections from + %% this user. Moved to rabbitmq/rabbitmq-server#628. + {ok, State}; +handle_event(_Event, State) -> + {ok, State}. + +handle_call(_Request, State) -> + {ok, not_understood, State}. + +handle_info(_Info, State) -> + {ok, State}. + +terminate(_Arg, _State) -> + ok. + +code_change(_OldVsn, State, _Extra) -> + {ok, State}. diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index 7f410ac7523b..c56e519fe5e0 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -74,6 +74,9 @@ {clear_policy, [?VHOST_DEF]}, {list_policies, [?VHOST_DEF]}, + {set_vhost_limits, [?VHOST_DEF]}, + {clear_vhost_limits, [?VHOST_DEF]}, + {list_queues, [?VHOST_DEF, ?OFFLINE_DEF, ?ONLINE_DEF]}, {list_exchanges, [?VHOST_DEF]}, {list_bindings, [?VHOST_DEF]}, @@ -544,6 +547,18 @@ action(clear_policy, Node, [Key], Opts, Inform) -> Inform("Clearing policy ~p", [Key]), rpc_call(Node, rabbit_policy, delete, [VHostArg, list_to_binary(Key)]); +action(set_vhost_limits, Node, [Defn], Opts, Inform) -> + Msg = "Setting vhost limits for vhost ~p", + VHostArg = list_to_binary(proplists:get_value(?VHOST_OPT, Opts)), + Inform(Msg, [VHostArg]), + rpc_call(Node, rabbit_vhost_limit, parse_set, [VHostArg, Defn]), + ok; + +action(clear_vhost_limits, Node, [], Opts, Inform) -> + VHostArg = list_to_binary(proplists:get_value(?VHOST_OPT, Opts)), + Inform("Clearing vhost ~p limits", [VHostArg]), + rpc_call(Node, rabbit_vhost_limit, clear, [VHostArg]); + action(report, Node, _Args, _Opts, Inform) -> Inform("Reporting server status on ~p~n~n", [erlang:universaltime()]), [begin ok = action(Action, N, [], [], Inform), io:nl() end || diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index 26a864f0f567..43d268c37472 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -466,12 +466,14 @@ init_db(ClusterNodes, NodeType, CheckOtherNodes) -> {[], true, disc} -> %% First disc node up maybe_force_load(), + ok = rabbit_table:ensure_secondary_indices(), ok; {[_ | _], _, _} -> %% Subsequent node in cluster, catch up maybe_force_load(), ok = rabbit_table:wait_for_replicated(), - ok = rabbit_table:create_local_copy(NodeType) + ok = rabbit_table:create_local_copy(NodeType), + ok = rabbit_table:ensure_secondary_indices() end, ensure_schema_integrity(), rabbit_node_monitor:update_cluster_status(), diff --git a/src/rabbit_node_monitor.erl b/src/rabbit_node_monitor.erl index 0322aacfd151..9ed790c75dcb 100644 --- a/src/rabbit_node_monitor.erl +++ b/src/rabbit_node_monitor.erl @@ -732,6 +732,7 @@ handle_dead_rabbit(Node, State = #state{partitions = Partitions, ok = rabbit_amqqueue:on_node_down(Node), ok = rabbit_alarm:on_node_down(Node), ok = rabbit_mnesia:on_node_down(Node), + ok = rabbit_connection_tracking:on_node_down(Node), %% If we have been partitioned, and we are now in the only remaining %% partition, we no longer care about partitions - forget them. Note %% that we do not attempt to deal with individual (other) partitions @@ -760,7 +761,8 @@ ensure_keepalive_timer(State) -> handle_live_rabbit(Node) -> ok = rabbit_amqqueue:on_node_up(Node), ok = rabbit_alarm:on_node_up(Node), - ok = rabbit_mnesia:on_node_up(Node). + ok = rabbit_mnesia:on_node_up(Node), + ok = rabbit_connection_tracking:on_node_up(Node). maybe_autoheal(State = #state{partitions = []}) -> State; diff --git a/src/rabbit_table.erl b/src/rabbit_table.erl index 390909696499..60996c153912 100644 --- a/src/rabbit_table.erl +++ b/src/rabbit_table.erl @@ -18,7 +18,8 @@ -export([create/0, create_local_copy/1, wait_for_replicated/0, wait/1, force_load/0, is_present/0, is_empty/0, needs_default_data/0, - check_schema_integrity/0, clear_ram_only_tables/0, wait_timeout/0]). + check_schema_integrity/0, clear_ram_only_tables/0, wait_timeout/0, + ensure_secondary_indices/0, ensure_secondary_indices/2]). -include("rabbit.hrl"). @@ -50,6 +51,7 @@ create() -> Tab, TabDef1, Reason}}) end end, definitions()), + ok = rabbit_table:ensure_secondary_indices(), ok. %% The sequence in which we delete the schema and then the other @@ -63,6 +65,13 @@ create_local_copy(ram) -> create_local_copies(ram), create_local_copy(schema, ram_copies). +ensure_secondary_indices() -> + ensure_secondary_indices(rabbit_tracked_connection, [vhost, username]), + ok. + +ensure_secondary_indices(Tab, Fields) -> + [mnesia:add_table_index(Tab, Field) || Field <- Fields]. + wait_for_replicated() -> wait([Tab || {Tab, TabDef} <- definitions(), not lists:member({local_content, true}, TabDef)]). @@ -297,9 +306,24 @@ definitions() -> {rabbit_queue, [{record_name, amqqueue}, {attributes, record_info(fields, amqqueue)}, - {match, #amqqueue{name = queue_name_match(), _='_'}}]}] - ++ gm:table_definitions() - ++ mirrored_supervisor:table_definitions(). + {match, #amqqueue{name = queue_name_match(), _='_'}}]}, + + %% Used to track connections across virtual hosts + %% e.g. so that limits can be enforced. + %% + %% All data in this table is transient. + {rabbit_tracked_connection, + [{record_name, tracked_connection}, + {attributes, record_info(fields, tracked_connection)}, + {match, #tracked_connection{_ = '_'}}]}, + + {rabbit_tracked_connection_per_vhost, + [{record_name, tracked_connection_per_vhost}, + {attributes, record_info(fields, tracked_connection_per_vhost)}, + {match, #tracked_connection_per_vhost{_ = '_'}}]} + + ] ++ gm:table_definitions() + ++ mirrored_supervisor:table_definitions(). binding_match() -> #binding{source = exchange_name_match(), diff --git a/src/rabbit_upgrade_functions.erl b/src/rabbit_upgrade_functions.erl index 3d624752ea65..47053fafe731 100644 --- a/src/rabbit_upgrade_functions.erl +++ b/src/rabbit_upgrade_functions.erl @@ -55,6 +55,9 @@ -rabbit_upgrade({policy_version, mnesia, [recoverable_slaves]}). -rabbit_upgrade({slave_pids_pending_shutdown, mnesia, [policy_version]}). -rabbit_upgrade({user_password_hashing, mnesia, [hash_passwords]}). +-rabbit_upgrade({vhost_limits, mnesia, []}). +-rabbit_upgrade({tracked_connection, mnesia, [vhost_limits]}). +-rabbit_upgrade({tracked_connection_per_vhost, mnesia, [tracked_connection]}). %% ------------------------------------------------------------------- @@ -88,9 +91,36 @@ -spec queue_state() -> 'ok'. -spec recoverable_slaves() -> 'ok'. -spec user_password_hashing() -> 'ok'. +-spec vhost_limits() -> 'ok'. +-spec tracked_connection() -> 'ok'. +-spec tracked_connection_per_vhost() -> 'ok'. + %%-------------------------------------------------------------------- +tracked_connection() -> + create(rabbit_tracked_connection, [{record_name, tracked_connection}, + {attributes, [id, node, vhost, name, + pid, protocol, + peer_host, peer_port, + username, connected_at]}]). + +tracked_connection_per_vhost() -> + create(tracked_connection_per_vhost, [{record_name, tracked_connection_per_vhost}, + {attributes, [vhost, connection_count]}]). + +%% replaces vhost.dummy (used to avoid having a single-field record +%% which Mnesia doesn't like) with vhost.limits (which is actually +%% used) +vhost_limits() -> + io:format("vhost_limits vhost_limits vhost_limits~n"), + transform( + rabbit_vhost, + fun ({vhost, VHost, _Dummy}) -> + {vhost, VHost, undefined} + end, + [virtual_host, limits]). + %% It's a bad idea to use records or record_info here, even for the %% destination form. Because in the future, the destination form of %% your current transform may not match the record any more, and it @@ -510,6 +540,11 @@ create(Tab, TabDef) -> {atomic, ok} = mnesia:create_table(Tab, TabDef), ok. +create(Tab, TabDef, SecondaryIndices) -> + {atomic, ok} = mnesia:create_table(Tab, TabDef), + [mnesia:add_table_index(Tab, Idx) || Idx <- SecondaryIndices], + ok. + %% Dumb replacement for rabbit_exchange:declare that does not require %% the exchange type registry or worker pool to be running by dint of %% not validating anything and assuming the exchange type does not @@ -518,3 +553,7 @@ create(Tab, TabDef) -> declare_exchange(XName, Type) -> X = {exchange, XName, Type, true, false, false, []}, ok = mnesia:dirty_write(rabbit_durable_exchange, X). + +add_indices(Tab, FieldList) -> + [mnesia:add_table_index(Tab, Field) || Field <- FieldList], + ok. diff --git a/src/rabbit_vhost.erl b/src/rabbit_vhost.erl index df2f8423b48a..01f1046fb8b9 100644 --- a/src/rabbit_vhost.erl +++ b/src/rabbit_vhost.erl @@ -20,11 +20,14 @@ %%---------------------------------------------------------------------------- --export([add/1, delete/1, exists/1, list/0, with/2, assert/1]). +-export([add/1, delete/1, exists/1, list/0, with/2, assert/1, update/2, + set_limits/2, limits_of/1]). -export([info/1, info/2, info_all/0, info_all/1, info_all/2, info_all/3]). + -spec add(rabbit_types:vhost()) -> 'ok'. -spec delete(rabbit_types:vhost()) -> 'ok'. +-spec update(rabbit_types:vhost(), rabbit_misc:thunk(A)) -> A. -spec exists(rabbit_types:vhost()) -> boolean(). -spec list() -> [rabbit_types:vhost()]. -spec with(rabbit_types:vhost(), rabbit_misc:thunk(A)) -> A. @@ -138,6 +141,32 @@ assert(VHostPath) -> case exists(VHostPath) of false -> throw({error, {no_such_vhost, VHostPath}}) end. +update(VHostPath, Fun) -> + case mnesia:read({rabbit_vhost, VHostPath}) of + [] -> + mnesia:abort({no_such_vhost, VHostPath}); + [V] -> + V1 = Fun(V), + ok = mnesia:write(rabbit_vhost, V1, write), + V1 + end. + +limits_of(VHostPath) when is_binary(VHostPath) -> + assert(VHostPath), + case mnesia:dirty_read({rabbit_vhost, VHostPath}) of + [] -> + mnesia:abort({no_such_vhost, VHostPath}); + [#vhost{limits = Limits}] -> + Limits + end; +limits_of(#vhost{virtual_host = Name}) -> + limits_of(Name). + +set_limits(VHost = #vhost{}, undefined) -> + VHost#vhost{limits = undefined}; +set_limits(VHost = #vhost{}, Limits) -> + VHost#vhost{limits = Limits}. + %%---------------------------------------------------------------------------- infos(Items, X) -> [{Item, i(Item, X)} || Item <- Items]. diff --git a/src/rabbit_vhost_limit.erl b/src/rabbit_vhost_limit.erl new file mode 100644 index 000000000000..dd7ce51089f4 --- /dev/null +++ b/src/rabbit_vhost_limit.erl @@ -0,0 +1,98 @@ +%% 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-2016 Pivotal Software, Inc. All rights reserved. +%% + +-module(rabbit_vhost_limit). + +-behaviour(rabbit_runtime_parameter). + +-include("rabbit.hrl"). + +-export([register/0]). +-export([parse_set/2, clear/1]). +-export([validate/5, notify/4, notify_clear/3]). +-export([connection_limit/1]). + +-import(rabbit_misc, [pget/2]). + +-rabbit_boot_step({?MODULE, + [{description, "vhost limit parameters"}, + {mfa, {rabbit_vhost_limit, register, []}}, + {requires, rabbit_registry}, + {enables, recovery}]}). + +%%---------------------------------------------------------------------------- + +register() -> + rabbit_registry:register(runtime_parameter, <<"vhost-limits">>, ?MODULE). + +validate(_VHost, <<"vhost-limits">>, Name, Term, _User) -> + rabbit_parameter_validation:proplist( + Name, vhost_limit_validation(), Term). + +notify(VHost, <<"vhost-limits">>, <<"limits">>, Limits) -> + rabbit_event:notify(vhost_limits_set, [{name, <<"limits">>} | Limits]), + update_vhost(VHost, Limits). + +notify_clear(VHost, <<"vhost-limits">>, <<"limits">>) -> + rabbit_event:notify(vhost_limits_cleared, [{name, <<"limits">>}]), + update_vhost(VHost, undefined). + +connection_limit(VirtualHost) -> + get_limit(VirtualHost, <<"max-connections">>). + +%%---------------------------------------------------------------------------- + +parse_set(VHost, Defn) -> + case rabbit_misc:json_decode(Defn) of + {ok, JSON} -> + set(VHost, rabbit_misc:json_to_term(JSON)); + error -> + {error_string, "JSON decoding error"} + end. + +set(VHost, Defn) -> + rabbit_runtime_parameters:set_any(VHost, <<"vhost-limits">>, + <<"limits">>, Defn, none). + +clear(VHost) -> + rabbit_runtime_parameters:clear_any(VHost, <<"vhost-limits">>, + <<"limits">>). + +vhost_limit_validation() -> + [{<<"max-connections">>, fun rabbit_parameter_validation:number/2, mandatory}]. + +update_vhost(VHostName, Limits) -> + rabbit_misc:execute_mnesia_transaction( + fun() -> + rabbit_vhost:update(VHostName, + fun(VHost) -> + rabbit_vhost:set_limits(VHost, Limits) + end) + end), + ok. + +get_limit(VirtualHost, Limit) -> + case rabbit_runtime_parameters:list(VirtualHost, <<"vhost-limits">>) of + [] -> undefined; + [Param] -> case pget(value, Param) of + undefined -> undefined; + Val -> case pget(Limit, Val) of + undefined -> undefined; + N when N =< 0 -> undefined; + N when N > 0 -> {ok, N} + end + end + end. diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl new file mode 100644 index 000000000000..0b1f5adf8495 --- /dev/null +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -0,0 +1,660 @@ +%% 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) 2011-2016 Pivotal Software, Inc. All rights reserved. +%% + +-module(per_vhost_connection_limit_SUITE). + +-include_lib("common_test/include/ct.hrl"). +-include_lib("amqp_client/include/amqp_client.hrl"). +-include_lib("eunit/include/eunit.hrl"). + +-compile(export_all). + +-import(rabbit_ct_client_helpers, [open_unmanaged_connection/2, + open_unmanaged_connection/3]). + + +all() -> + [ + {group, cluster_size_1}, + {group, cluster_size_2} + ]. + +groups() -> + [ + {cluster_size_1, [], [ + most_basic_single_node_connection_count_test, + single_node_single_vhost_connection_count_test, + single_node_multiple_vhost_connection_count_test, + single_node_list_in_vhost_test, + single_node_connection_reregistration_idempotency_test, + single_node_single_vhost_limit_test, + single_node_multiple_vhost_limit_test, + single_node_vhost_deletion_forces_connection_closure_test + ]}, + {cluster_size_2, [], [ + most_basic_cluster_connection_count_test, + cluster_single_vhost_connection_count_test, + cluster_multiple_vhost_connection_count_test, + cluster_node_restart_connection_count_test, + cluster_node_list_on_node_test, + cluster_connection_reregistration_idempotency_test, + cluster_single_vhost_limit_test + ]} + ]. + +%% see partitions_SUITE +-define(DELAY, 9000). + +%% ------------------------------------------------------------------- +%% Testsuite setup/teardown. +%% ------------------------------------------------------------------- + +init_per_suite(Config) -> + rabbit_ct_helpers:log_environment(), + rabbit_ct_helpers:run_setup_steps(Config, [ + fun rabbit_ct_broker_helpers:enable_dist_proxy_manager/1 + ]). + +end_per_suite(Config) -> + rabbit_ct_helpers:run_teardown_steps(Config). + +init_per_group(cluster_size_1, Config) -> + Suffix = rabbit_ct_helpers:testcase_absname(Config, "", "-"), + Config1 = rabbit_ct_helpers:set_config(Config, [ + {rmq_nodes_count, 1}, + {rmq_nodename_suffix, Suffix} + ]), + rabbit_ct_helpers:run_steps(Config1, + rabbit_ct_broker_helpers:setup_steps() ++ + rabbit_ct_client_helpers:setup_steps()); +init_per_group(cluster_size_2, Config) -> + init_per_multinode_group(cluster_size_2, Config, 2); +init_per_group(partition_handling, Config) -> + Config1 = rabbit_ct_helpers:set_config(Config, [{net_ticktime, 1}]), + init_per_multinode_group(partition_handling, Config1, 3). + +init_per_multinode_group(_GroupName, Config, NodeCount) -> + Suffix = rabbit_ct_helpers:testcase_absname(Config, "", "-"), + Config1 = rabbit_ct_helpers:set_config(Config, [ + {rmq_nodes_count, NodeCount}, + {rmq_nodename_suffix, Suffix} + ]), + rabbit_ct_helpers:run_steps(Config1, + rabbit_ct_broker_helpers:setup_steps() ++ + rabbit_ct_client_helpers:setup_steps()). + +end_per_group(_Group, Config) -> + rabbit_ct_helpers:run_steps(Config, + rabbit_ct_client_helpers:teardown_steps() ++ + rabbit_ct_broker_helpers:teardown_steps()). + +init_per_testcase(Testcase, Config) -> + rabbit_ct_client_helpers:setup_steps(), + rabbit_ct_helpers:testcase_started(Config, Testcase). + +end_per_testcase(Testcase, Config) -> + rabbit_ct_client_helpers:teardown_steps(), + rabbit_ct_helpers:testcase_finished(Config, Testcase). + +%% ------------------------------------------------------------------- +%% Test cases. +%% ------------------------------------------------------------------- + +most_basic_single_node_connection_count_test(Config) -> + VHost = <<"/">>, + ?assertEqual(0, count_connections_in(Config, VHost)), + Conn = open_unmanaged_connection(Config, 0), + ?assertEqual(1, count_connections_in(Config, VHost)), + rabbit_ct_client_helpers:close_connection(Conn), + ?assertEqual(0, count_connections_in(Config, VHost)), + + passed. + +single_node_single_vhost_connection_count_test(Config) -> + VHost = <<"/">>, + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn1 = open_unmanaged_connection(Config, 0), + ?assertEqual(1, count_connections_in(Config, VHost)), + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn2 = open_unmanaged_connection(Config, 0), + ?assertEqual(1, count_connections_in(Config, VHost)), + + Conn3 = open_unmanaged_connection(Config, 0), + ?assertEqual(2, count_connections_in(Config, VHost)), + + Conn4 = open_unmanaged_connection(Config, 0), + ?assertEqual(3, count_connections_in(Config, VHost)), + + (catch exit(Conn4, please_terminate)), + ?assertEqual(2, count_connections_in(Config, VHost)), + + Conn5 = open_unmanaged_connection(Config, 0), + ?assertEqual(3, count_connections_in(Config, VHost)), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn2, Conn3, Conn5]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + passed. + +single_node_multiple_vhost_connection_count_test(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, + + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + ?assertEqual(1, count_connections_in(Config, VHost1)), + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, count_connections_in(Config, VHost1)), + + Conn2 = open_unmanaged_connection(Config, 0, VHost2), + ?assertEqual(1, count_connections_in(Config, VHost2)), + + Conn3 = open_unmanaged_connection(Config, 0, VHost1), + ?assertEqual(1, count_connections_in(Config, VHost1)), + ?assertEqual(1, count_connections_in(Config, VHost2)), + + Conn4 = open_unmanaged_connection(Config, 0, VHost1), + ?assertEqual(2, count_connections_in(Config, VHost1)), + + (catch exit(Conn4, please_terminate)), + ?assertEqual(1, count_connections_in(Config, VHost1)), + + Conn5 = open_unmanaged_connection(Config, 0, VHost2), + ?assertEqual(2, count_connections_in(Config, VHost2)), + + Conn6 = open_unmanaged_connection(Config, 0, VHost2), + ?assertEqual(3, count_connections_in(Config, VHost2)), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn2, Conn3, Conn5, Conn6]), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), + + passed. + +single_node_list_in_vhost_test(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, + + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + ?assertEqual(0, length(connections_in(Config, VHost1))), + ?assertEqual(0, length(connections_in(Config, VHost2))), + + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + [#tracked_connection{vhost = VHost1}] = connections_in(Config, VHost1), + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, length(connections_in(Config, VHost1))), + + Conn2 = open_unmanaged_connection(Config, 0, VHost2), + [#tracked_connection{vhost = VHost2}] = connections_in(Config, VHost2), + + Conn3 = open_unmanaged_connection(Config, 0, VHost1), + [#tracked_connection{vhost = VHost1}] = connections_in(Config, VHost1), + + Conn4 = open_unmanaged_connection(Config, 0, VHost1), + (catch exit(Conn4, please_terminate)), + [#tracked_connection{vhost = VHost1}] = connections_in(Config, VHost1), + + Conn5 = open_unmanaged_connection(Config, 0, VHost2), + Conn6 = open_unmanaged_connection(Config, 0, VHost2), + [<<"vhost1">>, <<"vhost2">>] = + lists:usort(lists:map(fun (#tracked_connection{vhost = V}) -> V end, + all_connections(Config))), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn2, Conn3, Conn5, Conn6]), + + ?assertEqual(0, length(all_connections(Config))), + + rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), + + passed. + +most_basic_cluster_connection_count_test(Config) -> + VHost = <<"/">>, + ?assertEqual(0, count_connections_in(Config, VHost)), + Conn1 = open_unmanaged_connection(Config, 0), + ?assertEqual(1, count_connections_in(Config, VHost)), + + Conn2 = open_unmanaged_connection(Config, 1), + ?assertEqual(2, count_connections_in(Config, VHost)), + + Conn3 = open_unmanaged_connection(Config, 1), + ?assertEqual(3, count_connections_in(Config, VHost)), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + passed. + +cluster_single_vhost_connection_count_test(Config) -> + VHost = <<"/">>, + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn1 = open_unmanaged_connection(Config, 0), + ?assertEqual(1, count_connections_in(Config, VHost)), + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn2 = open_unmanaged_connection(Config, 1), + ?assertEqual(1, count_connections_in(Config, VHost)), + + Conn3 = open_unmanaged_connection(Config, 0), + ?assertEqual(2, count_connections_in(Config, VHost)), + + Conn4 = open_unmanaged_connection(Config, 1), + ?assertEqual(3, count_connections_in(Config, VHost)), + + (catch exit(Conn4, please_terminate)), + ?assertEqual(2, count_connections_in(Config, VHost)), + + Conn5 = open_unmanaged_connection(Config, 1), + ?assertEqual(3, count_connections_in(Config, VHost)), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn2, Conn3, Conn5]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + passed. + +cluster_multiple_vhost_connection_count_test(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, + + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + ?assertEqual(1, count_connections_in(Config, VHost1)), + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, count_connections_in(Config, VHost1)), + + Conn2 = open_unmanaged_connection(Config, 1, VHost2), + ?assertEqual(1, count_connections_in(Config, VHost2)), + + Conn3 = open_unmanaged_connection(Config, 1, VHost1), + ?assertEqual(1, count_connections_in(Config, VHost1)), + ?assertEqual(1, count_connections_in(Config, VHost2)), + + Conn4 = open_unmanaged_connection(Config, 0, VHost1), + ?assertEqual(2, count_connections_in(Config, VHost1)), + + (catch exit(Conn4, please_terminate)), + ?assertEqual(1, count_connections_in(Config, VHost1)), + + Conn5 = open_unmanaged_connection(Config, 1, VHost2), + ?assertEqual(2, count_connections_in(Config, VHost2)), + + Conn6 = open_unmanaged_connection(Config, 0, VHost2), + ?assertEqual(3, count_connections_in(Config, VHost2)), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn2, Conn3, Conn5, Conn6]), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), + + passed. + +cluster_node_restart_connection_count_test(Config) -> + VHost = <<"/">>, + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn1 = open_unmanaged_connection(Config, 0), + ?assertEqual(1, count_connections_in(Config, VHost)), + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn2 = open_unmanaged_connection(Config, 1), + ?assertEqual(1, count_connections_in(Config, VHost)), + + Conn3 = open_unmanaged_connection(Config, 0), + ?assertEqual(2, count_connections_in(Config, VHost)), + + Conn4 = open_unmanaged_connection(Config, 1), + ?assertEqual(3, count_connections_in(Config, VHost)), + + Conn5 = open_unmanaged_connection(Config, 1), + ?assertEqual(4, count_connections_in(Config, VHost)), + + rabbit_ct_broker_helpers:restart_broker(Config, 1), + ?assertEqual(1, count_connections_in(Config, VHost)), + + lists:foreach(fun (C) -> + (catch rabbit_ct_client_helpers:close_connection(C)) + end, [Conn2, Conn3, Conn4, Conn5]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + passed. + +cluster_node_list_on_node_test(Config) -> + [A, B] = rabbit_ct_broker_helpers:get_node_configs(Config, nodename), + + ?assertEqual(0, length(all_connections(Config))), + ?assertEqual(0, length(connections_on_node(Config, 0))), + + Conn1 = open_unmanaged_connection(Config, 0), + [#tracked_connection{node = A}] = connections_on_node(Config, 0), + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, length(connections_on_node(Config, 0))), + + _Conn2 = open_unmanaged_connection(Config, 1), + [#tracked_connection{node = B}] = connections_on_node(Config, 1), + + Conn3 = open_unmanaged_connection(Config, 0), + ?assertEqual(1, length(connections_on_node(Config, 0))), + + Conn4 = open_unmanaged_connection(Config, 1), + ?assertEqual(2, length(connections_on_node(Config, 1))), + + (catch exit(Conn4, please_terminate)), + ?assertEqual(1, length(connections_on_node(Config, 1))), + + Conn5 = open_unmanaged_connection(Config, 0), + ?assertEqual(2, length(connections_on_node(Config, 0))), + + rabbit_ct_broker_helpers:stop_broker(Config, 1), + ?assertEqual(2, length(all_connections(Config))), + ?assertEqual(0, length(connections_on_node(Config, 0, B))), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn3, Conn5]), + + timer:sleep(100), + ?assertEqual(0, length(all_connections(Config, 0))), + + rabbit_ct_broker_helpers:start_broker(Config, 1), + + passed. + +single_node_connection_reregistration_idempotency_test(Config) -> + VHost = <<"/">>, + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn1 = open_unmanaged_connection(Config, 0), + Conn2 = open_unmanaged_connection(Config, 0), + Conn3 = open_unmanaged_connection(Config, 0), + Conn4 = open_unmanaged_connection(Config, 0), + Conn5 = open_unmanaged_connection(Config, 0), + + ?assertEqual(5, count_connections_in(Config, VHost)), + + reregister_connections_on(Config, 0), + timer:sleep(100), + + ?assertEqual(5, count_connections_in(Config, VHost)), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4, Conn5]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + passed. + +cluster_connection_reregistration_idempotency_test(Config) -> + VHost = <<"/">>, + + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn1 = open_unmanaged_connection(Config, 0), + Conn2 = open_unmanaged_connection(Config, 1), + Conn3 = open_unmanaged_connection(Config, 0), + Conn4 = open_unmanaged_connection(Config, 1), + Conn5 = open_unmanaged_connection(Config, 1), + + ?assertEqual(5, count_connections_in(Config, VHost)), + + reregister_connections_on(Config, 0), + reregister_connections_on(Config, 1), + timer:sleep(100), + + ?assertEqual(5, count_connections_in(Config, VHost)), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4, Conn5]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + passed. + +single_node_single_vhost_limit_test(Config) -> + VHost = <<"/">>, + set_vhost_connection_limit(Config, VHost, 3), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn1 = open_unmanaged_connection(Config, 0), + Conn2 = open_unmanaged_connection(Config, 0), + Conn3 = open_unmanaged_connection(Config, 0), + + %% we've crossed the limit + {error, not_allowed} = open_unmanaged_connection(Config, 0), + {error, not_allowed} = open_unmanaged_connection(Config, 0), + {error, not_allowed} = open_unmanaged_connection(Config, 0), + + set_vhost_connection_limit(Config, VHost, 5), + Conn4 = open_unmanaged_connection(Config, 0), + Conn5 = open_unmanaged_connection(Config, 0), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4, Conn5]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + set_vhost_connection_limit(Config, VHost, 0), + + passed. + +single_node_multiple_vhost_limit_test(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, + + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + set_vhost_connection_limit(Config, VHost1, 2), + set_vhost_connection_limit(Config, VHost2, 2), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + Conn2 = open_unmanaged_connection(Config, 0, VHost1), + Conn3 = open_unmanaged_connection(Config, 0, VHost2), + Conn4 = open_unmanaged_connection(Config, 0, VHost2), + + %% we've crossed the limit + {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost1), + {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost2), + + Conn5 = open_unmanaged_connection(Config, 0), + + set_vhost_connection_limit(Config, VHost1, 5), + set_vhost_connection_limit(Config, VHost2, 5), + + Conn6 = open_unmanaged_connection(Config, 0, VHost1), + Conn7 = open_unmanaged_connection(Config, 0, VHost1), + Conn8 = open_unmanaged_connection(Config, 0, VHost1), + Conn9 = open_unmanaged_connection(Config, 0, VHost2), + Conn10 = open_unmanaged_connection(Config, 0, VHost2), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4, Conn5, + Conn6, Conn7, Conn8, Conn9, Conn10]), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + set_vhost_connection_limit(Config, VHost1, 100000000), + set_vhost_connection_limit(Config, VHost2, 100000000), + + rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), + + passed. + +cluster_single_vhost_limit_test(Config) -> + VHost = <<"/">>, + set_vhost_connection_limit(Config, VHost, 2), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + Conn1 = open_unmanaged_connection(Config, 0, VHost), + Conn2 = open_unmanaged_connection(Config, 1, VHost), + + %% we've crossed the limit + {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost), + {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost), + + set_vhost_connection_limit(Config, VHost, 5), + + Conn3 = open_unmanaged_connection(Config, 0, VHost), + Conn4 = open_unmanaged_connection(Config, 0, VHost), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + set_vhost_connection_limit(Config, VHost, 0), + + passed. + +single_node_vhost_deletion_forces_connection_closure_test(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, + + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + ?assertEqual(1, count_connections_in(Config, VHost1)), + + _Conn2 = open_unmanaged_connection(Config, 0, VHost2), + ?assertEqual(1, count_connections_in(Config, VHost2)), + + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), + timer:sleep(200), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, count_connections_in(Config, VHost1)), + + rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), + + passed. + + +%% ------------------------------------------------------------------- +%% Helpers +%% ------------------------------------------------------------------- + +count_connections_in(Config, VHost) -> + count_connections_in(Config, VHost, 0). +count_connections_in(Config, VHost, NodeIndex) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + count_connections_in, [VHost]). + +connections_in(Config, VHost) -> + connections_in(Config, 0, VHost). +connections_in(Config, NodeIndex, VHost) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list, [VHost]). + +connections_on_node(Config) -> + connections_on_node(Config, 0). +connections_on_node(Config, NodeIndex) -> + Node = rabbit_ct_broker_helpers:get_node_config(Config, NodeIndex, nodename), + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list_on_node, [Node]). +connections_on_node(Config, NodeIndex, NodeForListing) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list_on_node, [NodeForListing]). + +all_connections(Config) -> + all_connections(Config, 0). +all_connections(Config, NodeIndex) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list, []). + +reregister_connections_on(Config, NodeIndex) -> + Node = rabbit_ct_broker_helpers:get_node_config( + Config, NodeIndex, nodename), + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracker, + reregister, + [Node]). + +set_up_vhost(Config, VHost) -> + rabbit_ct_broker_helpers:add_vhost(Config, VHost), + rabbit_ct_broker_helpers:set_full_permissions(Config, <<"guest">>, VHost). + +set_vhost_connection_limit(Config, VHost, Count) -> + set_vhost_connection_limit(Config, 0, VHost, Count). + +set_vhost_connection_limit(Config, NodeIndex, VHost, Count) -> + Node = rabbit_ct_broker_helpers:get_node_config( + Config, NodeIndex, nodename), + rabbit_ct_broker_helpers:control_action( + set_vhost_limits, Node, + ["{\"max-connections\": " ++ integer_to_list(Count) ++ "}"], + [{"-p", binary_to_list(VHost)}]). diff --git a/test/per_vhost_connection_limit_partitions_SUITE.erl b/test/per_vhost_connection_limit_partitions_SUITE.erl new file mode 100644 index 000000000000..6ef6a2549686 --- /dev/null +++ b/test/per_vhost_connection_limit_partitions_SUITE.erl @@ -0,0 +1,164 @@ +%% 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) 2011-2015 Pivotal Software, Inc. All rights reserved. +%% + +-module(per_vhost_connection_limit_partitions_SUITE). + +-include_lib("common_test/include/ct.hrl"). +-include_lib("amqp_client/include/amqp_client.hrl"). +-include_lib("eunit/include/eunit.hrl"). + +-compile(export_all). + +-import(rabbit_ct_client_helpers, [open_unmanaged_connection/2, + open_unmanaged_connection/3]). + + +all() -> + [ + {group, net_ticktime_1} + ]. + +groups() -> + [ + {net_ticktime_1, [], [ + cluster_full_partition_with_autoheal_test + ]} + ]. + +%% see partitions_SUITE +-define(DELAY, 12000). + +%% ------------------------------------------------------------------- +%% Testsuite setup/teardown. +%% ------------------------------------------------------------------- + +init_per_suite(Config) -> + rabbit_ct_helpers:log_environment(), + rabbit_ct_helpers:run_setup_steps(Config, [ + fun rabbit_ct_broker_helpers:enable_dist_proxy_manager/1 + ]). + +end_per_suite(Config) -> + rabbit_ct_helpers:run_teardown_steps(Config). + +init_per_group(net_ticktime_1 = GroupName, Config) -> + Config1 = rabbit_ct_helpers:set_config(Config, [{net_ticktime, 1}]), + init_per_multinode_group(GroupName, Config1, 3). + +init_per_multinode_group(_GroupName, Config, NodeCount) -> + Suffix = rabbit_ct_helpers:testcase_absname(Config, "", "-"), + Config1 = rabbit_ct_helpers:set_config(Config, [ + {rmq_nodes_count, NodeCount}, + {rmq_nodename_suffix, Suffix}, + {rmq_nodes_clustered, false} + ]), + rabbit_ct_helpers:run_steps(Config1, + rabbit_ct_broker_helpers:setup_steps() ++ [ + fun rabbit_ct_broker_helpers:enable_dist_proxy/1, + fun rabbit_ct_broker_helpers:cluster_nodes/1 + ]). + +end_per_group(_Group, Config) -> + rabbit_ct_helpers:run_steps(Config, + rabbit_ct_broker_helpers:teardown_steps()). + +init_per_testcase(Testcase, Config) -> + rabbit_ct_client_helpers:setup_steps(), + rabbit_ct_helpers:testcase_started(Config, Testcase). + +end_per_testcase(Testcase, Config) -> + rabbit_ct_client_helpers:teardown_steps(), + rabbit_ct_helpers:testcase_finished(Config, Testcase). + +%% ------------------------------------------------------------------- +%% Test cases. +%% ------------------------------------------------------------------- + +cluster_full_partition_with_autoheal_test(Config) -> + VHost = <<"/">>, + rabbit_ct_broker_helpers:set_partition_handling_mode_globally(Config, autoheal), + + ?assertEqual(0, count_connections_in(Config, VHost)), + [A, B, C] = rabbit_ct_broker_helpers:get_node_configs(Config, nodename), + + %% 6 connections, 2 per node + Conn1 = open_unmanaged_connection(Config, A), + Conn2 = open_unmanaged_connection(Config, A), + Conn3 = open_unmanaged_connection(Config, B), + Conn4 = open_unmanaged_connection(Config, B), + Conn5 = open_unmanaged_connection(Config, C), + Conn6 = open_unmanaged_connection(Config, C), + ?assertEqual(6, count_connections_in(Config, VHost)), + + %% B drops off the network, non-reachable by either A or C + rabbit_ct_broker_helpers:block_traffic_between(A, B), + rabbit_ct_broker_helpers:block_traffic_between(B, C), + timer:sleep(?DELAY), + + %% A and C are still connected, so 4 connections are tracked + ?assertEqual(4, count_connections_in(Config, VHost)), + + rabbit_ct_broker_helpers:allow_traffic_between(A, B), + rabbit_ct_broker_helpers:allow_traffic_between(B, C), + timer:sleep(?DELAY), + + %% during autoheal B's connections were dropped + ?assertEqual(4, count_connections_in(Config, VHost)), + + lists:foreach(fun (Conn) -> + (catch rabbit_ct_client_helpers:close_connection(Conn)) + end, [Conn1, Conn2, Conn3, Conn4, + Conn5, Conn6]), + + passed. + + +%% ------------------------------------------------------------------- +%% Helpers +%% ------------------------------------------------------------------- + +count_connections_in(Config, VHost) -> + count_connections_in(Config, VHost, 0). +count_connections_in(Config, VHost, NodeIndex) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + count_connections_in, [VHost]). + +connections_in(Config, VHost) -> + connections_in(Config, 0, VHost). +connections_in(Config, NodeIndex, VHost) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list, [VHost]). + +connections_on_node(Config) -> + connections_on_node(Config, 0). +connections_on_node(Config, NodeIndex) -> + Node = rabbit_ct_broker_helpers:get_node_config(Config, NodeIndex, nodename), + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list_on_node, [Node]). +connections_on_node(Config, NodeIndex, NodeForListing) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list_on_node, [NodeForListing]). + +all_connections(Config) -> + all_connections(Config, 0). +all_connections(Config, NodeIndex) -> + rabbit_ct_broker_helpers:rpc(Config, NodeIndex, + rabbit_connection_tracking, + list, []). From d4aaae97b672e2b630eac48ce1e5cf40d1b0afba Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Tue, 26 Jul 2016 15:00:04 +0300 Subject: [PATCH 02/29] Use ?PER_VHOST_COUNTER_TABLE here --- src/rabbit_connection_tracking.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index c65023a2a850..908a762caff9 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -52,7 +52,7 @@ register_connection(#tracked_connection{vhost = VHost, id = ConnId} = Conn) -> [] -> mnesia:write(?TABLE, Conn, write), mnesia:dirty_update_counter( - rabbit_tracked_connection_per_vhost, VHost, 1); + ?PER_VHOST_COUNTER_TABLE, VHost, 1); [_Row] -> ok end, From 995c430fbb1981d81f8c18740153c611c49fda4d Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Tue, 26 Jul 2016 16:24:22 +0300 Subject: [PATCH 03/29] Correct a type spec --- src/rabbit_connection_tracking.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 908a762caff9..f059c2698b5c 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -113,7 +113,7 @@ on_node_up(Node) -> rabbit_connection_tracker:reregister(Node), ok. --spec is_over_connection_limit(rabbit_types:vhost()) -> boolean(). +-spec is_over_connection_limit(rabbit_types:vhost()) -> {true, non_neg_integer()} | false. is_over_connection_limit(VirtualHost) -> ConnectionCount = count_connections_in(VirtualHost), From 747b41a9d3794706390b21003eb4bf6929422213 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Tue, 2 Aug 2016 06:06:09 -0700 Subject: [PATCH 04/29] Switch to a single table per node for connection tracking This way we have a single writer, multiple readers and no lost counter update due to natural race conditions. Data for tables from available nodes are then aggregated. Two open questions: * Whether to use a parallel version of lists:map/2 * When to clean up the tables --- src/rabbit.erl | 6 +- src/rabbit_connection_tracker.erl | 99 ----------- src/rabbit_connection_tracking.erl | 186 +++++++++++++++------ src/rabbit_connection_tracking_handler.erl | 38 +++-- src/rabbit_mnesia.erl | 2 + src/rabbit_node_monitor.erl | 4 +- src/rabbit_upgrade_functions.erl | 15 -- test/per_vhost_connection_limit_SUITE.erl | 115 ++++++------- 8 files changed, 215 insertions(+), 250 deletions(-) delete mode 100644 src/rabbit_connection_tracker.erl diff --git a/src/rabbit.erl b/src/rabbit.erl index 59ede3c802de..5db2c40b666a 100644 --- a/src/rabbit.erl +++ b/src/rabbit.erl @@ -178,9 +178,9 @@ {mfa, {rabbit_direct, boot, []}}, {requires, log_relay}]}). --rabbit_boot_step({connection_tracker, - [{description, "helps track node-local connections"}, - {mfa, {rabbit_connection_tracker, boot, []}}, +-rabbit_boot_step({connection_tracking, + [{description, "sets up internal storage for node-local connections"}, + {mfa, {rabbit_connection_tracking, boot, []}}, {requires, log_relay}]}). -rabbit_boot_step({networking, diff --git a/src/rabbit_connection_tracker.erl b/src/rabbit_connection_tracker.erl deleted file mode 100644 index 0177627d8316..000000000000 --- a/src/rabbit_connection_tracker.erl +++ /dev/null @@ -1,99 +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-2016 Pivotal Software, Inc. All rights reserved. -%% - --module(rabbit_connection_tracker). - -%% Abstracts away how tracked connection records are stored -%% and queried. -%% -%% See also: -%% -%% * rabbit_connection_tracking_handler -%% * rabbit_reader -%% * rabbit_event - --behaviour(gen_server2). - -%% API --export([boot/0, start_link/0, reregister/1]). - -%% gen_fsm callbacks --export([init/1, - handle_call/3, - handle_cast/2, - handle_info/2, - terminate/2, - code_change/3]). - --define(SERVER, ?MODULE). - - -%%%=================================================================== -%%% API -%%%=================================================================== - -boot() -> - {ok, _} = start_link(), - ok. - -start_link() -> - gen_server2:start_link({local, ?SERVER}, ?MODULE, [], []). - -reregister(Node) -> - rabbit_log:info("Telling node ~p to re-register tracked connections", [Node]), - gen_server2:cast({?SERVER, Node}, reregister). - -%%%=================================================================== -%%% gen_server callbacks -%%%=================================================================== - -init([]) -> - {ok, {}}. - -handle_call(_Req, _From, State) -> - {noreply, State}. - -handle_cast(reregister, State) -> - Cs = rabbit_networking:connections_local(), - rabbit_log:info("Connection tracker: asked to re-register ~p client connections", [length(Cs)]), - case Cs of - [] -> ok; - Cs -> - [reregister_connection(C) || C <- Cs], - ok - end, - rabbit_log:info("Done re-registering client connections"), - {noreply, State}. - -handle_info(_Req, State) -> - {noreply, State}. - -terminate(_Reason, _State) -> - ok. - -code_change(_OldVsn, State, _Extra) -> - {ok, State}. - -%%%=================================================================== -%%% Internal functions -%%%=================================================================== - -reregister_connection(Conn) -> - try - Conn ! reregister - catch _:Error -> - rabbit_log:error("Failed to re-register connection ~p after a network split: ~p", [Conn, Error]) - end. diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index f059c2698b5c..17e2d79034ed 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -25,34 +25,117 @@ %% * rabbit_reader %% * rabbit_event --export([register_connection/1, unregister_connection/1, +-export([boot/0, + ensure_tracked_connections_table_for_node/1, + ensure_per_vhost_tracked_connections_table_for_node/1, + ensure_tracked_connections_table_for_this_node/0, + ensure_per_vhost_tracked_connections_table_for_this_node/0, + tracked_connection_table_name_for/1, tracked_connection_per_vhost_table_name_for/1, + clear_tracked_connections_table_for_this_node/0, + register_connection/1, unregister_connection/1, list/0, list/1, list_on_node/1, tracked_connection_from_connection_created/1, tracked_connection_from_connection_state/1, - is_over_connection_limit/1, count_connections_in/1, - on_node_down/1, on_node_up/1]). + is_over_connection_limit/1, count_connections_in/1]). -include_lib("rabbit.hrl"). --define(TABLE, rabbit_tracked_connection). --define(PER_VHOST_COUNTER_TABLE, rabbit_tracked_connection_per_vhost). --define(SERVER, ?MODULE). - %% %% API %% +-spec boot() -> ok. + +%% Sets up and resets connection tracking tables for this +%% node. +boot() -> + ensure_tracked_connections_table_for_this_node(), + rabbit_log:info("Created a table for connection tracking on this node: ~p", + [tracked_connection_table_name_for(node())]), + ensure_per_vhost_tracked_connections_table_for_this_node(), + rabbit_log:info("Created a table for per-vhost connection counting on this node: ~p", + [tracked_connection_per_vhost_table_name_for(node())]), + clear_tracked_connections_table_for_this_node(), + ok. + + +-spec ensure_tracked_connections_table_for_this_node() -> ok. + +ensure_tracked_connections_table_for_this_node() -> + ensure_tracked_connections_table_for_node(node()). + + +-spec ensure_per_vhost_tracked_connections_table_for_this_node() -> ok. + +ensure_per_vhost_tracked_connections_table_for_this_node() -> + ensure_per_vhost_tracked_connections_table_for_node(node()). + + +-spec ensure_tracked_connections_table_for_node(node()) -> ok. + +ensure_tracked_connections_table_for_node(Node) -> + TableName = tracked_connection_table_name_for(Node), + case mnesia:create_table(TableName, [{record_name, tracked_connection}, + {attributes, [id, node, vhost, name, + pid, protocol, + peer_host, peer_port, + username, connected_at]}]) of + {atomic, ok} -> ok; + {aborted, _} -> ok + %% TODO: propagate errors + end. + + +-spec ensure_per_vhost_tracked_connections_table_for_node(node()) -> ok. + +ensure_per_vhost_tracked_connections_table_for_node(Node) -> + TableName = tracked_connection_per_vhost_table_name_for(Node), + case mnesia:create_table(TableName, [{record_name, tracked_connection_per_vhost}, + {attributes, [vhost, connection_count]}]) of + {atomic, ok} -> ok; + {aborted, _} -> ok + %% TODO: propagate errors + end. + + +-spec clear_tracked_connections_table_for_this_node() -> ok. + +clear_tracked_connections_table_for_this_node() -> + case mnesia:clear_table(tracked_connection_table_name_for(node())) of + {atomic, ok} -> ok; + {aborted, _} -> ok + end, + case mnesia:clear_table(tracked_connection_per_vhost_table_name_for(node())) of + {atomic, ok} -> ok; + {aborted, _} -> ok + end. + + +-spec tracked_connection_table_name_for(node()) -> atom(). + +tracked_connection_table_name_for(Node) -> + list_to_atom(rabbit_misc:format("tracked_connection_on_node_~s", [Node])). + +-spec tracked_connection_per_vhost_table_name_for(node()) -> atom(). + +tracked_connection_per_vhost_table_name_for(Node) -> + list_to_atom(rabbit_misc:format("tracked_connection_per_vhost_on_node_~s", [Node])). + + -spec register_connection(rabbit_types:tracked_connection()) -> ok. -register_connection(#tracked_connection{vhost = VHost, id = ConnId} = Conn) -> +register_connection(#tracked_connection{vhost = VHost, id = ConnId, node = Node} = Conn) when Node =:= node() -> + TableName = tracked_connection_table_name_for(Node), + PerVhostTableName = tracked_connection_per_vhost_table_name_for(Node), rabbit_misc:execute_mnesia_transaction( fun() -> %% upsert - case mnesia:dirty_read(?TABLE, ConnId) of + case mnesia:dirty_read(TableName, ConnId) of [] -> - mnesia:write(?TABLE, Conn, write), + %% TODO: counter table + mnesia:write(TableName, Conn, write), mnesia:dirty_update_counter( - ?PER_VHOST_COUNTER_TABLE, VHost, 1); + PerVhostTableName, VHost, 1); [_Row] -> ok end, @@ -61,16 +144,17 @@ register_connection(#tracked_connection{vhost = VHost, id = ConnId} = Conn) -> -spec unregister_connection(rabbit_types:connection_name()) -> ok. -unregister_connection(ConnId = {_Node, _Name}) -> +unregister_connection(ConnId = {Node, _Name}) when Node =:= node() -> + TableName = tracked_connection_table_name_for(Node), + PerVhostTableName = tracked_connection_per_vhost_table_name_for(Node), rabbit_misc:execute_mnesia_transaction( fun() -> - case mnesia:dirty_read(?TABLE, ConnId) of - [] -> ok; + case mnesia:dirty_read(TableName, ConnId) of + [] -> ok; [Row] -> mnesia:dirty_update_counter( - ?PER_VHOST_COUNTER_TABLE, - Row#tracked_connection.vhost, -1), - mnesia:delete({?TABLE, ConnId}) + PerVhostTableName, Row#tracked_connection.vhost, -1), + mnesia:delete({TableName, ConnId}) end end). @@ -78,40 +162,33 @@ unregister_connection(ConnId = {_Node, _Name}) -> -spec list() -> [rabbit_types:tracked_connection()]. list() -> - mnesia:dirty_match_object(?TABLE, #tracked_connection{_ = '_'}). + Chunks = lists:map( + fun (Node) -> + Tab = tracked_connection_table_name_for(Node), + mnesia:dirty_match_object(Tab, #tracked_connection{_ = '_'}) + end, rabbit_mnesia:cluster_nodes(running)), + lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). -spec list(rabbit_types:vhost()) -> [rabbit_types:tracked_connection()]. list(VHost) -> - mnesia:dirty_match_object(?TABLE, #tracked_connection{vhost = VHost, _ = '_'}). + Chunks = lists:map( + fun (Node) -> + Tab = tracked_connection_table_name_for(Node), + mnesia:dirty_match_object(Tab, #tracked_connection{vhost = VHost, _ = '_'}) + end, rabbit_mnesia:cluster_nodes(running)), + lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). -spec list_on_node(node()) -> [rabbit_types:tracked_connection()]. list_on_node(Node) -> - mnesia:dirty_match_object(?TABLE, #tracked_connection{node = Node, _ = '_'}). - - --spec on_node_down(node()) -> ok. - -on_node_down(Node) -> - case lists:member(Node, nodes()) of - false -> - Cs = list_on_node(Node), - rabbit_log:info( - "Node ~p is down, unregistering ~p client connections~n", - [Node, length(Cs)]), - [unregister_connection(Id) || #tracked_connection{id = Id} <- Cs], - ok; - true -> rabbit_log:info( - "Keeping ~s connections: the node is already back~n", [Node]) - end. - --spec on_node_up(node()) -> ok. -on_node_up(Node) -> - rabbit_connection_tracker:reregister(Node), - ok. + try mnesia:dirty_match_object( + tracked_connection_table_name_for(Node), + #tracked_connection{_ = '_'}) + catch exit:{aborted, {no_exists, _}} -> [] + end. -spec is_over_connection_limit(rabbit_types:vhost()) -> {true, non_neg_integer()} | false. @@ -133,19 +210,24 @@ is_over_connection_limit(VirtualHost) -> count_connections_in(VirtualHost) -> try - case mnesia:transaction( + Ns = lists:map( + fun (Node) -> + Tab = tracked_connection_per_vhost_table_name_for(Node), + try + case mnesia:transaction( fun() -> - case mnesia:dirty_read( - {?PER_VHOST_COUNTER_TABLE, - VirtualHost}) of - [] -> 0; - [Val] -> - Val#tracked_connection_per_vhost.connection_count - end + case mnesia:dirty_read({Tab, VirtualHost}) of + [] -> 0; + [Val] -> Val#tracked_connection_per_vhost.connection_count + end end) of - {atomic, Val} -> Val; - {aborted, _Reason} -> 0 - end + {atomic, Val} -> Val; + {aborted, _Reason} -> 0 + end + catch _ -> 0 + end + end, rabbit_mnesia:cluster_nodes(running)), + lists:foldl(fun(X, Acc) -> Acc + X end, 0, Ns) catch _:Err -> rabbit_log:error( diff --git a/src/rabbit_connection_tracking_handler.erl b/src/rabbit_connection_tracking_handler.erl index deaf9bc7f666..91b38d2c3929 100644 --- a/src/rabbit_connection_tracking_handler.erl +++ b/src/rabbit_connection_tracking_handler.erl @@ -53,23 +53,31 @@ init([]) -> {ok, []}. handle_event(#event{type = connection_created, props = Details}, State) -> - rabbit_connection_tracking:register_connection( - rabbit_connection_tracking:tracked_connection_from_connection_created(Details) - ), - {ok, State}; -%% see rabbit_reader -handle_event(#event{type = connection_reregistered, props = [{state, ConnState}]}, State) -> - rabbit_connection_tracking:register_connection( - rabbit_connection_tracking:tracked_connection_from_connection_state(ConnState) - ), + ThisNode = node(), + case proplists:get_value(node, Details) of + ThisNode -> + rabbit_connection_tracking:register_connection( + rabbit_connection_tracking:tracked_connection_from_connection_created(Details) + ); + _OtherNode -> + %% ignore + ok + end, {ok, State}; handle_event(#event{type = connection_closed, props = Details}, State) -> - %% [{name,<<"127.0.0.1:64078 -> 127.0.0.1:5672">>}, - %% {pid,<0.1774.0>}, - %% {node, rabbit@hostname}] - rabbit_connection_tracking:unregister_connection( - {proplists:get_value(node, Details), - proplists:get_value(name, Details)}), + ThisNode = node(), + case proplists:get_value(node, Details) of + ThisNode -> + %% [{name,<<"127.0.0.1:64078 -> 127.0.0.1:5672">>}, + %% {pid,<0.1774.0>}, + %% {node, rabbit@hostname}] + rabbit_connection_tracking:unregister_connection( + {proplists:get_value(node, Details), + proplists:get_value(name, Details)}); + _OtherNode -> + %% ignore + ok + end, {ok, State}; handle_event(#event{type = vhost_deleted, props = Details}, State) -> VHost = proplists:get_value(name, Details), diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index 43d268c37472..88ea3a80f537 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -148,6 +148,7 @@ auto_cluster(TryNodes, NodeType) -> rabbit_log:info("Node '~p' selected for auto-clustering~n", [Node]), {ok, {_, DiscNodes, _}} = discover_cluster0(Node), init_db_and_upgrade(DiscNodes, NodeType, true), + rabbit_connection_tracking:boot(), rabbit_node_monitor:notify_joined_cluster(); none -> rabbit_log:warning( @@ -194,6 +195,7 @@ join_cluster(DiscoveryNode, NodeType) -> [ClusterNodes, NodeType]), ok = init_db_with_mnesia(ClusterNodes, NodeType, true, true), + rabbit_connection_tracking:boot(), rabbit_node_monitor:notify_joined_cluster(), ok; {error, Reason} -> diff --git a/src/rabbit_node_monitor.erl b/src/rabbit_node_monitor.erl index 9ed790c75dcb..0322aacfd151 100644 --- a/src/rabbit_node_monitor.erl +++ b/src/rabbit_node_monitor.erl @@ -732,7 +732,6 @@ handle_dead_rabbit(Node, State = #state{partitions = Partitions, ok = rabbit_amqqueue:on_node_down(Node), ok = rabbit_alarm:on_node_down(Node), ok = rabbit_mnesia:on_node_down(Node), - ok = rabbit_connection_tracking:on_node_down(Node), %% If we have been partitioned, and we are now in the only remaining %% partition, we no longer care about partitions - forget them. Note %% that we do not attempt to deal with individual (other) partitions @@ -761,8 +760,7 @@ ensure_keepalive_timer(State) -> handle_live_rabbit(Node) -> ok = rabbit_amqqueue:on_node_up(Node), ok = rabbit_alarm:on_node_up(Node), - ok = rabbit_mnesia:on_node_up(Node), - ok = rabbit_connection_tracking:on_node_up(Node). + ok = rabbit_mnesia:on_node_up(Node). maybe_autoheal(State = #state{partitions = []}) -> State; diff --git a/src/rabbit_upgrade_functions.erl b/src/rabbit_upgrade_functions.erl index 47053fafe731..7efa36d63748 100644 --- a/src/rabbit_upgrade_functions.erl +++ b/src/rabbit_upgrade_functions.erl @@ -56,8 +56,6 @@ -rabbit_upgrade({slave_pids_pending_shutdown, mnesia, [policy_version]}). -rabbit_upgrade({user_password_hashing, mnesia, [hash_passwords]}). -rabbit_upgrade({vhost_limits, mnesia, []}). --rabbit_upgrade({tracked_connection, mnesia, [vhost_limits]}). --rabbit_upgrade({tracked_connection_per_vhost, mnesia, [tracked_connection]}). %% ------------------------------------------------------------------- @@ -92,23 +90,10 @@ -spec recoverable_slaves() -> 'ok'. -spec user_password_hashing() -> 'ok'. -spec vhost_limits() -> 'ok'. --spec tracked_connection() -> 'ok'. --spec tracked_connection_per_vhost() -> 'ok'. %%-------------------------------------------------------------------- -tracked_connection() -> - create(rabbit_tracked_connection, [{record_name, tracked_connection}, - {attributes, [id, node, vhost, name, - pid, protocol, - peer_host, peer_port, - username, connected_at]}]). - -tracked_connection_per_vhost() -> - create(tracked_connection_per_vhost, [{record_name, tracked_connection_per_vhost}, - {attributes, [vhost, connection_count]}]). - %% replaces vhost.dummy (used to avoid having a single-field record %% which Mnesia doesn't like) with vhost.limits (which is actually %% used) diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index 0b1f5adf8495..6b9fb3ec2969 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -39,7 +39,6 @@ groups() -> single_node_single_vhost_connection_count_test, single_node_multiple_vhost_connection_count_test, single_node_list_in_vhost_test, - single_node_connection_reregistration_idempotency_test, single_node_single_vhost_limit_test, single_node_multiple_vhost_limit_test, single_node_vhost_deletion_forces_connection_closure_test @@ -50,8 +49,8 @@ groups() -> cluster_multiple_vhost_connection_count_test, cluster_node_restart_connection_count_test, cluster_node_list_on_node_test, - cluster_connection_reregistration_idempotency_test, - cluster_single_vhost_limit_test + cluster_single_vhost_limit_test, + cluster_single_vhost_limit2_test ]} ]. @@ -102,13 +101,22 @@ end_per_group(_Group, Config) -> rabbit_ct_broker_helpers:teardown_steps()). init_per_testcase(Testcase, Config) -> + clear_all_connection_tracking_tables(Config), rabbit_ct_client_helpers:setup_steps(), rabbit_ct_helpers:testcase_started(Config, Testcase). end_per_testcase(Testcase, Config) -> + clear_all_connection_tracking_tables(Config), rabbit_ct_client_helpers:teardown_steps(), rabbit_ct_helpers:testcase_finished(Config, Testcase). +clear_all_connection_tracking_tables(Config) -> + [rabbit_ct_broker_helpers:rpc(Config, + N, + rabbit_connection_tracking, + clear_tracked_connections_table_for_this_node, + []) || N <- rabbit_ct_broker_helpers:get_node_configs(Config, nodename)]. + %% ------------------------------------------------------------------- %% Test cases. %% ------------------------------------------------------------------- @@ -400,6 +408,8 @@ cluster_node_list_on_node_test(Config) -> ?assertEqual(2, length(connections_on_node(Config, 0))), rabbit_ct_broker_helpers:stop_broker(Config, 1), + await_running_node_refresh(Config, 0), + ?assertEqual(2, length(all_connections(Config))), ?assertEqual(0, length(connections_on_node(Config, 0, B))), @@ -414,58 +424,6 @@ cluster_node_list_on_node_test(Config) -> passed. -single_node_connection_reregistration_idempotency_test(Config) -> - VHost = <<"/">>, - ?assertEqual(0, count_connections_in(Config, VHost)), - - Conn1 = open_unmanaged_connection(Config, 0), - Conn2 = open_unmanaged_connection(Config, 0), - Conn3 = open_unmanaged_connection(Config, 0), - Conn4 = open_unmanaged_connection(Config, 0), - Conn5 = open_unmanaged_connection(Config, 0), - - ?assertEqual(5, count_connections_in(Config, VHost)), - - reregister_connections_on(Config, 0), - timer:sleep(100), - - ?assertEqual(5, count_connections_in(Config, VHost)), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4, Conn5]), - - ?assertEqual(0, count_connections_in(Config, VHost)), - - passed. - -cluster_connection_reregistration_idempotency_test(Config) -> - VHost = <<"/">>, - - ?assertEqual(0, count_connections_in(Config, VHost)), - - Conn1 = open_unmanaged_connection(Config, 0), - Conn2 = open_unmanaged_connection(Config, 1), - Conn3 = open_unmanaged_connection(Config, 0), - Conn4 = open_unmanaged_connection(Config, 1), - Conn5 = open_unmanaged_connection(Config, 1), - - ?assertEqual(5, count_connections_in(Config, VHost)), - - reregister_connections_on(Config, 0), - reregister_connections_on(Config, 1), - timer:sleep(100), - - ?assertEqual(5, count_connections_in(Config, VHost)), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4, Conn5]), - - ?assertEqual(0, count_connections_in(Config, VHost)), - - passed. - single_node_single_vhost_limit_test(Config) -> VHost = <<"/">>, set_vhost_connection_limit(Config, VHost, 3), @@ -549,8 +507,11 @@ cluster_single_vhost_limit_test(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)), + %% here connections are opened to different nodes Conn1 = open_unmanaged_connection(Config, 0, VHost), Conn2 = open_unmanaged_connection(Config, 1, VHost), + %% give tracked connection rows some time to propagate in both directions + timer:sleep(200), %% we've crossed the limit {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost), @@ -571,6 +532,39 @@ cluster_single_vhost_limit_test(Config) -> passed. +cluster_single_vhost_limit2_test(Config) -> + VHost = <<"/">>, + set_vhost_connection_limit(Config, VHost, 2), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + %% here a limit is reached on one node first + Conn1 = open_unmanaged_connection(Config, 0, VHost), + Conn2 = open_unmanaged_connection(Config, 0, VHost), + + %% we've crossed the limit + {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost), + + timer:sleep(200), + {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost), + + set_vhost_connection_limit(Config, VHost, 5), + + Conn3 = open_unmanaged_connection(Config, 1, VHost), + Conn4 = open_unmanaged_connection(Config, 1, VHost), + Conn5 = open_unmanaged_connection(Config, 1, VHost), + {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4, Conn5]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + set_vhost_connection_limit(Config, VHost, 0), + + passed. + single_node_vhost_deletion_forces_connection_closure_test(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -636,14 +630,6 @@ all_connections(Config, NodeIndex) -> rabbit_connection_tracking, list, []). -reregister_connections_on(Config, NodeIndex) -> - Node = rabbit_ct_broker_helpers:get_node_config( - Config, NodeIndex, nodename), - rabbit_ct_broker_helpers:rpc(Config, NodeIndex, - rabbit_connection_tracker, - reregister, - [Node]). - set_up_vhost(Config, VHost) -> rabbit_ct_broker_helpers:add_vhost(Config, VHost), rabbit_ct_broker_helpers:set_full_permissions(Config, <<"guest">>, VHost). @@ -658,3 +644,6 @@ set_vhost_connection_limit(Config, NodeIndex, VHost, Count) -> set_vhost_limits, Node, ["{\"max-connections\": " ++ integer_to_list(Count) ++ "}"], [{"-p", binary_to_list(VHost)}]). + +await_running_node_refresh(Config, NodeIndex) -> + timer:sleep(250). From 295c7b21f6328501fb1cee7a35b879d743dca9e9 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 11:29:38 -0700 Subject: [PATCH 05/29] Remove a duplicate comment --- src/rabbit_connection_tracking_handler.erl | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/rabbit_connection_tracking_handler.erl b/src/rabbit_connection_tracking_handler.erl index 91b38d2c3929..66ef1f2524d8 100644 --- a/src/rabbit_connection_tracking_handler.erl +++ b/src/rabbit_connection_tracking_handler.erl @@ -22,12 +22,6 @@ %% %% Events from other nodes are ignored. -%% This module keeps track of connection creation and termination events -%% on its local node. The primary goal here is to decouple connection -%% tracking from rabbit_reader in rabbit_common. -%% -%% Events from other nodes are ignored. - -behaviour(gen_event). -export([init/1, handle_call/2, handle_event/2, handle_info/2, From 87c2d6f2c0da37d8edc3d16790733305a1594075 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 11:48:05 -0700 Subject: [PATCH 06/29] Inline --- src/rabbit_control_main.erl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl index 0e377b7c3c28..ae98e5144303 100644 --- a/src/rabbit_control_main.erl +++ b/src/rabbit_control_main.erl @@ -548,9 +548,8 @@ action(clear_policy, Node, [Key], Opts, Inform) -> rpc_call(Node, rabbit_policy, delete, [VHostArg, list_to_binary(Key)]); action(set_vhost_limits, Node, [Defn], Opts, Inform) -> - Msg = "Setting vhost limits for vhost ~p", VHostArg = list_to_binary(proplists:get_value(?VHOST_OPT, Opts)), - Inform(Msg, [VHostArg]), + Inform("Setting vhost limits for vhost ~p", [VHostArg]), rpc_call(Node, rabbit_vhost_limit, parse_set, [VHostArg, Defn]), ok; From 95b6e8f48f7a581cf21e4807560d327714e1abd2 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 11:48:58 -0700 Subject: [PATCH 07/29] Remove a debug message --- src/rabbit_upgrade_functions.erl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rabbit_upgrade_functions.erl b/src/rabbit_upgrade_functions.erl index 7efa36d63748..305a1ece93fb 100644 --- a/src/rabbit_upgrade_functions.erl +++ b/src/rabbit_upgrade_functions.erl @@ -98,7 +98,6 @@ %% which Mnesia doesn't like) with vhost.limits (which is actually %% used) vhost_limits() -> - io:format("vhost_limits vhost_limits vhost_limits~n"), transform( rabbit_vhost, fun ({vhost, VHost, _Dummy}) -> From da08e953d1de90d0e06481a267585f0316861ea7 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 12:00:48 -0700 Subject: [PATCH 08/29] Drop the _test prefixes, remove some dead code --- test/per_vhost_connection_limit_SUITE.erl | 63 +++++++++---------- ...host_connection_limit_partitions_SUITE.erl | 4 +- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index 6b9fb3ec2969..62e42496beb1 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -35,22 +35,22 @@ all() -> groups() -> [ {cluster_size_1, [], [ - most_basic_single_node_connection_count_test, - single_node_single_vhost_connection_count_test, - single_node_multiple_vhost_connection_count_test, - single_node_list_in_vhost_test, - single_node_single_vhost_limit_test, - single_node_multiple_vhost_limit_test, - single_node_vhost_deletion_forces_connection_closure_test + most_basic_single_node_connection_count, + single_node_single_vhost_connection_count, + single_node_multiple_vhost_connection_count, + single_node_list_in_vhost, + single_node_single_vhost_limit, + single_node_multiple_vhost_limit, + single_node_vhost_deletion_forces_connection_closure ]}, {cluster_size_2, [], [ - most_basic_cluster_connection_count_test, - cluster_single_vhost_connection_count_test, - cluster_multiple_vhost_connection_count_test, - cluster_node_restart_connection_count_test, - cluster_node_list_on_node_test, - cluster_single_vhost_limit_test, - cluster_single_vhost_limit2_test + most_basic_cluster_connection_count, + cluster_single_vhost_connection_count, + cluster_multiple_vhost_connection_count, + cluster_node_restart_connection_count, + cluster_node_list_on_node, + cluster_single_vhost_limit, + cluster_single_vhost_limit2 ]} ]. @@ -80,10 +80,7 @@ init_per_group(cluster_size_1, Config) -> rabbit_ct_broker_helpers:setup_steps() ++ rabbit_ct_client_helpers:setup_steps()); init_per_group(cluster_size_2, Config) -> - init_per_multinode_group(cluster_size_2, Config, 2); -init_per_group(partition_handling, Config) -> - Config1 = rabbit_ct_helpers:set_config(Config, [{net_ticktime, 1}]), - init_per_multinode_group(partition_handling, Config1, 3). + init_per_multinode_group(cluster_size_2, Config, 2). init_per_multinode_group(_GroupName, Config, NodeCount) -> Suffix = rabbit_ct_helpers:testcase_absname(Config, "", "-"), @@ -121,7 +118,7 @@ clear_all_connection_tracking_tables(Config) -> %% Test cases. %% ------------------------------------------------------------------- -most_basic_single_node_connection_count_test(Config) -> +most_basic_single_node_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), Conn = open_unmanaged_connection(Config, 0), @@ -131,7 +128,7 @@ most_basic_single_node_connection_count_test(Config) -> passed. -single_node_single_vhost_connection_count_test(Config) -> +single_node_single_vhost_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), @@ -163,7 +160,7 @@ single_node_single_vhost_connection_count_test(Config) -> passed. -single_node_multiple_vhost_connection_count_test(Config) -> +single_node_multiple_vhost_connection_count(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -209,7 +206,7 @@ single_node_multiple_vhost_connection_count_test(Config) -> passed. -single_node_list_in_vhost_test(Config) -> +single_node_list_in_vhost(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -251,7 +248,7 @@ single_node_list_in_vhost_test(Config) -> passed. -most_basic_cluster_connection_count_test(Config) -> +most_basic_cluster_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), Conn1 = open_unmanaged_connection(Config, 0), @@ -271,7 +268,7 @@ most_basic_cluster_connection_count_test(Config) -> passed. -cluster_single_vhost_connection_count_test(Config) -> +cluster_single_vhost_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), @@ -303,7 +300,7 @@ cluster_single_vhost_connection_count_test(Config) -> passed. -cluster_multiple_vhost_connection_count_test(Config) -> +cluster_multiple_vhost_connection_count(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -349,7 +346,7 @@ cluster_multiple_vhost_connection_count_test(Config) -> passed. -cluster_node_restart_connection_count_test(Config) -> +cluster_node_restart_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), @@ -381,7 +378,7 @@ cluster_node_restart_connection_count_test(Config) -> passed. -cluster_node_list_on_node_test(Config) -> +cluster_node_list_on_node(Config) -> [A, B] = rabbit_ct_broker_helpers:get_node_configs(Config, nodename), ?assertEqual(0, length(all_connections(Config))), @@ -424,7 +421,7 @@ cluster_node_list_on_node_test(Config) -> passed. -single_node_single_vhost_limit_test(Config) -> +single_node_single_vhost_limit(Config) -> VHost = <<"/">>, set_vhost_connection_limit(Config, VHost, 3), @@ -452,7 +449,7 @@ single_node_single_vhost_limit_test(Config) -> passed. -single_node_multiple_vhost_limit_test(Config) -> +single_node_multiple_vhost_limit(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -501,7 +498,7 @@ single_node_multiple_vhost_limit_test(Config) -> passed. -cluster_single_vhost_limit_test(Config) -> +cluster_single_vhost_limit(Config) -> VHost = <<"/">>, set_vhost_connection_limit(Config, VHost, 2), @@ -532,7 +529,7 @@ cluster_single_vhost_limit_test(Config) -> passed. -cluster_single_vhost_limit2_test(Config) -> +cluster_single_vhost_limit2(Config) -> VHost = <<"/">>, set_vhost_connection_limit(Config, VHost, 2), @@ -565,7 +562,7 @@ cluster_single_vhost_limit2_test(Config) -> passed. -single_node_vhost_deletion_forces_connection_closure_test(Config) -> +single_node_vhost_deletion_forces_connection_closure(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -645,5 +642,5 @@ set_vhost_connection_limit(Config, NodeIndex, VHost, Count) -> ["{\"max-connections\": " ++ integer_to_list(Count) ++ "}"], [{"-p", binary_to_list(VHost)}]). -await_running_node_refresh(Config, NodeIndex) -> +await_running_node_refresh(_Config, _NodeIndex) -> timer:sleep(250). diff --git a/test/per_vhost_connection_limit_partitions_SUITE.erl b/test/per_vhost_connection_limit_partitions_SUITE.erl index 6ef6a2549686..c7fae3a77cf2 100644 --- a/test/per_vhost_connection_limit_partitions_SUITE.erl +++ b/test/per_vhost_connection_limit_partitions_SUITE.erl @@ -34,7 +34,7 @@ all() -> groups() -> [ {net_ticktime_1, [], [ - cluster_full_partition_with_autoheal_test + cluster_full_partition_with_autoheal ]} ]. @@ -87,7 +87,7 @@ end_per_testcase(Testcase, Config) -> %% Test cases. %% ------------------------------------------------------------------- -cluster_full_partition_with_autoheal_test(Config) -> +cluster_full_partition_with_autoheal(Config) -> VHost = <<"/">>, rabbit_ct_broker_helpers:set_partition_handling_mode_globally(Config, autoheal), From 0457b588f33751b23fad2a0e618c45174de1701e Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 12:12:36 -0700 Subject: [PATCH 09/29] Simplify --- src/rabbit_connection_tracking_handler.erl | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/rabbit_connection_tracking_handler.erl b/src/rabbit_connection_tracking_handler.erl index 66ef1f2524d8..3fda7e9797c1 100644 --- a/src/rabbit_connection_tracking_handler.erl +++ b/src/rabbit_connection_tracking_handler.erl @@ -76,12 +76,9 @@ handle_event(#event{type = connection_closed, props = Details}, State) -> handle_event(#event{type = vhost_deleted, props = Details}, State) -> VHost = proplists:get_value(name, Details), rabbit_log_connection:info("Closing all connections in vhost '~s' because it's being deleted", [VHost]), - case rabbit_connection_tracking:list(VHost) of - [] -> {ok, State}; - Cs -> - [rabbit_networking:close_connection(Pid, rabbit_misc:format("vhost '~s' is deleted", [VHost])) || #tracked_connection{pid = Pid} <- Cs], - {ok, State} - end; + [rabbit_networking:close_connection(Pid, rabbit_misc:format("vhost '~s' is deleted", [VHost])) || + #tracked_connection{pid = Pid} <- rabbit_connection_tracking:list(VHost)], + {ok, State}; handle_event(#event{type = user_deleted, props = Details}, State) -> _Username = proplists:get_value(name, Details), %% TODO: force close and unregister connections from From a9fbb98cedb912014eec89fb2969c61c52b6df37 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 18:41:13 -0700 Subject: [PATCH 10/29] Use rabbit_misc:pget/2 here --- src/rabbit_connection_tracking.erl | 18 ++++++++++-------- src/rabbit_connection_tracking_handler.erl | 13 +++++++------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 17e2d79034ed..5e8c4415d3e7 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -40,6 +40,8 @@ -include_lib("rabbit.hrl"). +-import(rabbit_misc, [pget/2]). + %% %% API %% @@ -280,17 +282,17 @@ tracked_connection_from_connection_created(EventDetails) -> %% {<<"information">>,longstr, %% <<"http://rubybunny.info">>}]}, %% {connected_at,1453214290847}] - Name = proplists:get_value(name, EventDetails), - Node = proplists:get_value(node, EventDetails), + Name = pget(name, EventDetails), + Node = pget(node, EventDetails), #tracked_connection{id = {Node, Name}, name = Name, node = Node, - vhost = proplists:get_value(vhost, EventDetails), - username = proplists:get_value(user, EventDetails), - connected_at = proplists:get_value(connected_at, EventDetails), - pid = proplists:get_value(pid, EventDetails), - peer_host = proplists:get_value(peer_host, EventDetails), - peer_port = proplists:get_value(peer_port, EventDetails)}. + vhost = pget(vhost, EventDetails), + username = pget(user, EventDetails), + connected_at = pget(connected_at, EventDetails), + pid = pget(pid, EventDetails), + peer_host = pget(peer_host, EventDetails), + peer_port = pget(peer_port, EventDetails)}. tracked_connection_from_connection_state(#connection{ vhost = VHost, diff --git a/src/rabbit_connection_tracking_handler.erl b/src/rabbit_connection_tracking_handler.erl index 3fda7e9797c1..404d84f69b36 100644 --- a/src/rabbit_connection_tracking_handler.erl +++ b/src/rabbit_connection_tracking_handler.erl @@ -28,6 +28,7 @@ terminate/2, code_change/3]). -include_lib("rabbit.hrl"). +-import(rabbit_misc, [pget/2]). -rabbit_boot_step({?MODULE, [{description, "connection tracking event handler"}, @@ -48,7 +49,7 @@ init([]) -> handle_event(#event{type = connection_created, props = Details}, State) -> ThisNode = node(), - case proplists:get_value(node, Details) of + case pget(node, Details) of ThisNode -> rabbit_connection_tracking:register_connection( rabbit_connection_tracking:tracked_connection_from_connection_created(Details) @@ -60,27 +61,27 @@ handle_event(#event{type = connection_created, props = Details}, State) -> {ok, State}; handle_event(#event{type = connection_closed, props = Details}, State) -> ThisNode = node(), - case proplists:get_value(node, Details) of + case pget(node, Details) of ThisNode -> %% [{name,<<"127.0.0.1:64078 -> 127.0.0.1:5672">>}, %% {pid,<0.1774.0>}, %% {node, rabbit@hostname}] rabbit_connection_tracking:unregister_connection( - {proplists:get_value(node, Details), - proplists:get_value(name, Details)}); + {pget(node, Details), + pget(name, Details)}); _OtherNode -> %% ignore ok end, {ok, State}; handle_event(#event{type = vhost_deleted, props = Details}, State) -> - VHost = proplists:get_value(name, Details), + VHost = pget(name, Details), rabbit_log_connection:info("Closing all connections in vhost '~s' because it's being deleted", [VHost]), [rabbit_networking:close_connection(Pid, rabbit_misc:format("vhost '~s' is deleted", [VHost])) || #tracked_connection{pid = Pid} <- rabbit_connection_tracking:list(VHost)], {ok, State}; handle_event(#event{type = user_deleted, props = Details}, State) -> - _Username = proplists:get_value(name, Details), + _Username = pget(name, Details), %% TODO: force close and unregister connections from %% this user. Moved to rabbitmq/rabbitmq-server#628. {ok, State}; From e447910d2765d730be650662d992244877250c98 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 19:03:59 -0700 Subject: [PATCH 11/29] Refactor --- test/per_vhost_connection_limit_SUITE.erl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index 62e42496beb1..f6d2ff9f085c 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -71,14 +71,7 @@ end_per_suite(Config) -> rabbit_ct_helpers:run_teardown_steps(Config). init_per_group(cluster_size_1, Config) -> - Suffix = rabbit_ct_helpers:testcase_absname(Config, "", "-"), - Config1 = rabbit_ct_helpers:set_config(Config, [ - {rmq_nodes_count, 1}, - {rmq_nodename_suffix, Suffix} - ]), - rabbit_ct_helpers:run_steps(Config1, - rabbit_ct_broker_helpers:setup_steps() ++ - rabbit_ct_client_helpers:setup_steps()); + init_per_multinode_group(cluster_size_1, Config, 1); init_per_group(cluster_size_2, Config) -> init_per_multinode_group(cluster_size_2, Config, 2). From 363d0a183e4b54bdb3aa2cf584175ff43156992e Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 4 Aug 2016 22:36:25 -0700 Subject: [PATCH 12/29] One more test case --- test/per_vhost_connection_limit_SUITE.erl | 81 +++++++++++------------ 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index f6d2ff9f085c..033bd0abdda8 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -50,7 +50,8 @@ groups() -> cluster_node_restart_connection_count, cluster_node_list_on_node, cluster_single_vhost_limit, - cluster_single_vhost_limit2 + cluster_single_vhost_limit2, + cluster_vhost_deletion_forces_connection_closure ]} ]. @@ -117,9 +118,7 @@ most_basic_single_node_connection_count(Config) -> Conn = open_unmanaged_connection(Config, 0), ?assertEqual(1, count_connections_in(Config, VHost)), rabbit_ct_client_helpers:close_connection(Conn), - ?assertEqual(0, count_connections_in(Config, VHost)), - - passed. + ?assertEqual(0, count_connections_in(Config, VHost)). single_node_single_vhost_connection_count(Config) -> VHost = <<"/">>, @@ -149,9 +148,7 @@ single_node_single_vhost_connection_count(Config) -> rabbit_ct_client_helpers:close_connection(C) end, [Conn2, Conn3, Conn5]), - ?assertEqual(0, count_connections_in(Config, VHost)), - - passed. + ?assertEqual(0, count_connections_in(Config, VHost)). single_node_multiple_vhost_connection_count(Config) -> VHost1 = <<"vhost1">>, @@ -195,9 +192,7 @@ single_node_multiple_vhost_connection_count(Config) -> ?assertEqual(0, count_connections_in(Config, VHost2)), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), - rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), - - passed. + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2). single_node_list_in_vhost(Config) -> VHost1 = <<"vhost1">>, @@ -237,9 +232,7 @@ single_node_list_in_vhost(Config) -> ?assertEqual(0, length(all_connections(Config))), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), - rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), - - passed. + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2). most_basic_cluster_connection_count(Config) -> VHost = <<"/">>, @@ -257,9 +250,7 @@ most_basic_cluster_connection_count(Config) -> rabbit_ct_client_helpers:close_connection(C) end, [Conn1, Conn2, Conn3]), - ?assertEqual(0, count_connections_in(Config, VHost)), - - passed. + ?assertEqual(0, count_connections_in(Config, VHost)). cluster_single_vhost_connection_count(Config) -> VHost = <<"/">>, @@ -289,9 +280,7 @@ cluster_single_vhost_connection_count(Config) -> rabbit_ct_client_helpers:close_connection(C) end, [Conn2, Conn3, Conn5]), - ?assertEqual(0, count_connections_in(Config, VHost)), - - passed. + ?assertEqual(0, count_connections_in(Config, VHost)). cluster_multiple_vhost_connection_count(Config) -> VHost1 = <<"vhost1">>, @@ -335,9 +324,7 @@ cluster_multiple_vhost_connection_count(Config) -> ?assertEqual(0, count_connections_in(Config, VHost2)), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), - rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), - - passed. + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2). cluster_node_restart_connection_count(Config) -> VHost = <<"/">>, @@ -367,9 +354,7 @@ cluster_node_restart_connection_count(Config) -> (catch rabbit_ct_client_helpers:close_connection(C)) end, [Conn2, Conn3, Conn4, Conn5]), - ?assertEqual(0, count_connections_in(Config, VHost)), - - passed. + ?assertEqual(0, count_connections_in(Config, VHost)). cluster_node_list_on_node(Config) -> [A, B] = rabbit_ct_broker_helpers:get_node_configs(Config, nodename), @@ -410,9 +395,7 @@ cluster_node_list_on_node(Config) -> timer:sleep(100), ?assertEqual(0, length(all_connections(Config, 0))), - rabbit_ct_broker_helpers:start_broker(Config, 1), - - passed. + rabbit_ct_broker_helpers:start_broker(Config, 1). single_node_single_vhost_limit(Config) -> VHost = <<"/">>, @@ -438,9 +421,7 @@ single_node_single_vhost_limit(Config) -> end, [Conn1, Conn2, Conn3, Conn4, Conn5]), ?assertEqual(0, count_connections_in(Config, VHost)), - set_vhost_connection_limit(Config, VHost, 0), - - passed. + set_vhost_connection_limit(Config, VHost, 0). single_node_multiple_vhost_limit(Config) -> VHost1 = <<"vhost1">>, @@ -487,9 +468,7 @@ single_node_multiple_vhost_limit(Config) -> set_vhost_connection_limit(Config, VHost2, 100000000), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), - rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), - - passed. + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2). cluster_single_vhost_limit(Config) -> VHost = <<"/">>, @@ -518,9 +497,7 @@ cluster_single_vhost_limit(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)), - set_vhost_connection_limit(Config, VHost, 0), - - passed. + set_vhost_connection_limit(Config, VHost, 0). cluster_single_vhost_limit2(Config) -> VHost = <<"/">>, @@ -551,9 +528,7 @@ cluster_single_vhost_limit2(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)), - set_vhost_connection_limit(Config, VHost, 0), - - passed. + set_vhost_connection_limit(Config, VHost, 0). single_node_vhost_deletion_forces_connection_closure(Config) -> VHost1 = <<"vhost1">>, @@ -578,10 +553,32 @@ single_node_vhost_deletion_forces_connection_closure(Config) -> rabbit_ct_client_helpers:close_connection(Conn1), ?assertEqual(0, count_connections_in(Config, VHost1)), - rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), + rabbit_ct_broker_helpers:delete_vhost(Config, VHost1). + +cluster_vhost_deletion_forces_connection_closure(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, - passed. + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + ?assertEqual(1, count_connections_in(Config, VHost1)), + + _Conn2 = open_unmanaged_connection(Config, 1, VHost2), + ?assertEqual(1, count_connections_in(Config, VHost2)), + + rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), + timer:sleep(200), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + rabbit_ct_client_helpers:close_connection(Conn1), + ?assertEqual(0, count_connections_in(Config, VHost1)), + rabbit_ct_broker_helpers:delete_vhost(Config, VHost1). %% ------------------------------------------------------------------- %% Helpers From 0199aa9a82055b1be7c5fa0a0d5089bd3fc6607d Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Fri, 5 Aug 2016 17:27:19 -0700 Subject: [PATCH 13/29] Use negative values to disable per-vhost limits, 0 to refuse all connections Per recommendation from @dumbbell. --- src/rabbit_connection_tracking.erl | 25 ++++-- src/rabbit_parameter_validation.erl | 8 +- src/rabbit_vhost_limit.erl | 7 +- test/per_vhost_connection_limit_SUITE.erl | 104 ++++++++++++++++++---- 4 files changed, 115 insertions(+), 29 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 5e8c4415d3e7..2d23e7c888f0 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -195,16 +195,23 @@ list_on_node(Node) -> -spec is_over_connection_limit(rabbit_types:vhost()) -> {true, non_neg_integer()} | false. is_over_connection_limit(VirtualHost) -> - ConnectionCount = count_connections_in(VirtualHost), case rabbit_vhost_limit:connection_limit(VirtualHost) of - undefined -> false; - {ok, Limit} -> case {ConnectionCount, ConnectionCount >= Limit} of - %% 0 = no limit - {0, _} -> false; - %% the limit hasn't been reached - {_, false} -> false; - {_N, true} -> {true, Limit} - end + %% no limit configured + undefined -> false; + %% with limit = 0, no connections are allowed + {ok, 0} -> {true, 0}; + {ok, Limit} when is_integer(Limit) andalso Limit > 0 -> + ConnectionCount = count_connections_in(VirtualHost), + case ConnectionCount >= Limit of + false -> false; + true -> {true, Limit} + end; + %% any negative value means "no limit". Note that parameter validation + %% will replace negative integers with 'undefined', so this is to be + %% explicit and extra defensive + {ok, Limit} when is_integer(Limit) andalso Limit < 0 -> false; + %% ignore non-integer limits + {ok, _Limit} -> false end. diff --git a/src/rabbit_parameter_validation.erl b/src/rabbit_parameter_validation.erl index 90ab1d528611..00e83d275718 100644 --- a/src/rabbit_parameter_validation.erl +++ b/src/rabbit_parameter_validation.erl @@ -16,7 +16,7 @@ -module(rabbit_parameter_validation). --export([number/2, binary/2, boolean/2, list/2, regex/2, proplist/3, enum/1]). +-export([number/2, integer/2, binary/2, boolean/2, list/2, regex/2, proplist/3, enum/1]). number(_Name, Term) when is_number(Term) -> ok; @@ -24,6 +24,12 @@ number(_Name, Term) when is_number(Term) -> number(Name, Term) -> {error, "~s should be number, actually was ~p", [Name, Term]}. +integer(_Name, Term) when is_integer(Term) -> + ok; + +integer(Name, Term) -> + {error, "~s should be number, actually was ~p", [Name, Term]}. + binary(_Name, Term) when is_binary(Term) -> ok; diff --git a/src/rabbit_vhost_limit.erl b/src/rabbit_vhost_limit.erl index dd7ce51089f4..2d9a2f075e2d 100644 --- a/src/rabbit_vhost_limit.erl +++ b/src/rabbit_vhost_limit.erl @@ -72,7 +72,7 @@ clear(VHost) -> <<"limits">>). vhost_limit_validation() -> - [{<<"max-connections">>, fun rabbit_parameter_validation:number/2, mandatory}]. + [{<<"max-connections">>, fun rabbit_parameter_validation:integer/2, mandatory}]. update_vhost(VHostName, Limits) -> rabbit_misc:execute_mnesia_transaction( @@ -91,8 +91,9 @@ get_limit(VirtualHost, Limit) -> undefined -> undefined; Val -> case pget(Limit, Val) of undefined -> undefined; - N when N =< 0 -> undefined; - N when N > 0 -> {ok, N} + %% no limit + N when N < 0 -> undefined; + N when N >= 0 -> {ok, N} end end end. diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index 033bd0abdda8..f327e82cb743 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -40,7 +40,9 @@ groups() -> single_node_multiple_vhost_connection_count, single_node_list_in_vhost, single_node_single_vhost_limit, + single_node_single_vhost_zero_limit, single_node_multiple_vhost_limit, + single_node_multiple_vhost_zero_limit, single_node_vhost_deletion_forces_connection_closure ]}, {cluster_size_2, [], [ @@ -398,6 +400,10 @@ cluster_node_list_on_node(Config) -> rabbit_ct_broker_helpers:start_broker(Config, 1). single_node_single_vhost_limit(Config) -> + single_node_single_vhost_limit_with(Config, 5), + single_node_single_vhost_limit_with(Config, -1). + +single_node_single_vhost_limit_with(Config, WatermarkLimit) -> VHost = <<"/">>, set_vhost_connection_limit(Config, VHost, 3), @@ -408,11 +414,11 @@ single_node_single_vhost_limit(Config) -> Conn3 = open_unmanaged_connection(Config, 0), %% we've crossed the limit - {error, not_allowed} = open_unmanaged_connection(Config, 0), - {error, not_allowed} = open_unmanaged_connection(Config, 0), - {error, not_allowed} = open_unmanaged_connection(Config, 0), + expect_that_client_connection_is_rejected(Config, 0), + expect_that_client_connection_is_rejected(Config, 0), + expect_that_client_connection_is_rejected(Config, 0), - set_vhost_connection_limit(Config, VHost, 5), + set_vhost_connection_limit(Config, VHost, WatermarkLimit), Conn4 = open_unmanaged_connection(Config, 0), Conn5 = open_unmanaged_connection(Config, 0), @@ -421,7 +427,29 @@ single_node_single_vhost_limit(Config) -> end, [Conn1, Conn2, Conn3, Conn4, Conn5]), ?assertEqual(0, count_connections_in(Config, VHost)), - set_vhost_connection_limit(Config, VHost, 0). + set_vhost_connection_limit(Config, VHost, -1). + +single_node_single_vhost_zero_limit(Config) -> + VHost = <<"/">>, + set_vhost_connection_limit(Config, VHost, 0), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + %% with limit = 0 no connections are allowed + expect_that_client_connection_is_rejected(Config), + expect_that_client_connection_is_rejected(Config), + expect_that_client_connection_is_rejected(Config), + + set_vhost_connection_limit(Config, VHost, -1), + Conn1 = open_unmanaged_connection(Config, 0), + Conn2 = open_unmanaged_connection(Config, 0), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2]), + + ?assertEqual(0, count_connections_in(Config, VHost)). + single_node_multiple_vhost_limit(Config) -> VHost1 = <<"vhost1">>, @@ -442,13 +470,13 @@ single_node_multiple_vhost_limit(Config) -> Conn4 = open_unmanaged_connection(Config, 0, VHost2), %% we've crossed the limit - {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost1), - {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost2), + expect_that_client_connection_is_rejected(Config, 0, VHost1), + expect_that_client_connection_is_rejected(Config, 0, VHost2), Conn5 = open_unmanaged_connection(Config, 0), set_vhost_connection_limit(Config, VHost1, 5), - set_vhost_connection_limit(Config, VHost2, 5), + set_vhost_connection_limit(Config, VHost2, -10), Conn6 = open_unmanaged_connection(Config, 0, VHost1), Conn7 = open_unmanaged_connection(Config, 0, VHost1), @@ -464,12 +492,47 @@ single_node_multiple_vhost_limit(Config) -> ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), - set_vhost_connection_limit(Config, VHost1, 100000000), - set_vhost_connection_limit(Config, VHost2, 100000000), + set_vhost_connection_limit(Config, VHost1, -1), + set_vhost_connection_limit(Config, VHost2, -1), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), rabbit_ct_broker_helpers:delete_vhost(Config, VHost2). + +single_node_multiple_vhost_zero_limit(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, + + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + set_vhost_connection_limit(Config, VHost1, 0), + set_vhost_connection_limit(Config, VHost2, 0), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + %% with limit = 0 no connections are allowed + expect_that_client_connection_is_rejected(Config, 0, VHost1), + expect_that_client_connection_is_rejected(Config, 0, VHost2), + expect_that_client_connection_is_rejected(Config, 0, VHost1), + + set_vhost_connection_limit(Config, VHost1, -1), + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + Conn2 = open_unmanaged_connection(Config, 0, VHost1), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2]), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + set_vhost_connection_limit(Config, VHost1, -1), + set_vhost_connection_limit(Config, VHost2, -1). + + + cluster_single_vhost_limit(Config) -> VHost = <<"/">>, set_vhost_connection_limit(Config, VHost, 2), @@ -483,8 +546,8 @@ cluster_single_vhost_limit(Config) -> timer:sleep(200), %% we've crossed the limit - {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost), - {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost), + expect_that_client_connection_is_rejected(Config, 0, VHost), + expect_that_client_connection_is_rejected(Config, 1, VHost), set_vhost_connection_limit(Config, VHost, 5), @@ -497,7 +560,7 @@ cluster_single_vhost_limit(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)), - set_vhost_connection_limit(Config, VHost, 0). + set_vhost_connection_limit(Config, VHost, -1). cluster_single_vhost_limit2(Config) -> VHost = <<"/">>, @@ -510,10 +573,10 @@ cluster_single_vhost_limit2(Config) -> Conn2 = open_unmanaged_connection(Config, 0, VHost), %% we've crossed the limit - {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost), + expect_that_client_connection_is_rejected(Config, 0, VHost), timer:sleep(200), - {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost), + expect_that_client_connection_is_rejected(Config, 1, VHost), set_vhost_connection_limit(Config, VHost, 5), @@ -528,7 +591,7 @@ cluster_single_vhost_limit2(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)), - set_vhost_connection_limit(Config, VHost, 0). + set_vhost_connection_limit(Config, VHost, -1). single_node_vhost_deletion_forces_connection_closure(Config) -> VHost1 = <<"vhost1">>, @@ -634,3 +697,12 @@ set_vhost_connection_limit(Config, NodeIndex, VHost, Count) -> await_running_node_refresh(_Config, _NodeIndex) -> timer:sleep(250). + +expect_that_client_connection_is_rejected(Config) -> + expect_that_client_connection_is_rejected(Config, 0). + +expect_that_client_connection_is_rejected(Config, NodeIndex) -> + {error, not_allowed} = open_unmanaged_connection(Config, NodeIndex). + +expect_that_client_connection_is_rejected(Config, NodeIndex, VHost) -> + {error, not_allowed} = open_unmanaged_connection(Config, NodeIndex, VHost). From d36d1d4dfe528b9426f7b0f3452e54d033667bc8 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Fri, 5 Aug 2016 17:55:32 -0700 Subject: [PATCH 14/29] More test cases --- test/per_vhost_connection_limit_SUITE.erl | 86 ++++++++++++++++++++--- 1 file changed, 77 insertions(+), 9 deletions(-) diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index f327e82cb743..da197ca6ec3d 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -37,22 +37,24 @@ groups() -> {cluster_size_1, [], [ most_basic_single_node_connection_count, single_node_single_vhost_connection_count, - single_node_multiple_vhost_connection_count, + single_node_multiple_vhosts_connection_count, single_node_list_in_vhost, single_node_single_vhost_limit, single_node_single_vhost_zero_limit, - single_node_multiple_vhost_limit, - single_node_multiple_vhost_zero_limit, + single_node_multiple_vhosts_limit, + single_node_multiple_vhosts_zero_limit, single_node_vhost_deletion_forces_connection_closure ]}, {cluster_size_2, [], [ most_basic_cluster_connection_count, cluster_single_vhost_connection_count, - cluster_multiple_vhost_connection_count, + cluster_multiple_vhosts_connection_count, cluster_node_restart_connection_count, cluster_node_list_on_node, cluster_single_vhost_limit, cluster_single_vhost_limit2, + cluster_single_vhost_zero_limit, + cluster_multiple_vhosts_zero_limit, cluster_vhost_deletion_forces_connection_closure ]} ]. @@ -152,7 +154,7 @@ single_node_single_vhost_connection_count(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)). -single_node_multiple_vhost_connection_count(Config) -> +single_node_multiple_vhosts_connection_count(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -284,7 +286,7 @@ cluster_single_vhost_connection_count(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)). -cluster_multiple_vhost_connection_count(Config) -> +cluster_multiple_vhosts_connection_count(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -451,7 +453,7 @@ single_node_single_vhost_zero_limit(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)). -single_node_multiple_vhost_limit(Config) -> +single_node_multiple_vhosts_limit(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -499,7 +501,7 @@ single_node_multiple_vhost_limit(Config) -> rabbit_ct_broker_helpers:delete_vhost(Config, VHost2). -single_node_multiple_vhost_zero_limit(Config) -> +single_node_multiple_vhosts_zero_limit(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -593,6 +595,71 @@ cluster_single_vhost_limit2(Config) -> set_vhost_connection_limit(Config, VHost, -1). + +cluster_single_vhost_zero_limit(Config) -> + VHost = <<"/">>, + set_vhost_connection_limit(Config, VHost, 0), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + %% with limit = 0 no connections are allowed + expect_that_client_connection_is_rejected(Config, 0), + expect_that_client_connection_is_rejected(Config, 1), + expect_that_client_connection_is_rejected(Config, 0), + + set_vhost_connection_limit(Config, VHost, -1), + Conn1 = open_unmanaged_connection(Config, 0), + Conn2 = open_unmanaged_connection(Config, 1), + Conn3 = open_unmanaged_connection(Config, 0), + Conn4 = open_unmanaged_connection(Config, 1), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4]), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + set_vhost_connection_limit(Config, VHost, -1). + + +cluster_multiple_vhosts_zero_limit(Config) -> + VHost1 = <<"vhost1">>, + VHost2 = <<"vhost2">>, + + set_up_vhost(Config, VHost1), + set_up_vhost(Config, VHost2), + + set_vhost_connection_limit(Config, VHost1, 0), + set_vhost_connection_limit(Config, VHost2, 0), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + %% with limit = 0 no connections are allowed + expect_that_client_connection_is_rejected(Config, 0, VHost1), + expect_that_client_connection_is_rejected(Config, 0, VHost2), + expect_that_client_connection_is_rejected(Config, 1, VHost1), + expect_that_client_connection_is_rejected(Config, 1, VHost2), + + set_vhost_connection_limit(Config, VHost1, -1), + set_vhost_connection_limit(Config, VHost2, -1), + + Conn1 = open_unmanaged_connection(Config, 0, VHost1), + Conn2 = open_unmanaged_connection(Config, 0, VHost2), + Conn3 = open_unmanaged_connection(Config, 1, VHost1), + Conn4 = open_unmanaged_connection(Config, 1, VHost2), + + lists:foreach(fun (C) -> + rabbit_ct_client_helpers:close_connection(C) + end, [Conn1, Conn2, Conn3, Conn4]), + + ?assertEqual(0, count_connections_in(Config, VHost1)), + ?assertEqual(0, count_connections_in(Config, VHost2)), + + set_vhost_connection_limit(Config, VHost1, -1), + set_vhost_connection_limit(Config, VHost2, -1). + + single_node_vhost_deletion_forces_connection_closure(Config) -> VHost1 = <<"vhost1">>, VHost2 = <<"vhost2">>, @@ -682,7 +749,8 @@ all_connections(Config, NodeIndex) -> set_up_vhost(Config, VHost) -> rabbit_ct_broker_helpers:add_vhost(Config, VHost), - rabbit_ct_broker_helpers:set_full_permissions(Config, <<"guest">>, VHost). + rabbit_ct_broker_helpers:set_full_permissions(Config, <<"guest">>, VHost), + set_vhost_connection_limit(Config, VHost, -1). set_vhost_connection_limit(Config, VHost, Count) -> set_vhost_connection_limit(Config, 0, VHost, Count). From efc0f993e1e66b724f22be77bbc1d3830b2ddcdb Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Wed, 10 Aug 2016 18:37:06 +0300 Subject: [PATCH 15/29] We can use record_info/2 here; log errors in table creation --- src/rabbit_connection_tracking.erl | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 2d23e7c888f0..fe5392a8a07e 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -78,13 +78,11 @@ ensure_per_vhost_tracked_connections_table_for_this_node() -> ensure_tracked_connections_table_for_node(Node) -> TableName = tracked_connection_table_name_for(Node), case mnesia:create_table(TableName, [{record_name, tracked_connection}, - {attributes, [id, node, vhost, name, - pid, protocol, - peer_host, peer_port, - username, connected_at]}]) of - {atomic, ok} -> ok; - {aborted, _} -> ok - %% TODO: propagate errors + {attributes, record_info(fields, tracked_connection)}]) of + {atomic, ok} -> ok; + {aborted, Error} -> + rabbit_log:error("Failed to create a tracked connection table for node ~p: ~p", [Node, Error]), + ok end. From ede361d09288eb8a04cf55f0f7e53f01c069d6c0 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Wed, 10 Aug 2016 20:18:33 +0300 Subject: [PATCH 16/29] Emit a node_deleted event when a node is removed from the cluster --- src/rabbit_mnesia.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index 0c46193ca36a..a1cfe0948f13 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -308,7 +308,9 @@ forget_cluster_node(Node, RemoveWhenOffline) -> {false, true} -> rabbit_log:info( "Removing node ~p from cluster~n", [Node]), case remove_node_if_mnesia_running(Node) of - ok -> ok; + ok -> + rabbit_event:notify(node_deleted, [{node, Node}]), + ok; {error, _} = Err -> throw(Err) end end. From 0363bac78b403e666ab36584f58c527682ac7869 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Sun, 14 Aug 2016 10:09:30 +0300 Subject: [PATCH 17/29] Delete connection tracking tables for nodes removed from the cluster --- src/rabbit_connection_tracking.erl | 31 ++++++++++++++++++++-- src/rabbit_connection_tracking_handler.erl | 7 +++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index fe5392a8a07e..27d713ae4e2a 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -31,6 +31,7 @@ ensure_tracked_connections_table_for_this_node/0, ensure_per_vhost_tracked_connections_table_for_this_node/0, tracked_connection_table_name_for/1, tracked_connection_per_vhost_table_name_for/1, + delete_tracked_connections_table_for_node/1, delete_per_vhost_tracked_connections_table_for_node/1, clear_tracked_connections_table_for_this_node/0, register_connection/1, unregister_connection/1, list/0, list/1, list_on_node/1, @@ -52,10 +53,10 @@ %% node. boot() -> ensure_tracked_connections_table_for_this_node(), - rabbit_log:info("Created a table for connection tracking on this node: ~p", + rabbit_log:info("Setting up a table for connection tracking on this node: ~p", [tracked_connection_table_name_for(node())]), ensure_per_vhost_tracked_connections_table_for_this_node(), - rabbit_log:info("Created a table for per-vhost connection counting on this node: ~p", + rabbit_log:info("Setting up a table for per-vhost connection counting on this node: ~p", [tracked_connection_per_vhost_table_name_for(node())]), clear_tracked_connections_table_for_this_node(), ok. @@ -111,6 +112,32 @@ clear_tracked_connections_table_for_this_node() -> end. +-spec delete_tracked_connections_table_for_node(node()) -> ok. + +delete_tracked_connections_table_for_node(Node) -> + TableName = tracked_connection_table_name_for(Node), + case mnesia:delete_table(TableName) of + {atomic, ok} -> ok; + {aborted, {no_exists, _}} -> ok; + {aborted, Error} -> + rabbit_log:error("Failed to delete a tracked connection table for node ~p: ~p", [Node, Error]), + ok + end. + + +-spec delete_per_vhost_tracked_connections_table_for_node(node()) -> ok. + +delete_per_vhost_tracked_connections_table_for_node(Node) -> + TableName = tracked_connection_per_vhost_table_name_for(Node), + case mnesia:delete_table(TableName) of + {atomic, ok} -> ok; + {aborted, {no_exists, _}} -> ok; + {aborted, Error} -> + rabbit_log:error("Failed to delete a per-vhost tracked connection table for node ~p: ~p", [Node, Error]), + ok + end. + + -spec tracked_connection_table_name_for(node()) -> atom(). tracked_connection_table_name_for(Node) -> diff --git a/src/rabbit_connection_tracking_handler.erl b/src/rabbit_connection_tracking_handler.erl index 404d84f69b36..fd1df8c88adc 100644 --- a/src/rabbit_connection_tracking_handler.erl +++ b/src/rabbit_connection_tracking_handler.erl @@ -85,6 +85,13 @@ handle_event(#event{type = user_deleted, props = Details}, State) -> %% TODO: force close and unregister connections from %% this user. Moved to rabbitmq/rabbitmq-server#628. {ok, State}; +%% A node had been deleted from the cluster. +handle_event(#event{type = node_deleted, props = Details}, State) -> + Node = pget(node, Details), + rabbit_log_connection:info("Node '~s' was removed from the cluster, deleting its connection tracking tables...", [Node]), + rabbit_connection_tracking:delete_tracked_connections_table_for_node(Node), + rabbit_connection_tracking:delete_per_vhost_tracked_connections_table_for_node(Node), + {ok, State}; handle_event(_Event, State) -> {ok, State}. From 93682bd779b4af0f1350dd9363be727418d081bf Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Sun, 14 Aug 2016 11:03:42 +0300 Subject: [PATCH 18/29] Rename function --- src/rabbit_connection_tracking.erl | 8 ++++---- test/per_vhost_connection_limit_SUITE.erl | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 27d713ae4e2a..8220495a72d1 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -32,7 +32,7 @@ ensure_per_vhost_tracked_connections_table_for_this_node/0, tracked_connection_table_name_for/1, tracked_connection_per_vhost_table_name_for/1, delete_tracked_connections_table_for_node/1, delete_per_vhost_tracked_connections_table_for_node/1, - clear_tracked_connections_table_for_this_node/0, + clear_tracked_connection_tables_for_this_node/0, register_connection/1, unregister_connection/1, list/0, list/1, list_on_node/1, tracked_connection_from_connection_created/1, @@ -58,7 +58,7 @@ boot() -> ensure_per_vhost_tracked_connections_table_for_this_node(), rabbit_log:info("Setting up a table for per-vhost connection counting on this node: ~p", [tracked_connection_per_vhost_table_name_for(node())]), - clear_tracked_connections_table_for_this_node(), + clear_tracked_connection_tables_for_this_node(), ok. @@ -99,9 +99,9 @@ ensure_per_vhost_tracked_connections_table_for_node(Node) -> end. --spec clear_tracked_connections_table_for_this_node() -> ok. +-spec clear_tracked_connection_tables_for_this_node() -> ok. -clear_tracked_connections_table_for_this_node() -> +clear_tracked_connection_tables_for_this_node() -> case mnesia:clear_table(tracked_connection_table_name_for(node())) of {atomic, ok} -> ok; {aborted, _} -> ok diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index da197ca6ec3d..6b6293922676 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -109,7 +109,7 @@ clear_all_connection_tracking_tables(Config) -> [rabbit_ct_broker_helpers:rpc(Config, N, rabbit_connection_tracking, - clear_tracked_connections_table_for_this_node, + clear_tracked_connection_tables_for_this_node, []) || N <- rabbit_ct_broker_helpers:get_node_configs(Config, nodename)]. %% ------------------------------------------------------------------- From 9febe154c3f396cebee365543266bba3cb4b2c75 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Wed, 17 Aug 2016 01:30:34 +0300 Subject: [PATCH 19/29] Combine map/2 and foldl/3 --- src/rabbit_connection_tracking.erl | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 8220495a72d1..752bb885c500 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -243,12 +243,10 @@ is_over_connection_limit(VirtualHost) -> -spec count_connections_in(rabbit_types:vhost()) -> non_neg_integer(). count_connections_in(VirtualHost) -> - try - Ns = lists:map( - fun (Node) -> + lists:foldl(fun (Node, Acc) -> Tab = tracked_connection_per_vhost_table_name_for(Node), try - case mnesia:transaction( + N = case mnesia:transaction( fun() -> case mnesia:dirty_read({Tab, VirtualHost}) of [] -> 0; @@ -257,18 +255,15 @@ count_connections_in(VirtualHost) -> end) of {atomic, Val} -> Val; {aborted, _Reason} -> 0 - end - catch _ -> 0 - end - end, rabbit_mnesia:cluster_nodes(running)), - lists:foldl(fun(X, Acc) -> Acc + X end, 0, Ns) - catch - _:Err -> + end, + Acc + N + catch _:Err -> rabbit_log:error( - "Failed to fetch number of connections in vhost ~p:~n~p~n", - [VirtualHost, Err]), - 0 - end. + "Failed to fetch number of connections in vhost ~p on node ~p:~n~p~n", + [VirtualHost, Err, Node]), + Acc + end + end, 0, rabbit_mnesia:cluster_nodes(running)). %% Returns a #tracked_connection from connection_created %% event details. From 0db9986808b207466aad066f380f7f473268d6e0 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 18 Aug 2016 09:32:50 +0300 Subject: [PATCH 20/29] Reformat with emacs --- src/rabbit_connection_tracking.erl | 200 ++++++++++++++--------------- 1 file changed, 100 insertions(+), 100 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 752bb885c500..8ce32fb609c3 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -52,26 +52,26 @@ %% Sets up and resets connection tracking tables for this %% node. boot() -> - ensure_tracked_connections_table_for_this_node(), - rabbit_log:info("Setting up a table for connection tracking on this node: ~p", - [tracked_connection_table_name_for(node())]), - ensure_per_vhost_tracked_connections_table_for_this_node(), - rabbit_log:info("Setting up a table for per-vhost connection counting on this node: ~p", - [tracked_connection_per_vhost_table_name_for(node())]), - clear_tracked_connection_tables_for_this_node(), - ok. + ensure_tracked_connections_table_for_this_node(), + rabbit_log:info("Setting up a table for connection tracking on this node: ~p", + [tracked_connection_table_name_for(node())]), + ensure_per_vhost_tracked_connections_table_for_this_node(), + rabbit_log:info("Setting up a table for per-vhost connection counting on this node: ~p", + [tracked_connection_per_vhost_table_name_for(node())]), + clear_tracked_connection_tables_for_this_node(), + ok. -spec ensure_tracked_connections_table_for_this_node() -> ok. ensure_tracked_connections_table_for_this_node() -> - ensure_tracked_connections_table_for_node(node()). + ensure_tracked_connections_table_for_node(node()). -spec ensure_per_vhost_tracked_connections_table_for_this_node() -> ok. ensure_per_vhost_tracked_connections_table_for_this_node() -> - ensure_per_vhost_tracked_connections_table_for_node(node()). + ensure_per_vhost_tracked_connections_table_for_node(node()). -spec ensure_tracked_connections_table_for_node(node()) -> ok. @@ -80,10 +80,10 @@ ensure_tracked_connections_table_for_node(Node) -> TableName = tracked_connection_table_name_for(Node), case mnesia:create_table(TableName, [{record_name, tracked_connection}, {attributes, record_info(fields, tracked_connection)}]) of - {atomic, ok} -> ok; - {aborted, Error} -> - rabbit_log:error("Failed to create a tracked connection table for node ~p: ~p", [Node, Error]), - ok + {atomic, ok} -> ok; + {aborted, Error} -> + rabbit_log:error("Failed to create a tracked connection table for node ~p: ~p", [Node, Error]), + ok end. @@ -92,24 +92,24 @@ ensure_tracked_connections_table_for_node(Node) -> ensure_per_vhost_tracked_connections_table_for_node(Node) -> TableName = tracked_connection_per_vhost_table_name_for(Node), case mnesia:create_table(TableName, [{record_name, tracked_connection_per_vhost}, - {attributes, [vhost, connection_count]}]) of - {atomic, ok} -> ok; - {aborted, _} -> ok - %% TODO: propagate errors + {attributes, [vhost, connection_count]}]) of + {atomic, ok} -> ok; + {aborted, _} -> ok + %% TODO: propagate errors end. -spec clear_tracked_connection_tables_for_this_node() -> ok. clear_tracked_connection_tables_for_this_node() -> - case mnesia:clear_table(tracked_connection_table_name_for(node())) of - {atomic, ok} -> ok; - {aborted, _} -> ok - end, - case mnesia:clear_table(tracked_connection_per_vhost_table_name_for(node())) of - {atomic, ok} -> ok; - {aborted, _} -> ok - end. + case mnesia:clear_table(tracked_connection_table_name_for(node())) of + {atomic, ok} -> ok; + {aborted, _} -> ok + end, + case mnesia:clear_table(tracked_connection_per_vhost_table_name_for(node())) of + {atomic, ok} -> ok; + {aborted, _} -> ok + end. -spec delete_tracked_connections_table_for_node(node()) -> ok. @@ -117,11 +117,11 @@ clear_tracked_connection_tables_for_this_node() -> delete_tracked_connections_table_for_node(Node) -> TableName = tracked_connection_table_name_for(Node), case mnesia:delete_table(TableName) of - {atomic, ok} -> ok; - {aborted, {no_exists, _}} -> ok; - {aborted, Error} -> - rabbit_log:error("Failed to delete a tracked connection table for node ~p: ~p", [Node, Error]), - ok + {atomic, ok} -> ok; + {aborted, {no_exists, _}} -> ok; + {aborted, Error} -> + rabbit_log:error("Failed to delete a tracked connection table for node ~p: ~p", [Node, Error]), + ok end. @@ -130,23 +130,23 @@ delete_tracked_connections_table_for_node(Node) -> delete_per_vhost_tracked_connections_table_for_node(Node) -> TableName = tracked_connection_per_vhost_table_name_for(Node), case mnesia:delete_table(TableName) of - {atomic, ok} -> ok; - {aborted, {no_exists, _}} -> ok; - {aborted, Error} -> - rabbit_log:error("Failed to delete a per-vhost tracked connection table for node ~p: ~p", [Node, Error]), - ok + {atomic, ok} -> ok; + {aborted, {no_exists, _}} -> ok; + {aborted, Error} -> + rabbit_log:error("Failed to delete a per-vhost tracked connection table for node ~p: ~p", [Node, Error]), + ok end. -spec tracked_connection_table_name_for(node()) -> atom(). tracked_connection_table_name_for(Node) -> - list_to_atom(rabbit_misc:format("tracked_connection_on_node_~s", [Node])). + list_to_atom(rabbit_misc:format("tracked_connection_on_node_~s", [Node])). -spec tracked_connection_per_vhost_table_name_for(node()) -> atom(). tracked_connection_per_vhost_table_name_for(Node) -> - list_to_atom(rabbit_misc:format("tracked_connection_per_vhost_on_node_~s", [Node])). + list_to_atom(rabbit_misc:format("tracked_connection_per_vhost_on_node_~s", [Node])). -spec register_connection(rabbit_types:tracked_connection()) -> ok. @@ -156,12 +156,12 @@ register_connection(#tracked_connection{vhost = VHost, id = ConnId, node = Node} PerVhostTableName = tracked_connection_per_vhost_table_name_for(Node), rabbit_misc:execute_mnesia_transaction( fun() -> - %% upsert + %% upsert case mnesia:dirty_read(TableName, ConnId) of [] -> - %% TODO: counter table - mnesia:write(TableName, Conn, write), - mnesia:dirty_update_counter( + %% TODO: counter table + mnesia:write(TableName, Conn, write), + mnesia:dirty_update_counter( PerVhostTableName, VHost, 1); [_Row] -> ok @@ -189,33 +189,33 @@ unregister_connection(ConnId = {Node, _Name}) when Node =:= node() -> -spec list() -> [rabbit_types:tracked_connection()]. list() -> - Chunks = lists:map( - fun (Node) -> - Tab = tracked_connection_table_name_for(Node), - mnesia:dirty_match_object(Tab, #tracked_connection{_ = '_'}) - end, rabbit_mnesia:cluster_nodes(running)), - lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). + Chunks = lists:map( + fun (Node) -> + Tab = tracked_connection_table_name_for(Node), + mnesia:dirty_match_object(Tab, #tracked_connection{_ = '_'}) + end, rabbit_mnesia:cluster_nodes(running)), + lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). -spec list(rabbit_types:vhost()) -> [rabbit_types:tracked_connection()]. list(VHost) -> - Chunks = lists:map( - fun (Node) -> - Tab = tracked_connection_table_name_for(Node), - mnesia:dirty_match_object(Tab, #tracked_connection{vhost = VHost, _ = '_'}) - end, rabbit_mnesia:cluster_nodes(running)), - lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). + Chunks = lists:map( + fun (Node) -> + Tab = tracked_connection_table_name_for(Node), + mnesia:dirty_match_object(Tab, #tracked_connection{vhost = VHost, _ = '_'}) + end, rabbit_mnesia:cluster_nodes(running)), + lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). -spec list_on_node(node()) -> [rabbit_types:tracked_connection()]. list_on_node(Node) -> - try mnesia:dirty_match_object( - tracked_connection_table_name_for(Node), - #tracked_connection{_ = '_'}) - catch exit:{aborted, {no_exists, _}} -> [] - end. + try mnesia:dirty_match_object( + tracked_connection_table_name_for(Node), + #tracked_connection{_ = '_'}) + catch exit:{aborted, {no_exists, _}} -> [] + end. -spec is_over_connection_limit(rabbit_types:vhost()) -> {true, non_neg_integer()} | false. @@ -226,11 +226,11 @@ is_over_connection_limit(VirtualHost) -> %% with limit = 0, no connections are allowed {ok, 0} -> {true, 0}; {ok, Limit} when is_integer(Limit) andalso Limit > 0 -> - ConnectionCount = count_connections_in(VirtualHost), - case ConnectionCount >= Limit of - false -> false; - true -> {true, Limit} - end; + ConnectionCount = count_connections_in(VirtualHost), + case ConnectionCount >= Limit of + false -> false; + true -> {true, Limit} + end; %% any negative value means "no limit". Note that parameter validation %% will replace negative integers with 'undefined', so this is to be %% explicit and extra defensive @@ -244,26 +244,26 @@ is_over_connection_limit(VirtualHost) -> count_connections_in(VirtualHost) -> lists:foldl(fun (Node, Acc) -> - Tab = tracked_connection_per_vhost_table_name_for(Node), - try - N = case mnesia:transaction( - fun() -> - case mnesia:dirty_read({Tab, VirtualHost}) of - [] -> 0; - [Val] -> Val#tracked_connection_per_vhost.connection_count - end - end) of - {atomic, Val} -> Val; - {aborted, _Reason} -> 0 - end, - Acc + N - catch _:Err -> - rabbit_log:error( - "Failed to fetch number of connections in vhost ~p on node ~p:~n~p~n", - [VirtualHost, Err, Node]), - Acc - end - end, 0, rabbit_mnesia:cluster_nodes(running)). + Tab = tracked_connection_per_vhost_table_name_for(Node), + try + N = case mnesia:transaction( + fun() -> + case mnesia:dirty_read({Tab, VirtualHost}) of + [] -> 0; + [Val] -> Val#tracked_connection_per_vhost.connection_count + end + end) of + {atomic, Val} -> Val; + {aborted, _Reason} -> 0 + end, + Acc + N + catch _:Err -> + rabbit_log:error( + "Failed to fetch number of connections in vhost ~p on node ~p:~n~p~n", + [VirtualHost, Err, Node]), + Acc + end + end, 0, rabbit_mnesia:cluster_nodes(running)). %% Returns a #tracked_connection from connection_created %% event details. @@ -322,19 +322,19 @@ tracked_connection_from_connection_created(EventDetails) -> peer_port = pget(peer_port, EventDetails)}. tracked_connection_from_connection_state(#connection{ - vhost = VHost, - connected_at = Ts, - peer_host = PeerHost, - peer_port = PeerPort, - user = Username, - name = Name - }) -> - tracked_connection_from_connection_created( - [{name, Name}, - {node, node()}, - {vhost, VHost}, - {user, Username}, - {connected_at, Ts}, - {pid, self()}, - {peer_port, PeerPort}, - {peer_host, PeerHost}]). + vhost = VHost, + connected_at = Ts, + peer_host = PeerHost, + peer_port = PeerPort, + user = Username, + name = Name + }) -> + tracked_connection_from_connection_created( + [{name, Name}, + {node, node()}, + {vhost, VHost}, + {user, Username}, + {connected_at, Ts}, + {pid, self()}, + {peer_port, PeerPort}, + {peer_host, PeerHost}]). From 769472f9baa6bc66d77eadd1362bfc8a0bd2d33b Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 18 Aug 2016 09:38:29 +0300 Subject: [PATCH 21/29] Use lists:foldl/3 only --- src/rabbit_connection_tracking.erl | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 8ce32fb609c3..948bdc454b13 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -159,7 +159,6 @@ register_connection(#tracked_connection{vhost = VHost, id = ConnId, node = Node} %% upsert case mnesia:dirty_read(TableName, ConnId) of [] -> - %% TODO: counter table mnesia:write(TableName, Conn, write), mnesia:dirty_update_counter( PerVhostTableName, VHost, 1); @@ -189,23 +188,21 @@ unregister_connection(ConnId = {Node, _Name}) when Node =:= node() -> -spec list() -> [rabbit_types:tracked_connection()]. list() -> - Chunks = lists:map( - fun (Node) -> + Chunks = lists:foldl( + fun (Node, Acc) -> Tab = tracked_connection_table_name_for(Node), - mnesia:dirty_match_object(Tab, #tracked_connection{_ = '_'}) - end, rabbit_mnesia:cluster_nodes(running)), - lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). + Acc ++ mnesia:dirty_match_object(Tab, #tracked_connection{_ = '_'}) + end, [], rabbit_mnesia:cluster_nodes(running)). -spec list(rabbit_types:vhost()) -> [rabbit_types:tracked_connection()]. list(VHost) -> - Chunks = lists:map( - fun (Node) -> + Chunks = lists:foldl( + fun (Node, Acc) -> Tab = tracked_connection_table_name_for(Node), - mnesia:dirty_match_object(Tab, #tracked_connection{vhost = VHost, _ = '_'}) - end, rabbit_mnesia:cluster_nodes(running)), - lists:foldl(fun(Chunk, Acc) -> Acc ++ Chunk end, [], Chunks). + Acc ++ mnesia:dirty_match_object(Tab, #tracked_connection{vhost = VHost, _ = '_'}) + end, [], rabbit_mnesia:cluster_nodes(running)). -spec list_on_node(node()) -> [rabbit_types:tracked_connection()]. From bbe671dd2a2bd9939b0368c19b9427e71ba1a305 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 18 Aug 2016 09:50:02 +0300 Subject: [PATCH 22/29] record_info/2 is appropriate here --- src/rabbit_connection_tracking.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index 948bdc454b13..bd82b7c27e51 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -92,7 +92,7 @@ ensure_tracked_connections_table_for_node(Node) -> ensure_per_vhost_tracked_connections_table_for_node(Node) -> TableName = tracked_connection_per_vhost_table_name_for(Node), case mnesia:create_table(TableName, [{record_name, tracked_connection_per_vhost}, - {attributes, [vhost, connection_count]}]) of + {attributes, record_info(fields, tracked_connection_per_vhost)}]) of {atomic, ok} -> ok; {aborted, _} -> ok %% TODO: propagate errors From dad8511a6409816d3d8012fdb70213122bb690e4 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Thu, 18 Aug 2016 11:18:58 +0300 Subject: [PATCH 23/29] Squash warnings --- src/rabbit_connection_tracking.erl | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/rabbit_connection_tracking.erl b/src/rabbit_connection_tracking.erl index bd82b7c27e51..ab945abc2f4f 100644 --- a/src/rabbit_connection_tracking.erl +++ b/src/rabbit_connection_tracking.erl @@ -188,21 +188,21 @@ unregister_connection(ConnId = {Node, _Name}) when Node =:= node() -> -spec list() -> [rabbit_types:tracked_connection()]. list() -> - Chunks = lists:foldl( - fun (Node, Acc) -> - Tab = tracked_connection_table_name_for(Node), - Acc ++ mnesia:dirty_match_object(Tab, #tracked_connection{_ = '_'}) - end, [], rabbit_mnesia:cluster_nodes(running)). + lists:foldl( + fun (Node, Acc) -> + Tab = tracked_connection_table_name_for(Node), + Acc ++ mnesia:dirty_match_object(Tab, #tracked_connection{_ = '_'}) + end, [], rabbit_mnesia:cluster_nodes(running)). -spec list(rabbit_types:vhost()) -> [rabbit_types:tracked_connection()]. list(VHost) -> - Chunks = lists:foldl( - fun (Node, Acc) -> - Tab = tracked_connection_table_name_for(Node), - Acc ++ mnesia:dirty_match_object(Tab, #tracked_connection{vhost = VHost, _ = '_'}) - end, [], rabbit_mnesia:cluster_nodes(running)). + lists:foldl( + fun (Node, Acc) -> + Tab = tracked_connection_table_name_for(Node), + Acc ++ mnesia:dirty_match_object(Tab, #tracked_connection{vhost = VHost, _ = '_'}) + end, [], rabbit_mnesia:cluster_nodes(running)). -spec list_on_node(node()) -> [rabbit_types:tracked_connection()]. From 2527a853a27595915edcb736dbe35cd56fb067a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Thu, 25 Aug 2016 17:43:17 +0200 Subject: [PATCH 24/29] rabbit_mnesia:forget_cluster_node/2: Skip event if node if offline When the node is offline (`--offline` is set on the command line), we must skip the emission of the `node_deleted` event because the rabbit_event event handler is not setup (RabbitMQ is stopped). Otherwise, the command fails with `badarg` in gen_event. References #500. [#116521809] --- src/rabbit_mnesia.erl | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index a1cfe0948f13..ef4ab674fdce 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -297,6 +297,9 @@ update_cluster_nodes(DiscoveryNode) -> %% the last or second to last after the node we're removing to go %% down forget_cluster_node(Node, RemoveWhenOffline) -> + forget_cluster_node(Node, RemoveWhenOffline, true). + +forget_cluster_node(Node, RemoveWhenOffline, EmitNodeDeletedEvent) -> case lists:member(Node, cluster_nodes(all)) of true -> ok; false -> e(not_a_cluster_node) @@ -308,9 +311,10 @@ forget_cluster_node(Node, RemoveWhenOffline) -> {false, true} -> rabbit_log:info( "Removing node ~p from cluster~n", [Node]), case remove_node_if_mnesia_running(Node) of - ok -> - rabbit_event:notify(node_deleted, [{node, Node}]), - ok; + ok when EmitNodeDeletedEvent -> + rabbit_event:notify(node_deleted, [{node, Node}]), + ok; + ok -> ok; {error, _} = Err -> throw(Err) end end. @@ -330,7 +334,10 @@ remove_node_offline_node(Node) -> %% they are loaded. rabbit_table:force_load(), rabbit_table:wait_for_replicated(), - forget_cluster_node(Node, false), + %% We skip the 'node_deleted' event because the + %% application is stopped and thus, rabbit_event is not + %% enabled. + forget_cluster_node(Node, false, false), force_load_next_boot() after stop_mnesia() From f43f6cd6133c57aeb814b2c4e6998b1a89beb7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Thu, 25 Aug 2016 17:49:49 +0200 Subject: [PATCH 25/29] rabbit_mnesia_rename: Backup local tables only The connection tracking tables are not replicated because the table tracking connections on node A logically exists only the node A. The backup made during the rename of a node failed because it wanted to access a remote offline node to backup its connection tracking tables. Obviously it didn't work. The solution is to not backup those tables. This is correct because they are only relevant while the node is running. References #500. [#116521809] --- src/rabbit_mnesia_rename.erl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/rabbit_mnesia_rename.erl b/src/rabbit_mnesia_rename.erl index 0945e3152234..0c3e7c236600 100644 --- a/src/rabbit_mnesia_rename.erl +++ b/src/rabbit_mnesia_rename.erl @@ -124,7 +124,13 @@ prepare(Node, NodeMapList) -> take_backup(Backup) -> start_mnesia(), - ok = mnesia:backup(Backup), + %% We backup only local tables: in particular, this excludes the + %% connection tracking tables which have no local replica. + LocalTables = mnesia:system_info(local_tables), + {ok, Name, _Nodes} = mnesia:activate_checkpoint([ + {max, LocalTables} + ]), + ok = mnesia:backup_checkpoint(Name, Backup), stop_mnesia(). restore_backup(Backup) -> From d431320559960968eef716f343cc90d9dbe5e4ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Thu, 25 Aug 2016 17:53:04 +0200 Subject: [PATCH 26/29] per_vhost_connection_limit_SUITE: Sleep 500ms after connection open/close This helps in CI where the connection tracking tables may not yet be up-to-date at the time the testcase verifies them. References #500. [#116521809] --- test/per_vhost_connection_limit_SUITE.erl | 315 ++++++++++------------ 1 file changed, 139 insertions(+), 176 deletions(-) diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index 6b6293922676..593819797e82 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -22,10 +22,6 @@ -compile(export_all). --import(rabbit_ct_client_helpers, [open_unmanaged_connection/2, - open_unmanaged_connection/3]). - - all() -> [ {group, cluster_size_1}, @@ -119,39 +115,36 @@ clear_all_connection_tracking_tables(Config) -> most_basic_single_node_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), - Conn = open_unmanaged_connection(Config, 0), + [Conn] = open_connections(Config, [0]), ?assertEqual(1, count_connections_in(Config, VHost)), - rabbit_ct_client_helpers:close_connection(Conn), + close_connections([Conn]), ?assertEqual(0, count_connections_in(Config, VHost)). single_node_single_vhost_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), - Conn1 = open_unmanaged_connection(Config, 0), + [Conn1] = open_connections(Config, [0]), ?assertEqual(1, count_connections_in(Config, VHost)), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, count_connections_in(Config, VHost)), - Conn2 = open_unmanaged_connection(Config, 0), + [Conn2] = open_connections(Config, [0]), ?assertEqual(1, count_connections_in(Config, VHost)), - Conn3 = open_unmanaged_connection(Config, 0), + [Conn3] = open_connections(Config, [0]), ?assertEqual(2, count_connections_in(Config, VHost)), - Conn4 = open_unmanaged_connection(Config, 0), + [Conn4] = open_connections(Config, [0]), ?assertEqual(3, count_connections_in(Config, VHost)), - (catch exit(Conn4, please_terminate)), + kill_connections([Conn4]), ?assertEqual(2, count_connections_in(Config, VHost)), - Conn5 = open_unmanaged_connection(Config, 0), + [Conn5] = open_connections(Config, [0]), ?assertEqual(3, count_connections_in(Config, VHost)), - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn2, Conn3, Conn5]), - + close_connections([Conn2, Conn3, Conn5]), ?assertEqual(0, count_connections_in(Config, VHost)). single_node_multiple_vhosts_connection_count(Config) -> @@ -164,34 +157,31 @@ single_node_multiple_vhosts_connection_count(Config) -> ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), + [Conn1] = open_connections(Config, [{0, VHost1}]), ?assertEqual(1, count_connections_in(Config, VHost1)), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, count_connections_in(Config, VHost1)), - Conn2 = open_unmanaged_connection(Config, 0, VHost2), + [Conn2] = open_connections(Config, [{0, VHost2}]), ?assertEqual(1, count_connections_in(Config, VHost2)), - Conn3 = open_unmanaged_connection(Config, 0, VHost1), + [Conn3] = open_connections(Config, [{0, VHost1}]), ?assertEqual(1, count_connections_in(Config, VHost1)), ?assertEqual(1, count_connections_in(Config, VHost2)), - Conn4 = open_unmanaged_connection(Config, 0, VHost1), + [Conn4] = open_connections(Config, [{0, VHost1}]), ?assertEqual(2, count_connections_in(Config, VHost1)), - (catch exit(Conn4, please_terminate)), + kill_connections([Conn4]), ?assertEqual(1, count_connections_in(Config, VHost1)), - Conn5 = open_unmanaged_connection(Config, 0, VHost2), + [Conn5] = open_connections(Config, [{0, VHost2}]), ?assertEqual(2, count_connections_in(Config, VHost2)), - Conn6 = open_unmanaged_connection(Config, 0, VHost2), + [Conn6] = open_connections(Config, [{0, VHost2}]), ?assertEqual(3, count_connections_in(Config, VHost2)), - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn2, Conn3, Conn5, Conn6]), - + close_connections([Conn2, Conn3, Conn5, Conn6]), ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), @@ -208,31 +198,27 @@ single_node_list_in_vhost(Config) -> ?assertEqual(0, length(connections_in(Config, VHost1))), ?assertEqual(0, length(connections_in(Config, VHost2))), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), + [Conn1] = open_connections(Config, [{0, VHost1}]), [#tracked_connection{vhost = VHost1}] = connections_in(Config, VHost1), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, length(connections_in(Config, VHost1))), - Conn2 = open_unmanaged_connection(Config, 0, VHost2), + [Conn2] = open_connections(Config, [{0, VHost2}]), [#tracked_connection{vhost = VHost2}] = connections_in(Config, VHost2), - Conn3 = open_unmanaged_connection(Config, 0, VHost1), + [Conn3] = open_connections(Config, [{0, VHost1}]), [#tracked_connection{vhost = VHost1}] = connections_in(Config, VHost1), - Conn4 = open_unmanaged_connection(Config, 0, VHost1), - (catch exit(Conn4, please_terminate)), + [Conn4] = open_connections(Config, [{0, VHost1}]), + kill_connections([Conn4]), [#tracked_connection{vhost = VHost1}] = connections_in(Config, VHost1), - Conn5 = open_unmanaged_connection(Config, 0, VHost2), - Conn6 = open_unmanaged_connection(Config, 0, VHost2), + [Conn5, Conn6] = open_connections(Config, [{0, VHost2}, {0, VHost2}]), [<<"vhost1">>, <<"vhost2">>] = lists:usort(lists:map(fun (#tracked_connection{vhost = V}) -> V end, all_connections(Config))), - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn2, Conn3, Conn5, Conn6]), - + close_connections([Conn2, Conn3, Conn5, Conn6]), ?assertEqual(0, length(all_connections(Config))), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1), @@ -241,49 +227,43 @@ single_node_list_in_vhost(Config) -> most_basic_cluster_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), - Conn1 = open_unmanaged_connection(Config, 0), + [Conn1] = open_connections(Config, [0]), ?assertEqual(1, count_connections_in(Config, VHost)), - Conn2 = open_unmanaged_connection(Config, 1), + [Conn2] = open_connections(Config, [1]), ?assertEqual(2, count_connections_in(Config, VHost)), - Conn3 = open_unmanaged_connection(Config, 1), + [Conn3] = open_connections(Config, [1]), ?assertEqual(3, count_connections_in(Config, VHost)), - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3]), - + close_connections([Conn1, Conn2, Conn3]), ?assertEqual(0, count_connections_in(Config, VHost)). cluster_single_vhost_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), - Conn1 = open_unmanaged_connection(Config, 0), + [Conn1] = open_connections(Config, [0]), ?assertEqual(1, count_connections_in(Config, VHost)), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, count_connections_in(Config, VHost)), - Conn2 = open_unmanaged_connection(Config, 1), + [Conn2] = open_connections(Config, [1]), ?assertEqual(1, count_connections_in(Config, VHost)), - Conn3 = open_unmanaged_connection(Config, 0), + [Conn3] = open_connections(Config, [0]), ?assertEqual(2, count_connections_in(Config, VHost)), - Conn4 = open_unmanaged_connection(Config, 1), + [Conn4] = open_connections(Config, [1]), ?assertEqual(3, count_connections_in(Config, VHost)), - (catch exit(Conn4, please_terminate)), + kill_connections([Conn4]), ?assertEqual(2, count_connections_in(Config, VHost)), - Conn5 = open_unmanaged_connection(Config, 1), + [Conn5] = open_connections(Config, [1]), ?assertEqual(3, count_connections_in(Config, VHost)), - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn2, Conn3, Conn5]), - + close_connections([Conn2, Conn3, Conn5]), ?assertEqual(0, count_connections_in(Config, VHost)). cluster_multiple_vhosts_connection_count(Config) -> @@ -296,34 +276,31 @@ cluster_multiple_vhosts_connection_count(Config) -> ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), + [Conn1] = open_connections(Config, [{0, VHost1}]), ?assertEqual(1, count_connections_in(Config, VHost1)), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, count_connections_in(Config, VHost1)), - Conn2 = open_unmanaged_connection(Config, 1, VHost2), + [Conn2] = open_connections(Config, [{1, VHost2}]), ?assertEqual(1, count_connections_in(Config, VHost2)), - Conn3 = open_unmanaged_connection(Config, 1, VHost1), + [Conn3] = open_connections(Config, [{1, VHost1}]), ?assertEqual(1, count_connections_in(Config, VHost1)), ?assertEqual(1, count_connections_in(Config, VHost2)), - Conn4 = open_unmanaged_connection(Config, 0, VHost1), + [Conn4] = open_connections(Config, [{0, VHost1}]), ?assertEqual(2, count_connections_in(Config, VHost1)), - (catch exit(Conn4, please_terminate)), + kill_connections([Conn4]), ?assertEqual(1, count_connections_in(Config, VHost1)), - Conn5 = open_unmanaged_connection(Config, 1, VHost2), + [Conn5] = open_connections(Config, [{1, VHost2}]), ?assertEqual(2, count_connections_in(Config, VHost2)), - Conn6 = open_unmanaged_connection(Config, 0, VHost2), + [Conn6] = open_connections(Config, [{0, VHost2}]), ?assertEqual(3, count_connections_in(Config, VHost2)), - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn2, Conn3, Conn5, Conn6]), - + close_connections([Conn2, Conn3, Conn5, Conn6]), ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), @@ -334,30 +311,27 @@ cluster_node_restart_connection_count(Config) -> VHost = <<"/">>, ?assertEqual(0, count_connections_in(Config, VHost)), - Conn1 = open_unmanaged_connection(Config, 0), + [Conn1] = open_connections(Config, [0]), ?assertEqual(1, count_connections_in(Config, VHost)), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, count_connections_in(Config, VHost)), - Conn2 = open_unmanaged_connection(Config, 1), + [Conn2] = open_connections(Config, [1]), ?assertEqual(1, count_connections_in(Config, VHost)), - Conn3 = open_unmanaged_connection(Config, 0), + [Conn3] = open_connections(Config, [0]), ?assertEqual(2, count_connections_in(Config, VHost)), - Conn4 = open_unmanaged_connection(Config, 1), + [Conn4] = open_connections(Config, [1]), ?assertEqual(3, count_connections_in(Config, VHost)), - Conn5 = open_unmanaged_connection(Config, 1), + [Conn5] = open_connections(Config, [1]), ?assertEqual(4, count_connections_in(Config, VHost)), rabbit_ct_broker_helpers:restart_broker(Config, 1), ?assertEqual(1, count_connections_in(Config, VHost)), - lists:foreach(fun (C) -> - (catch rabbit_ct_client_helpers:close_connection(C)) - end, [Conn2, Conn3, Conn4, Conn5]), - + close_connections([Conn2, Conn3, Conn4, Conn5]), ?assertEqual(0, count_connections_in(Config, VHost)). cluster_node_list_on_node(Config) -> @@ -366,24 +340,24 @@ cluster_node_list_on_node(Config) -> ?assertEqual(0, length(all_connections(Config))), ?assertEqual(0, length(connections_on_node(Config, 0))), - Conn1 = open_unmanaged_connection(Config, 0), + [Conn1] = open_connections(Config, [0]), [#tracked_connection{node = A}] = connections_on_node(Config, 0), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, length(connections_on_node(Config, 0))), - _Conn2 = open_unmanaged_connection(Config, 1), + [_Conn2] = open_connections(Config, [1]), [#tracked_connection{node = B}] = connections_on_node(Config, 1), - Conn3 = open_unmanaged_connection(Config, 0), + [Conn3] = open_connections(Config, [0]), ?assertEqual(1, length(connections_on_node(Config, 0))), - Conn4 = open_unmanaged_connection(Config, 1), + [Conn4] = open_connections(Config, [1]), ?assertEqual(2, length(connections_on_node(Config, 1))), - (catch exit(Conn4, please_terminate)), + kill_connections([Conn4]), ?assertEqual(1, length(connections_on_node(Config, 1))), - Conn5 = open_unmanaged_connection(Config, 0), + [Conn5] = open_connections(Config, [0]), ?assertEqual(2, length(connections_on_node(Config, 0))), rabbit_ct_broker_helpers:stop_broker(Config, 1), @@ -392,11 +366,7 @@ cluster_node_list_on_node(Config) -> ?assertEqual(2, length(all_connections(Config))), ?assertEqual(0, length(connections_on_node(Config, 0, B))), - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn3, Conn5]), - - timer:sleep(100), + close_connections([Conn3, Conn5]), ?assertEqual(0, length(all_connections(Config, 0))), rabbit_ct_broker_helpers:start_broker(Config, 1). @@ -411,9 +381,7 @@ single_node_single_vhost_limit_with(Config, WatermarkLimit) -> ?assertEqual(0, count_connections_in(Config, VHost)), - Conn1 = open_unmanaged_connection(Config, 0), - Conn2 = open_unmanaged_connection(Config, 0), - Conn3 = open_unmanaged_connection(Config, 0), + [Conn1, Conn2, Conn3] = open_connections(Config, [0, 0, 0]), %% we've crossed the limit expect_that_client_connection_is_rejected(Config, 0), @@ -421,14 +389,11 @@ single_node_single_vhost_limit_with(Config, WatermarkLimit) -> expect_that_client_connection_is_rejected(Config, 0), set_vhost_connection_limit(Config, VHost, WatermarkLimit), - Conn4 = open_unmanaged_connection(Config, 0), - Conn5 = open_unmanaged_connection(Config, 0), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4, Conn5]), + [Conn4, Conn5] = open_connections(Config, [0, 0]), + close_connections([Conn1, Conn2, Conn3, Conn4, Conn5]), ?assertEqual(0, count_connections_in(Config, VHost)), + set_vhost_connection_limit(Config, VHost, -1). single_node_single_vhost_zero_limit(Config) -> @@ -443,13 +408,9 @@ single_node_single_vhost_zero_limit(Config) -> expect_that_client_connection_is_rejected(Config), set_vhost_connection_limit(Config, VHost, -1), - Conn1 = open_unmanaged_connection(Config, 0), - Conn2 = open_unmanaged_connection(Config, 0), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2]), + [Conn1, Conn2] = open_connections(Config, [0, 0]), + close_connections([Conn1, Conn2]), ?assertEqual(0, count_connections_in(Config, VHost)). @@ -466,31 +427,30 @@ single_node_multiple_vhosts_limit(Config) -> ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), - Conn2 = open_unmanaged_connection(Config, 0, VHost1), - Conn3 = open_unmanaged_connection(Config, 0, VHost2), - Conn4 = open_unmanaged_connection(Config, 0, VHost2), + [Conn1, Conn2, Conn3, Conn4] = open_connections(Config, [ + {0, VHost1}, + {0, VHost1}, + {0, VHost2}, + {0, VHost2}]), %% we've crossed the limit expect_that_client_connection_is_rejected(Config, 0, VHost1), expect_that_client_connection_is_rejected(Config, 0, VHost2), - Conn5 = open_unmanaged_connection(Config, 0), + [Conn5] = open_connections(Config, [0]), set_vhost_connection_limit(Config, VHost1, 5), set_vhost_connection_limit(Config, VHost2, -10), - Conn6 = open_unmanaged_connection(Config, 0, VHost1), - Conn7 = open_unmanaged_connection(Config, 0, VHost1), - Conn8 = open_unmanaged_connection(Config, 0, VHost1), - Conn9 = open_unmanaged_connection(Config, 0, VHost2), - Conn10 = open_unmanaged_connection(Config, 0, VHost2), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4, Conn5, - Conn6, Conn7, Conn8, Conn9, Conn10]), + [Conn6, Conn7, Conn8, Conn9, Conn10] = open_connections(Config, [ + {0, VHost1}, + {0, VHost1}, + {0, VHost1}, + {0, VHost2}, + {0, VHost2}]), + close_connections([Conn1, Conn2, Conn3, Conn4, Conn5, + Conn6, Conn7, Conn8, Conn9, Conn10]), ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), @@ -520,13 +480,9 @@ single_node_multiple_vhosts_zero_limit(Config) -> expect_that_client_connection_is_rejected(Config, 0, VHost1), set_vhost_connection_limit(Config, VHost1, -1), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), - Conn2 = open_unmanaged_connection(Config, 0, VHost1), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2]), + [Conn1, Conn2] = open_connections(Config, [{0, VHost1}, {0, VHost1}]), + close_connections([Conn1, Conn2]), ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), @@ -534,7 +490,6 @@ single_node_multiple_vhosts_zero_limit(Config) -> set_vhost_connection_limit(Config, VHost2, -1). - cluster_single_vhost_limit(Config) -> VHost = <<"/">>, set_vhost_connection_limit(Config, VHost, 2), @@ -542,10 +497,7 @@ cluster_single_vhost_limit(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)), %% here connections are opened to different nodes - Conn1 = open_unmanaged_connection(Config, 0, VHost), - Conn2 = open_unmanaged_connection(Config, 1, VHost), - %% give tracked connection rows some time to propagate in both directions - timer:sleep(200), + [Conn1, Conn2] = open_connections(Config, [{0, VHost}, {1, VHost}]), %% we've crossed the limit expect_that_client_connection_is_rejected(Config, 0, VHost), @@ -553,13 +505,9 @@ cluster_single_vhost_limit(Config) -> set_vhost_connection_limit(Config, VHost, 5), - Conn3 = open_unmanaged_connection(Config, 0, VHost), - Conn4 = open_unmanaged_connection(Config, 0, VHost), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4]), + [Conn3, Conn4] = open_connections(Config, [{0, VHost}, {0, VHost}]), + close_connections([Conn1, Conn2, Conn3, Conn4]), ?assertEqual(0, count_connections_in(Config, VHost)), set_vhost_connection_limit(Config, VHost, -1). @@ -571,26 +519,21 @@ cluster_single_vhost_limit2(Config) -> ?assertEqual(0, count_connections_in(Config, VHost)), %% here a limit is reached on one node first - Conn1 = open_unmanaged_connection(Config, 0, VHost), - Conn2 = open_unmanaged_connection(Config, 0, VHost), + [Conn1, Conn2] = open_connections(Config, [{0, VHost}, {0, VHost}]), %% we've crossed the limit expect_that_client_connection_is_rejected(Config, 0, VHost), - - timer:sleep(200), expect_that_client_connection_is_rejected(Config, 1, VHost), set_vhost_connection_limit(Config, VHost, 5), - Conn3 = open_unmanaged_connection(Config, 1, VHost), - Conn4 = open_unmanaged_connection(Config, 1, VHost), - Conn5 = open_unmanaged_connection(Config, 1, VHost), - {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4, Conn5]), + [Conn3, Conn4, Conn5, {error, not_allowed}] = open_connections(Config, [ + {1, VHost}, + {1, VHost}, + {1, VHost}, + {1, VHost}]), + close_connections([Conn1, Conn2, Conn3, Conn4, Conn5]), ?assertEqual(0, count_connections_in(Config, VHost)), set_vhost_connection_limit(Config, VHost, -1). @@ -608,15 +551,9 @@ cluster_single_vhost_zero_limit(Config) -> expect_that_client_connection_is_rejected(Config, 0), set_vhost_connection_limit(Config, VHost, -1), - Conn1 = open_unmanaged_connection(Config, 0), - Conn2 = open_unmanaged_connection(Config, 1), - Conn3 = open_unmanaged_connection(Config, 0), - Conn4 = open_unmanaged_connection(Config, 1), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4]), + [Conn1, Conn2, Conn3, Conn4] = open_connections(Config, [0, 1, 0, 1]), + close_connections([Conn1, Conn2, Conn3, Conn4]), ?assertEqual(0, count_connections_in(Config, VHost)), set_vhost_connection_limit(Config, VHost, -1). @@ -644,15 +581,13 @@ cluster_multiple_vhosts_zero_limit(Config) -> set_vhost_connection_limit(Config, VHost1, -1), set_vhost_connection_limit(Config, VHost2, -1), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), - Conn2 = open_unmanaged_connection(Config, 0, VHost2), - Conn3 = open_unmanaged_connection(Config, 1, VHost1), - Conn4 = open_unmanaged_connection(Config, 1, VHost2), - - lists:foreach(fun (C) -> - rabbit_ct_client_helpers:close_connection(C) - end, [Conn1, Conn2, Conn3, Conn4]), + [Conn1, Conn2, Conn3, Conn4] = open_connections(Config, [ + {0, VHost1}, + {0, VHost2}, + {1, VHost1}, + {1, VHost2}]), + close_connections([Conn1, Conn2, Conn3, Conn4]), ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), @@ -670,17 +605,17 @@ single_node_vhost_deletion_forces_connection_closure(Config) -> ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), + [Conn1] = open_connections(Config, [{0, VHost1}]), ?assertEqual(1, count_connections_in(Config, VHost1)), - _Conn2 = open_unmanaged_connection(Config, 0, VHost2), + [_Conn2] = open_connections(Config, [{0, VHost2}]), ?assertEqual(1, count_connections_in(Config, VHost2)), rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), timer:sleep(200), ?assertEqual(0, count_connections_in(Config, VHost2)), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, count_connections_in(Config, VHost1)), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1). @@ -695,17 +630,17 @@ cluster_vhost_deletion_forces_connection_closure(Config) -> ?assertEqual(0, count_connections_in(Config, VHost1)), ?assertEqual(0, count_connections_in(Config, VHost2)), - Conn1 = open_unmanaged_connection(Config, 0, VHost1), + [Conn1] = open_connections(Config, [{0, VHost1}]), ?assertEqual(1, count_connections_in(Config, VHost1)), - _Conn2 = open_unmanaged_connection(Config, 1, VHost2), + [_Conn2] = open_connections(Config, [{1, VHost2}]), ?assertEqual(1, count_connections_in(Config, VHost2)), rabbit_ct_broker_helpers:delete_vhost(Config, VHost2), timer:sleep(200), ?assertEqual(0, count_connections_in(Config, VHost2)), - rabbit_ct_client_helpers:close_connection(Conn1), + close_connections([Conn1]), ?assertEqual(0, count_connections_in(Config, VHost1)), rabbit_ct_broker_helpers:delete_vhost(Config, VHost1). @@ -714,9 +649,35 @@ cluster_vhost_deletion_forces_connection_closure(Config) -> %% Helpers %% ------------------------------------------------------------------- +open_connections(Config, NodesAndVHosts) -> + Conns = lists:map(fun + ({Node, VHost}) -> + rabbit_ct_client_helpers:open_unmanaged_connection(Config, Node, + VHost); + (Node) -> + rabbit_ct_client_helpers:open_unmanaged_connection(Config, Node) + end, NodesAndVHosts), + timer:sleep(500), + Conns. + +close_connections(Conns) -> + lists:foreach(fun + (Conn) -> + rabbit_ct_client_helpers:close_connection(Conn) + end, Conns), + timer:sleep(500). + +kill_connections(Conns) -> + lists:foreach(fun + (Conn) -> + (catch exit(Conn, please_terminate)) + end, Conns), + timer:sleep(500). + count_connections_in(Config, VHost) -> count_connections_in(Config, VHost, 0). count_connections_in(Config, VHost, NodeIndex) -> + timer:sleep(200), rabbit_ct_broker_helpers:rpc(Config, NodeIndex, rabbit_connection_tracking, count_connections_in, [VHost]). @@ -770,7 +731,9 @@ expect_that_client_connection_is_rejected(Config) -> expect_that_client_connection_is_rejected(Config, 0). expect_that_client_connection_is_rejected(Config, NodeIndex) -> - {error, not_allowed} = open_unmanaged_connection(Config, NodeIndex). + {error, not_allowed} = + rabbit_ct_client_helpers:open_unmanaged_connection(Config, NodeIndex). expect_that_client_connection_is_rejected(Config, NodeIndex, VHost) -> - {error, not_allowed} = open_unmanaged_connection(Config, NodeIndex, VHost). + {error, not_allowed} = + rabbit_ct_client_helpers:open_unmanaged_connection(Config, NodeIndex, VHost). From 99f75622beaefa7e4acab9f11befef7067237215 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Thu, 25 Aug 2016 17:55:03 +0200 Subject: [PATCH 27/29] cluster_rename_SUITE: Lower the testsuite timeout While here, be more informative when a node rename fails. --- test/cluster_rename_SUITE.erl | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/cluster_rename_SUITE.erl b/test/cluster_rename_SUITE.erl index 8ce29a6695e7..9521e04eb534 100644 --- a/test/cluster_rename_SUITE.erl +++ b/test/cluster_rename_SUITE.erl @@ -43,6 +43,12 @@ groups() -> ]} ]. +suite() -> + [ + %% If a test hangs, no need to wait for 30 minutes. + {timetrap, {minutes, 8}} + ]. + %% ------------------------------------------------------------------- %% Testsuite setup/teardown. %% ------------------------------------------------------------------- @@ -245,7 +251,7 @@ rename_node(Config, Nodename, Map) -> Config1. rename_node_fail(Config, Nodename, Map) -> - error = do_rename_node(Config, Nodename, Map), + {error, _, _} = do_rename_node(Config, Nodename, Map), ok. do_rename_node(Config, Nodename, Map) -> @@ -265,8 +271,8 @@ do_rename_node(Config, Nodename, Map) -> {ok, _} -> Config1 = update_config_after_rename(Config, Map1), {ok, Config1}; - {error, _, _} -> - error + {error, _, _} = Error -> + Error end. update_config_after_rename(Config, [Old, New | Rest]) -> From a5cdde115e5373cae4c8a5577549308167540e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Fri, 26 Aug 2016 12:34:59 +0200 Subject: [PATCH 28/29] Remove dead code left from a previous incarnation of the patch References #500. [#116521809] --- src/rabbit_mnesia.erl | 4 +--- src/rabbit_table.erl | 32 ++++---------------------------- src/rabbit_upgrade_functions.erl | 9 --------- 3 files changed, 5 insertions(+), 40 deletions(-) diff --git a/src/rabbit_mnesia.erl b/src/rabbit_mnesia.erl index ef4ab674fdce..43dd2c3bb809 100644 --- a/src/rabbit_mnesia.erl +++ b/src/rabbit_mnesia.erl @@ -477,14 +477,12 @@ init_db(ClusterNodes, NodeType, CheckOtherNodes) -> {[], true, disc} -> %% First disc node up maybe_force_load(), - ok = rabbit_table:ensure_secondary_indices(), ok; {[_ | _], _, _} -> %% Subsequent node in cluster, catch up maybe_force_load(), ok = rabbit_table:wait_for_replicated(), - ok = rabbit_table:create_local_copy(NodeType), - ok = rabbit_table:ensure_secondary_indices() + ok = rabbit_table:create_local_copy(NodeType) end, ensure_schema_integrity(), rabbit_node_monitor:update_cluster_status(), diff --git a/src/rabbit_table.erl b/src/rabbit_table.erl index 60996c153912..390909696499 100644 --- a/src/rabbit_table.erl +++ b/src/rabbit_table.erl @@ -18,8 +18,7 @@ -export([create/0, create_local_copy/1, wait_for_replicated/0, wait/1, force_load/0, is_present/0, is_empty/0, needs_default_data/0, - check_schema_integrity/0, clear_ram_only_tables/0, wait_timeout/0, - ensure_secondary_indices/0, ensure_secondary_indices/2]). + check_schema_integrity/0, clear_ram_only_tables/0, wait_timeout/0]). -include("rabbit.hrl"). @@ -51,7 +50,6 @@ create() -> Tab, TabDef1, Reason}}) end end, definitions()), - ok = rabbit_table:ensure_secondary_indices(), ok. %% The sequence in which we delete the schema and then the other @@ -65,13 +63,6 @@ create_local_copy(ram) -> create_local_copies(ram), create_local_copy(schema, ram_copies). -ensure_secondary_indices() -> - ensure_secondary_indices(rabbit_tracked_connection, [vhost, username]), - ok. - -ensure_secondary_indices(Tab, Fields) -> - [mnesia:add_table_index(Tab, Field) || Field <- Fields]. - wait_for_replicated() -> wait([Tab || {Tab, TabDef} <- definitions(), not lists:member({local_content, true}, TabDef)]). @@ -306,24 +297,9 @@ definitions() -> {rabbit_queue, [{record_name, amqqueue}, {attributes, record_info(fields, amqqueue)}, - {match, #amqqueue{name = queue_name_match(), _='_'}}]}, - - %% Used to track connections across virtual hosts - %% e.g. so that limits can be enforced. - %% - %% All data in this table is transient. - {rabbit_tracked_connection, - [{record_name, tracked_connection}, - {attributes, record_info(fields, tracked_connection)}, - {match, #tracked_connection{_ = '_'}}]}, - - {rabbit_tracked_connection_per_vhost, - [{record_name, tracked_connection_per_vhost}, - {attributes, record_info(fields, tracked_connection_per_vhost)}, - {match, #tracked_connection_per_vhost{_ = '_'}}]} - - ] ++ gm:table_definitions() - ++ mirrored_supervisor:table_definitions(). + {match, #amqqueue{name = queue_name_match(), _='_'}}]}] + ++ gm:table_definitions() + ++ mirrored_supervisor:table_definitions(). binding_match() -> #binding{source = exchange_name_match(), diff --git a/src/rabbit_upgrade_functions.erl b/src/rabbit_upgrade_functions.erl index 305a1ece93fb..3c6f2a4f1f72 100644 --- a/src/rabbit_upgrade_functions.erl +++ b/src/rabbit_upgrade_functions.erl @@ -524,11 +524,6 @@ create(Tab, TabDef) -> {atomic, ok} = mnesia:create_table(Tab, TabDef), ok. -create(Tab, TabDef, SecondaryIndices) -> - {atomic, ok} = mnesia:create_table(Tab, TabDef), - [mnesia:add_table_index(Tab, Idx) || Idx <- SecondaryIndices], - ok. - %% Dumb replacement for rabbit_exchange:declare that does not require %% the exchange type registry or worker pool to be running by dint of %% not validating anything and assuming the exchange type does not @@ -537,7 +532,3 @@ create(Tab, TabDef, SecondaryIndices) -> declare_exchange(XName, Type) -> X = {exchange, XName, Type, true, false, false, []}, ok = mnesia:dirty_write(rabbit_durable_exchange, X). - -add_indices(Tab, FieldList) -> - [mnesia:add_table_index(Tab, Field) || Field <- FieldList], - ok. From a660b7009a61d1e05c096b518a961720669c0bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Fri, 26 Aug 2016 15:09:07 +0200 Subject: [PATCH 29/29] per_vhost_connection_limit_SUITE: Add a test around cluster rename This makes sure the limits are still good after a node was renamed and thus, the tracking tables were recreated with a new name too. While here, do some cleanup in the init/end functions in both the two per-vhost limit testsuite. References #500. [#116521809] --- test/per_vhost_connection_limit_SUITE.erl | 74 ++++++++++++++++--- ...host_connection_limit_partitions_SUITE.erl | 18 +++-- 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl index 593819797e82..4fae129a35be 100644 --- a/test/per_vhost_connection_limit_SUITE.erl +++ b/test/per_vhost_connection_limit_SUITE.erl @@ -52,9 +52,18 @@ groups() -> cluster_single_vhost_zero_limit, cluster_multiple_vhosts_zero_limit, cluster_vhost_deletion_forces_connection_closure + ]}, + {cluster_rename, [], [ + vhost_limit_after_node_renamed ]} ]. +suite() -> + [ + %% If a test hangs, no need to wait for 30 minutes. + {timetrap, {minutes, 8}} + ]. + %% see partitions_SUITE -define(DELAY, 9000). @@ -74,31 +83,52 @@ end_per_suite(Config) -> init_per_group(cluster_size_1, Config) -> init_per_multinode_group(cluster_size_1, Config, 1); init_per_group(cluster_size_2, Config) -> - init_per_multinode_group(cluster_size_2, Config, 2). + init_per_multinode_group(cluster_size_2, Config, 2); +init_per_group(cluster_rename, Config) -> + init_per_multinode_group(cluster_rename, Config, 2). -init_per_multinode_group(_GroupName, Config, NodeCount) -> +init_per_multinode_group(Group, Config, NodeCount) -> Suffix = rabbit_ct_helpers:testcase_absname(Config, "", "-"), Config1 = rabbit_ct_helpers:set_config(Config, [ {rmq_nodes_count, NodeCount}, {rmq_nodename_suffix, Suffix} ]), - rabbit_ct_helpers:run_steps(Config1, - rabbit_ct_broker_helpers:setup_steps() ++ - rabbit_ct_client_helpers:setup_steps()). - + case Group of + cluster_rename -> + % The broker is managed by {init,end}_per_testcase(). + Config1; + _ -> + rabbit_ct_helpers:run_steps(Config1, + rabbit_ct_broker_helpers:setup_steps() ++ + rabbit_ct_client_helpers:setup_steps()) + end. + +end_per_group(cluster_rename, Config) -> + % The broker is managed by {init,end}_per_testcase(). + Config; end_per_group(_Group, Config) -> rabbit_ct_helpers:run_steps(Config, rabbit_ct_client_helpers:teardown_steps() ++ rabbit_ct_broker_helpers:teardown_steps()). +init_per_testcase(vhost_limit_after_node_renamed = Testcase, Config) -> + rabbit_ct_helpers:testcase_started(Config, Testcase), + rabbit_ct_helpers:run_steps(Config, + rabbit_ct_broker_helpers:setup_steps() ++ + rabbit_ct_client_helpers:setup_steps()); init_per_testcase(Testcase, Config) -> + rabbit_ct_helpers:testcase_started(Config, Testcase), clear_all_connection_tracking_tables(Config), - rabbit_ct_client_helpers:setup_steps(), - rabbit_ct_helpers:testcase_started(Config, Testcase). + Config. +end_per_testcase(vhost_limit_after_node_renamed = Testcase, Config) -> + Config1 = ?config(save_config, Config), + rabbit_ct_helpers:run_steps(Config1, + rabbit_ct_client_helpers:teardown_steps() ++ + rabbit_ct_broker_helpers:teardown_steps()), + rabbit_ct_helpers:testcase_finished(Config1, Testcase); end_per_testcase(Testcase, Config) -> clear_all_connection_tracking_tables(Config), - rabbit_ct_client_helpers:teardown_steps(), rabbit_ct_helpers:testcase_finished(Config, Testcase). clear_all_connection_tracking_tables(Config) -> @@ -645,6 +675,32 @@ cluster_vhost_deletion_forces_connection_closure(Config) -> rabbit_ct_broker_helpers:delete_vhost(Config, VHost1). +vhost_limit_after_node_renamed(Config) -> + A = rabbit_ct_broker_helpers:get_node_config(Config, 0, nodename), + + VHost = <<"/renaming_node">>, + set_up_vhost(Config, VHost), + set_vhost_connection_limit(Config, VHost, 2), + + ?assertEqual(0, count_connections_in(Config, VHost)), + + [Conn1, Conn2, {error, not_allowed}] = open_connections(Config, + [{0, VHost}, {1, VHost}, {0, VHost}]), + ?assertEqual(2, count_connections_in(Config, VHost)), + close_connections([Conn1, Conn2]), + + Config1 = cluster_rename_SUITE:stop_rename_start(Config, A, [A, 'new-A']), + + ?assertEqual(0, count_connections_in(Config1, VHost)), + + [Conn3, Conn4, {error, not_allowed}] = open_connections(Config, + [{0, VHost}, {1, VHost}, {0, VHost}]), + ?assertEqual(2, count_connections_in(Config1, VHost)), + close_connections([Conn3, Conn4]), + + set_vhost_connection_limit(Config1, VHost, -1), + {save_config, Config1}. + %% ------------------------------------------------------------------- %% Helpers %% ------------------------------------------------------------------- diff --git a/test/per_vhost_connection_limit_partitions_SUITE.erl b/test/per_vhost_connection_limit_partitions_SUITE.erl index c7fae3a77cf2..051f3f13b758 100644 --- a/test/per_vhost_connection_limit_partitions_SUITE.erl +++ b/test/per_vhost_connection_limit_partitions_SUITE.erl @@ -38,6 +38,12 @@ groups() -> ]} ]. +suite() -> + [ + %% If a test hangs, no need to wait for 30 minutes. + {timetrap, {minutes, 8}} + ]. + %% see partitions_SUITE -define(DELAY, 12000). @@ -54,11 +60,11 @@ init_per_suite(Config) -> end_per_suite(Config) -> rabbit_ct_helpers:run_teardown_steps(Config). -init_per_group(net_ticktime_1 = GroupName, Config) -> +init_per_group(net_ticktime_1 = Group, Config) -> Config1 = rabbit_ct_helpers:set_config(Config, [{net_ticktime, 1}]), - init_per_multinode_group(GroupName, Config1, 3). + init_per_multinode_group(Group, Config1, 3). -init_per_multinode_group(_GroupName, Config, NodeCount) -> +init_per_multinode_group(_Group, Config, NodeCount) -> Suffix = rabbit_ct_helpers:testcase_absname(Config, "", "-"), Config1 = rabbit_ct_helpers:set_config(Config, [ {rmq_nodes_count, NodeCount}, @@ -66,21 +72,21 @@ init_per_multinode_group(_GroupName, Config, NodeCount) -> {rmq_nodes_clustered, false} ]), rabbit_ct_helpers:run_steps(Config1, - rabbit_ct_broker_helpers:setup_steps() ++ [ + rabbit_ct_broker_helpers:setup_steps() ++ + rabbit_ct_client_helpers:setup_steps() ++ [ fun rabbit_ct_broker_helpers:enable_dist_proxy/1, fun rabbit_ct_broker_helpers:cluster_nodes/1 ]). end_per_group(_Group, Config) -> rabbit_ct_helpers:run_steps(Config, + rabbit_ct_client_helpers:teardown_steps() ++ rabbit_ct_broker_helpers:teardown_steps()). init_per_testcase(Testcase, Config) -> - rabbit_ct_client_helpers:setup_steps(), rabbit_ct_helpers:testcase_started(Config, Testcase). end_per_testcase(Testcase, Config) -> - rabbit_ct_client_helpers:teardown_steps(), rabbit_ct_helpers:testcase_finished(Config, Testcase). %% -------------------------------------------------------------------