From 2f67d19bec2dc2d0d0372c9dcade79e072f88789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Thu, 3 Oct 2024 12:49:04 +0200 Subject: [PATCH 1/2] rabbit_feature_flags: Lock registry once and enable many feature flags [Why] Before this change, the controller was looping on all feature flags to enable, then for each: 1. it checked if it was supported 2. it acquired the registry lock 3. it enabled the feature flag 4. it released the registry lock It was done this way to not acquire the log if the feature flag was unsupported in the first place. However, this put more load on the lock mechanism. [How] This commit changes the order. The controller acquires the registry lock once, then loops on feature flags to enable. The support check is now under the registry lock. --- deps/rabbit/src/rabbit_ff_controller.erl | 42 ++++++++++++------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/deps/rabbit/src/rabbit_ff_controller.erl b/deps/rabbit/src/rabbit_ff_controller.erl index 13a2b4f5153d..0fef2c57502e 100644 --- a/deps/rabbit/src/rabbit_ff_controller.erl +++ b/deps/rabbit/src/rabbit_ff_controller.erl @@ -823,12 +823,29 @@ refresh_after_app_load_task() -> Ret :: ok | {error, Reason}, Reason :: term(). -enable_many(#{states_per_node := _} = Inventory, [FeatureName | Rest]) -> +enable_many(#{states_per_node := _} = Inventory, FeatureNames) -> + %% We acquire a lock before making any change to the registry. This is not + %% used by the controller (because it is already using a globally + %% registered name to prevent concurrent runs). But this is used in + %% `rabbit_feature_flags:is_enabled()' to block while the state is + %% `state_changing'. + rabbit_ff_registry_factory:acquire_state_change_lock(), + Ret = enable_many_locked(Inventory, FeatureNames), + rabbit_ff_registry_factory:release_state_change_lock(), + Ret. + +-spec enable_many_locked(Inventory, FeatureNames) -> Ret when + Inventory :: rabbit_feature_flags:cluster_inventory(), + FeatureNames :: [rabbit_feature_flags:feature_name()], + Ret :: ok | {error, Reason}, + Reason :: term(). + +enable_many_locked(#{states_per_node := _} = Inventory, [FeatureName | Rest]) -> case enable_if_supported(Inventory, FeatureName) of - {ok, Inventory1} -> enable_many(Inventory1, Rest); + {ok, Inventory1} -> enable_many_locked(Inventory1, Rest); Error -> Error end; -enable_many(_Inventory, []) -> +enable_many_locked(_Inventory, []) -> ok. -spec enable_if_supported(Inventory, FeatureName) -> Ret when @@ -845,7 +862,7 @@ enable_if_supported(#{states_per_node := _} = Inventory, FeatureName) -> "Feature flags: `~ts`: supported; continuing", [FeatureName], #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}), - lock_registry_and_enable(Inventory, FeatureName); + enable_with_registry_locked(Inventory, FeatureName); false -> ?LOG_DEBUG( "Feature flags: `~ts`: unsupported; aborting", @@ -854,23 +871,6 @@ enable_if_supported(#{states_per_node := _} = Inventory, FeatureName) -> {error, unsupported} end. --spec lock_registry_and_enable(Inventory, FeatureName) -> Ret when - Inventory :: rabbit_feature_flags:cluster_inventory(), - FeatureName :: rabbit_feature_flags:feature_name(), - Ret :: {ok, Inventory} | {error, Reason}, - Reason :: term(). - -lock_registry_and_enable(#{states_per_node := _} = Inventory, FeatureName) -> - %% We acquire a lock before making any change to the registry. This is not - %% used by the controller (because it is already using a globally - %% registered name to prevent concurrent runs). But this is used in - %% `rabbit_feature_flags:is_enabled()' to block while the state is - %% `state_changing'. - rabbit_ff_registry_factory:acquire_state_change_lock(), - Ret = enable_with_registry_locked(Inventory, FeatureName), - rabbit_ff_registry_factory:release_state_change_lock(), - Ret. - -spec enable_with_registry_locked(Inventory, FeatureName) -> Ret when Inventory :: rabbit_feature_flags:cluster_inventory(), FeatureName :: rabbit_feature_flags:feature_name(), From e4abbfd6c243ffa517d5008413e264a1f11d46b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Thu, 3 Oct 2024 12:54:24 +0200 Subject: [PATCH 2/2] rabbit_feature_flags: Fix copyright year The subsystem didn't exist before 2019. The deprecated features support was added in 2023. --- deps/rabbit/src/rabbit_depr_ff_extra.erl | 2 +- deps/rabbit/src/rabbit_deprecated_features.erl | 6 ++++-- deps/rabbit/src/rabbit_feature_flags.erl | 6 ++++-- deps/rabbit/src/rabbit_ff_controller.erl | 6 ++++-- deps/rabbit/src/rabbit_ff_extra.erl | 3 ++- deps/rabbit/src/rabbit_ff_registry.erl | 6 ++++-- deps/rabbit/src/rabbit_ff_registry_factory.erl | 3 ++- deps/rabbit/src/rabbit_ff_registry_wrapper.erl | 6 ++++-- 8 files changed, 25 insertions(+), 13 deletions(-) diff --git a/deps/rabbit/src/rabbit_depr_ff_extra.erl b/deps/rabbit/src/rabbit_depr_ff_extra.erl index 5267c3efbfb6..2b4998433167 100644 --- a/deps/rabbit/src/rabbit_depr_ff_extra.erl +++ b/deps/rabbit/src/rabbit_depr_ff_extra.erl @@ -2,7 +2,7 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% Copyright (c) 2023 Broadcom. All Rights Reserved. The term “Broadcom” +%% Copyright (c) 2023-2024 Broadcom. All Rights Reserved. The term “Broadcom” %% refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. %% %% @doc diff --git a/deps/rabbit/src/rabbit_deprecated_features.erl b/deps/rabbit/src/rabbit_deprecated_features.erl index 93289be033eb..ffafec5757b9 100644 --- a/deps/rabbit/src/rabbit_deprecated_features.erl +++ b/deps/rabbit/src/rabbit_deprecated_features.erl @@ -2,11 +2,13 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% Copyright (c) 2023-2024 Broadcom. All Rights Reserved. The term “Broadcom” +%% refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. %% %% @author The RabbitMQ team -%% @copyright 2007-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% @copyright 2023-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. +%% and/or its subsidiaries. All rights reserved. %% %% @doc %% This module provides an API to manage deprecated features in RabbitMQ. It diff --git a/deps/rabbit/src/rabbit_feature_flags.erl b/deps/rabbit/src/rabbit_feature_flags.erl index 07883d080ff1..3d2b19f8c7c6 100644 --- a/deps/rabbit/src/rabbit_feature_flags.erl +++ b/deps/rabbit/src/rabbit_feature_flags.erl @@ -2,11 +2,13 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% Copyright (c) 2019-2024 Broadcom. All Rights Reserved. The term “Broadcom” +%% refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. %% %% @author The RabbitMQ team -%% @copyright 2007-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% @copyright 2019-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. +%% and/or its subsidiaries. All rights reserved. %% %% @doc %% This module offers a framework to declare capabilities a RabbitMQ node diff --git a/deps/rabbit/src/rabbit_ff_controller.erl b/deps/rabbit/src/rabbit_ff_controller.erl index 0fef2c57502e..c522e1cd6c16 100644 --- a/deps/rabbit/src/rabbit_ff_controller.erl +++ b/deps/rabbit/src/rabbit_ff_controller.erl @@ -2,11 +2,13 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% Copyright (c) 2019-2024 Broadcom. All Rights Reserved. The term “Broadcom” +%% refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. %% %% @author The RabbitMQ team -%% @copyright 2007-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% @copyright 2019-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. +%% and/or its subsidiaries. All rights reserved. %% %% @doc %% The feature flag controller is responsible for synchronization and managing diff --git a/deps/rabbit/src/rabbit_ff_extra.erl b/deps/rabbit/src/rabbit_ff_extra.erl index 9eba72185936..0171c4200856 100644 --- a/deps/rabbit/src/rabbit_ff_extra.erl +++ b/deps/rabbit/src/rabbit_ff_extra.erl @@ -2,7 +2,8 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% @copyright 2007-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% @copyright 2019-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. +%% and/or its subsidiaries. All rights reserved. %% %% @doc %% This module provides extra functions unused by the feature flags diff --git a/deps/rabbit/src/rabbit_ff_registry.erl b/deps/rabbit/src/rabbit_ff_registry.erl index 864ff564dc64..eca99ebd9ec0 100644 --- a/deps/rabbit/src/rabbit_ff_registry.erl +++ b/deps/rabbit/src/rabbit_ff_registry.erl @@ -2,11 +2,13 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% Copyright (c) 2019-2024 Broadcom. All Rights Reserved. The term “Broadcom” +%% refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. %% %% @author The RabbitMQ team -%% @copyright 2007-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% @copyright 2019-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. +%% and/or its subsidiaries. All rights reserved. %% %% @doc %% This module exposes the API of the {@link rabbit_feature_flags} diff --git a/deps/rabbit/src/rabbit_ff_registry_factory.erl b/deps/rabbit/src/rabbit_ff_registry_factory.erl index 0d91a7b64955..68d81be6cf46 100644 --- a/deps/rabbit/src/rabbit_ff_registry_factory.erl +++ b/deps/rabbit/src/rabbit_ff_registry_factory.erl @@ -2,7 +2,8 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% Copyright (c) 2019-2024 Broadcom. All Rights Reserved. The term “Broadcom” +%% refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. %% -module(rabbit_ff_registry_factory). diff --git a/deps/rabbit/src/rabbit_ff_registry_wrapper.erl b/deps/rabbit/src/rabbit_ff_registry_wrapper.erl index beef32f657cf..a5f63eb64de4 100644 --- a/deps/rabbit/src/rabbit_ff_registry_wrapper.erl +++ b/deps/rabbit/src/rabbit_ff_registry_wrapper.erl @@ -2,11 +2,13 @@ %% License, v. 2.0. If a copy of the MPL was not distributed with this %% file, You can obtain one at https://mozilla.org/MPL/2.0/. %% -%% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% Copyright (c) 2019-2024 Broadcom. All Rights Reserved. The term “Broadcom” +%% refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. %% %% @author The RabbitMQ team -%% @copyright 2007-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% @copyright 2019-2024 Broadcom. The term “Broadcom” refers to Broadcom Inc. +%% and/or its subsidiaries. All rights reserved. %% %% @doc %% This module sits in front of {@link rabbit_ff_registry}.