From 308d5156f76e0f21e25ca1382acbd634e9658463 Mon Sep 17 00:00:00 2001 From: Simon Unge Date: Mon, 13 May 2024 17:54:00 +0000 Subject: [PATCH 1/5] Move the check to be called on all queue types (cherry picked from commit 23a32e18e7d04e5d6873f528399047e2a7a0ec40) --- deps/rabbit/src/rabbit_channel.erl | 10 ---------- deps/rabbit/src/rabbit_queue_type.erl | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/deps/rabbit/src/rabbit_channel.erl b/deps/rabbit/src/rabbit_channel.erl index c986594ffb9d..5bbbe8d9f0a4 100644 --- a/deps/rabbit/src/rabbit_channel.erl +++ b/deps/rabbit/src/rabbit_channel.erl @@ -1056,15 +1056,6 @@ check_msg_size(Content, MaxMessageSize, GCThreshold) -> _ -> ok end. -check_vhost_queue_limit(#resource{name = QueueName}, VHost) -> - case rabbit_vhost_limit:is_over_queue_limit(VHost) of - false -> ok; - {true, Limit} -> rabbit_misc:precondition_failed("cannot declare queue '~ts': " - "queue limit in vhost '~ts' (~tp) is reached", - [QueueName, VHost, Limit]) - - end. - qbin_to_resource(QueueNameBin, VHostPath) -> name_to_resource(queue, QueueNameBin, VHostPath). @@ -2525,7 +2516,6 @@ handle_method(#'queue.declare'{queue = QueueNameBin, {ok, QueueName, MessageCount, ConsumerCount}; {error, not_found} -> %% enforce the limit for newly declared queues only - check_vhost_queue_limit(QueueName, VHostPath), DlxKey = <<"x-dead-letter-exchange">>, case rabbit_misc:r_arg(VHostPath, exchange, Args, DlxKey) of undefined -> diff --git a/deps/rabbit/src/rabbit_queue_type.erl b/deps/rabbit/src/rabbit_queue_type.erl index 61f67f738c48..cfbf1ed7c5dc 100644 --- a/deps/rabbit/src/rabbit_queue_type.erl +++ b/deps/rabbit/src/rabbit_queue_type.erl @@ -282,6 +282,7 @@ is_compatible(Type, Durable, Exclusive, AutoDelete) -> declare(Q0, Node) -> Q = rabbit_queue_decorator:set(rabbit_policy:set(Q0)), Mod = amqqueue:get_type(Q), + ok = check_vhost_queue_limit(Q), Mod:declare(Q, Node). -spec delete(amqqueue:amqqueue(), boolean(), @@ -730,3 +731,13 @@ known_queue_type_names() -> {QueueTypes, _} = lists:unzip(Registered), QTypeBins = lists:map(fun(X) -> atom_to_binary(X) end, QueueTypes), ?KNOWN_QUEUE_TYPES ++ QTypeBins. + +check_vhost_queue_limit(Q) -> + #resource{name = QueueName} = amqqueue:get_name(Q), + VHost = amqqueue:get_vhost(Q), + case rabbit_vhost_limit:is_over_queue_limit(VHost) of + false -> ok; + {true, Limit} -> rabbit_misc:precondition_failed("cannot declare queue '~ts': " + "queue limit in vhost '~ts' (~tp) is reached", + [QueueName, VHost, Limit]) + end. From 23dafd19496eaf8616414eff69f79684ef325f7a Mon Sep 17 00:00:00 2001 From: Simon Unge Date: Tue, 14 May 2024 16:09:28 +0000 Subject: [PATCH 2/5] Instead of throwing an error, return protocol_error (cherry picked from commit 36f7e8d6f4c689f60e2514d911d31d5ee38677aa) --- deps/rabbit/src/rabbit_queue_type.erl | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/deps/rabbit/src/rabbit_queue_type.erl b/deps/rabbit/src/rabbit_queue_type.erl index cfbf1ed7c5dc..849e34361259 100644 --- a/deps/rabbit/src/rabbit_queue_type.erl +++ b/deps/rabbit/src/rabbit_queue_type.erl @@ -282,8 +282,12 @@ is_compatible(Type, Durable, Exclusive, AutoDelete) -> declare(Q0, Node) -> Q = rabbit_queue_decorator:set(rabbit_policy:set(Q0)), Mod = amqqueue:get_type(Q), - ok = check_vhost_queue_limit(Q), - Mod:declare(Q, Node). + case check_vhost_queue_limit(Q) of + ok -> + Mod:declare(Q, Node); + Error -> + Error + end. -spec delete(amqqueue:amqqueue(), boolean(), boolean(), rabbit_types:username()) -> @@ -736,8 +740,11 @@ check_vhost_queue_limit(Q) -> #resource{name = QueueName} = amqqueue:get_name(Q), VHost = amqqueue:get_vhost(Q), case rabbit_vhost_limit:is_over_queue_limit(VHost) of - false -> ok; - {true, Limit} -> rabbit_misc:precondition_failed("cannot declare queue '~ts': " - "queue limit in vhost '~ts' (~tp) is reached", - [QueueName, VHost, Limit]) + false-> + ok; + {true, Limit} -> + {protocol_error, precondition_failed, + "cannot declare queue '~ts': " + "queue limit in vhost '~ts' (~tp) is reached", + [QueueName, VHost, Limit]} end. From 5aedf85326cdc77c0f80c1548401d40f4d9c61ca Mon Sep 17 00:00:00 2001 From: Simon Unge Date: Tue, 14 May 2024 17:19:17 +0000 Subject: [PATCH 3/5] Prepare for adding more checks (cherry picked from commit d2192fbcfdf97bc2712ed7f5568f4a0e8187f7d2) --- deps/rabbit/src/rabbit_queue_type.erl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/deps/rabbit/src/rabbit_queue_type.erl b/deps/rabbit/src/rabbit_queue_type.erl index 849e34361259..8c70b7bbd6f4 100644 --- a/deps/rabbit/src/rabbit_queue_type.erl +++ b/deps/rabbit/src/rabbit_queue_type.erl @@ -6,6 +6,7 @@ %% -module(rabbit_queue_type). +-feature(maybe_expr, enable). -behaviour(rabbit_registry_class). @@ -282,7 +283,7 @@ is_compatible(Type, Durable, Exclusive, AutoDelete) -> declare(Q0, Node) -> Q = rabbit_queue_decorator:set(rabbit_policy:set(Q0)), Mod = amqqueue:get_type(Q), - case check_vhost_queue_limit(Q) of + case check_queue_limits(Q) of ok -> Mod:declare(Q, Node); Error -> @@ -736,6 +737,12 @@ known_queue_type_names() -> QTypeBins = lists:map(fun(X) -> atom_to_binary(X) end, QueueTypes), ?KNOWN_QUEUE_TYPES ++ QTypeBins. +check_queue_limits(Q) -> + maybe + %% Prepare for more checks + ok ?= check_vhost_queue_limit(Q) + end. + check_vhost_queue_limit(Q) -> #resource{name = QueueName} = amqqueue:get_name(Q), VHost = amqqueue:get_vhost(Q), From 5b2f9da6fb54e0311c7b1f4f09153c2281d403bc Mon Sep 17 00:00:00 2001 From: Simon Unge Date: Tue, 14 May 2024 20:54:43 +0000 Subject: [PATCH 4/5] Now with spec (cherry picked from commit a5214f356c254bbf100079ed41dc89adc48d9b45) --- deps/rabbit/src/rabbit_queue_type.erl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/deps/rabbit/src/rabbit_queue_type.erl b/deps/rabbit/src/rabbit_queue_type.erl index 8c70b7bbd6f4..e4a83050b169 100644 --- a/deps/rabbit/src/rabbit_queue_type.erl +++ b/deps/rabbit/src/rabbit_queue_type.erl @@ -737,6 +737,9 @@ known_queue_type_names() -> QTypeBins = lists:map(fun(X) -> atom_to_binary(X) end, QueueTypes), ?KNOWN_QUEUE_TYPES ++ QTypeBins. +-spec check_queue_limits(amqqueue:amqqueue()) -> + ok | + {protocol_error, Type :: atom(), Reason :: string(), Args :: term()}. check_queue_limits(Q) -> maybe %% Prepare for more checks From f97bc4267e0c199c1b24eaf20452ed69e9808128 Mon Sep 17 00:00:00 2001 From: Simon Unge Date: Tue, 14 May 2024 23:18:33 +0000 Subject: [PATCH 5/5] Minor cosmetic fix (cherry picked from commit 4a6c009df9c5db7deff38660d9ed378ac3a50fb1) --- deps/rabbit/src/rabbit_queue_type.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/rabbit/src/rabbit_queue_type.erl b/deps/rabbit/src/rabbit_queue_type.erl index e4a83050b169..56e31d40e138 100644 --- a/deps/rabbit/src/rabbit_queue_type.erl +++ b/deps/rabbit/src/rabbit_queue_type.erl @@ -750,7 +750,7 @@ check_vhost_queue_limit(Q) -> #resource{name = QueueName} = amqqueue:get_name(Q), VHost = amqqueue:get_vhost(Q), case rabbit_vhost_limit:is_over_queue_limit(VHost) of - false-> + false -> ok; {true, Limit} -> {protocol_error, precondition_failed,