Skip to content

Commit aa78bad

Browse files
committed
rabbit_feature_flags: Fix experimental feature flags inventory
When we collect feature flag properties from all nodes, we start with an empty cluster inventory (a common Erlang recursion pattern). This means that all feature flags are unknown at first. In `merge_feature_flags()`, we must compute a global stability level for each feature flag, in case all nodes are not on the same page (like one nodes considers a feature flag experimental, but another one marks it as stable). That's why we rank stability level: required > stable > experimental. This ranking had one issue: `rabbit_feature_flags:get_stability/1` defaults to `stable` if a feature flag has not explicit stability set. Therefore, with our empty starting inventory, the starting stability would be `stable`. And it would superceed an experimental feature flag stability level, even though all nodes agree on that. Now, if a feature flag is missing from our inventory being collected, we consider its stability level to be experimental. This is different from a known feature flag with no explicit stability level. This way, we are sure that feature flags marked as experimental everywhere will be considered experimental globally.
1 parent 70094e6 commit aa78bad

File tree

1 file changed

+25
-5
lines changed

1 file changed

+25
-5
lines changed

deps/rabbit/src/rabbit_ff_controller.erl

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -982,12 +982,32 @@ merge_feature_flags(FeatureFlagsA, FeatureFlagsB) ->
982982
FeatureFlags = maps:merge(FeatureFlagsA, FeatureFlagsB),
983983
maps:map(
984984
fun(FeatureName, FeatureProps) ->
985-
FeaturePropsA = maps:get(FeatureName, FeatureFlagsA, #{}),
986-
FeaturePropsB = maps:get(FeatureName, FeatureFlagsB, #{}),
985+
%% When we collect feature flag properties from all nodes, we
986+
%% start with an empty cluster inventory (a common Erlang
987+
%% recursion pattern). This means that all feature flags are
988+
%% unknown at first.
989+
%%
990+
%% Here we must compute a global stability level for each
991+
%% feature flag, in case all nodes are not on the same page
992+
%% (like one nodes considers a feature flag experimental, but
993+
%% another one marks it as stable). That's why we rank stability
994+
%% level: required > stable > experimental.
995+
%%
996+
%% We must distinguish an unknown feature flag (they are all
997+
%% unknown at the beginning of the collection) from a known
998+
%% feature flags with no explicit stability level. In the latter
999+
%% case, `rabbit_feature_flags:get_stability/1' will consider it
1000+
%% stable. However in the former case, we must consider it
1001+
%% experimental otherwise it would default to be stable would
1002+
%% superceed an experimental level, even though all nodes agree
1003+
%% on that level, because `rabbit_feature_flags:get_stability/1'
1004+
%% would return stable as well.
1005+
UnknownProps = #{stability => experimental},
1006+
FeaturePropsA = maps:get(
1007+
FeatureName, FeatureFlagsA, UnknownProps),
1008+
FeaturePropsB = maps:get(
1009+
FeatureName, FeatureFlagsB, UnknownProps),
9871010

988-
%% There is a rank between stability levels. If a feature flag
989-
%% is required somewhere, it is required globally. Otherwise if
990-
%% it is stable somewhere, it is stable globally.
9911011
StabilityA = rabbit_feature_flags:get_stability(FeaturePropsA),
9921012
StabilityB = rabbit_feature_flags:get_stability(FeaturePropsB),
9931013
Stability = case {StabilityA, StabilityB} of

0 commit comments

Comments
 (0)