Skip to content

Commit 2a8676b

Browse files
dumbbelldumbbell
committed
rabbit_feature_flags: Sync enabled feature flags differently on virgin node
If a virgin node starts as clustered (thanks to peer discovery), we need to mark feature flags already enabled remotely as enabled locally too. We can't do a regular cluster sync because remote nodes may have required feature flags which are disabled locally on the virgin node. Therefore, those nodes don't have the migration code for these feature flags anymore and the feature flags' state can't be changed. By doing this special sync, we allow a clustered virgin node to join a cluster with remote nodes having required feature flags. (cherry picked from commit 2f301b4)
1 parent 22a1a19 commit 2a8676b

File tree

4 files changed

+401
-10
lines changed

4 files changed

+401
-10
lines changed

‎deps/rabbit/src/rabbit_feature_flags.erl‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1872,7 +1872,59 @@ sync_cluster_v1([], NodeIsVirgin, _) ->
18721872
"current state"),
18731873
ok
18741874
end;
1875-
sync_cluster_v1(Nodes, _, Timeout) ->
1875+
sync_cluster_v1(Nodes, true = _NodeIsVirgin, Timeout) ->
1876+
[Node | _] = Nodes,
1877+
rabbit_log_feature_flags:debug(
1878+
"Feature flags: marking feature flags required remotely as enabled "
1879+
"locally because this node is virgin; using ~s as the remote node",
1880+
[Node]),
1881+
?assertNotEqual(node(), Node),
1882+
Ret1 = rpc:call(Node, ?MODULE, list, [enabled], Timeout),
1883+
case Ret1 of
1884+
FeatureFlags when is_map(FeatureFlags) ->
1885+
%% We only consider required feature flags (for which there is no
1886+
%% migration code on nodes where it is required). Remotely enabled
1887+
%% feature flags will be handled by the regular sync.
1888+
RequiredFeatureFlags =
1889+
maps:filter(
1890+
fun(_, FeatureProps) ->
1891+
required =:= get_stability(FeatureProps)
1892+
end, FeatureFlags),
1893+
FeatureNames = lists:sort(maps:keys(RequiredFeatureFlags)),
1894+
rabbit_log_feature_flags:error(
1895+
"Feature flags: list of feature flags to automatically mark as "
1896+
"enabled on this virgin node: ~0p",
1897+
[FeatureNames]),
1898+
1899+
%% We mark them as enabled directly (no migration code is executed)
1900+
%% because this node is virgin (i.e. no files on disk) and will
1901+
%% inherit the database from remote nodes.
1902+
Ret2 =
1903+
lists:foldl(
1904+
fun
1905+
(FeatureName, ok) ->
1906+
case is_supported_locally(FeatureName) of
1907+
true -> mark_as_enabled_locally(FeatureName, true);
1908+
false -> ok
1909+
end;
1910+
(_, Acc) ->
1911+
Acc
1912+
end, ok, FeatureNames),
1913+
1914+
%% We continue with the regular sync process. It will handle other
1915+
%% enabled feature flags and unsupported ones too.
1916+
case Ret2 of
1917+
ok -> sync_feature_flags_with_cluster(Nodes, false, Timeout);
1918+
_ -> Ret2
1919+
end;
1920+
{badrpc, _} = Reason ->
1921+
rabbit_log_feature_flags:error(
1922+
"Feature flags: failed to query remotely enabled feature flags "
1923+
"on node ~s: ~p",
1924+
[Node, Reason]),
1925+
{error, Reason}
1926+
end;
1927+
sync_cluster_v1(Nodes, _NodeIsVirgin, Timeout) ->
18761928
case sync_feature_flags_v2_first(Nodes, Timeout) of
18771929
true ->
18781930
?LOG_DEBUG(

‎deps/rabbit/src/rabbit_ff_controller.erl‎

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ enable_with_registry_locked(
577577
[FeatureName],
578578
#{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
579579

580-
case update_feature_state_and_enable(Inventory, FeatureName) of
580+
case check_required_and_enable(Inventory, FeatureName) of
581581
{ok, _Inventory} = Ok ->
582582
?LOG_NOTICE(
583583
"Feature flags: `~s` enabled",
@@ -593,6 +593,71 @@ enable_with_registry_locked(
593593
end
594594
end.
595595

596+
-spec check_required_and_enable(Inventory, FeatureName) -> Ret when
597+
Inventory :: rabbit_feature_flags:cluster_inventory(),
598+
FeatureName :: rabbit_feature_flags:feature_name(),
599+
Ret :: {ok, Inventory} | {error, Reason},
600+
Reason :: term().
601+
602+
check_required_and_enable(
603+
#{feature_flags := FeatureFlags,
604+
states_per_node := _} = Inventory,
605+
FeatureName) ->
606+
%% Required feature flags vs. virgin nodes.
607+
FeatureProps = maps:get(FeatureName, FeatureFlags),
608+
Stability = rabbit_feature_flags:get_stability(FeatureProps),
609+
NodesWhereDisabled = list_nodes_where_feature_flag_is_disabled(
610+
Inventory, FeatureName),
611+
612+
MarkDirectly = case Stability of
613+
required ->
614+
?LOG_DEBUG(
615+
"Feature flags: `~s`: the feature flag is "
616+
"required on some nodes; list virgin nodes "
617+
"to determine if the feature flag can simply "
618+
"be marked as enabled",
619+
[FeatureName],
620+
#{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
621+
VirginNodesWhereDisabled =
622+
lists:filter(
623+
fun(Node) ->
624+
case is_virgin_node(Node) of
625+
IsVirgin when is_boolean(IsVirgin) ->
626+
IsVirgin;
627+
undefined ->
628+
false
629+
end
630+
end, NodesWhereDisabled),
631+
VirginNodesWhereDisabled =:= NodesWhereDisabled;
632+
_ ->
633+
false
634+
end,
635+
636+
case MarkDirectly of
637+
false ->
638+
case Stability of
639+
required ->
640+
?LOG_DEBUG(
641+
"Feature flags: `~s`: some nodes where the feature "
642+
"flag is disabled are not virgin, we need to perform "
643+
"a regular sync",
644+
[FeatureName],
645+
#{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS});
646+
_ ->
647+
ok
648+
end,
649+
update_feature_state_and_enable(Inventory, FeatureName);
650+
true ->
651+
?LOG_DEBUG(
652+
"Feature flags: `~s`: all nodes where the feature flag is "
653+
"disabled are virgin, we can directly mark it as enabled "
654+
"there",
655+
[FeatureName],
656+
#{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
657+
mark_as_enabled_on_nodes(
658+
NodesWhereDisabled, Inventory, FeatureName, true)
659+
end.
660+
596661
-spec update_feature_state_and_enable(Inventory, FeatureName) -> Ret when
597662
Inventory :: rabbit_feature_flags:cluster_inventory(),
598663
FeatureName :: rabbit_feature_flags:feature_name(),
@@ -646,6 +711,14 @@ update_feature_state_and_enable(
646711
Error
647712
end.
648713

714+
is_virgin_node(Node) ->
715+
case rpc_call(Node, rabbit_mnesia, is_virgin_node, [], ?TIMEOUT) of
716+
IsVirgin when is_boolean(IsVirgin) ->
717+
IsVirgin;
718+
{error, _} ->
719+
undefined
720+
end.
721+
649722
-spec do_enable(Inventory, FeatureName, Nodes) -> Ret when
650723
Inventory :: rabbit_feature_flags:cluster_inventory(),
651724
FeatureName :: rabbit_feature_flags:feature_name(),
@@ -740,7 +813,7 @@ collect_inventory_on_nodes(Nodes, Timeout) ->
740813
{ok, #{feature_flags := FeatureFlags,
741814
applications_per_node := ScannedAppsPerNode,
742815
states_per_node := StatesPerNode} = Inventory}) ->
743-
FeatureFlags2 = maps:merge(FeatureFlags, FeatureFlags1),
816+
FeatureFlags2 = merge_feature_flags(FeatureFlags, FeatureFlags1),
744817
ScannedAppsPerNode1 = ScannedAppsPerNode#{Node => ScannedApps},
745818
StatesPerNode1 = StatesPerNode#{Node => FeatureStates},
746819
Inventory1 = Inventory#{
@@ -756,6 +829,32 @@ collect_inventory_on_nodes(Nodes, Timeout) ->
756829
Error
757830
end, {ok, Inventory0}, Rets).
758831

832+
merge_feature_flags(FeatureFlagsA, FeatureFlagsB) ->
833+
FeatureFlags = maps:merge(FeatureFlagsA, FeatureFlagsB),
834+
maps:map(
835+
fun(FeatureName, FeatureProps) ->
836+
FeaturePropsA = maps:get(FeatureName, FeatureFlagsA, #{}),
837+
FeaturePropsB = maps:get(FeatureName, FeatureFlagsB, #{}),
838+
839+
%% There is a rank between stability levels. If a feature flag
840+
%% is required somewhere, it is required globally. Otherwise if
841+
%% it is stable somewhere, it is stable globally.
842+
StabilityA = rabbit_feature_flags:get_stability(FeaturePropsA),
843+
StabilityB = rabbit_feature_flags:get_stability(FeaturePropsB),
844+
Stability = case {StabilityA, StabilityB} of
845+
{required, _} -> required;
846+
{_, required} -> required;
847+
{stable, _} -> stable;
848+
{_, stable} -> stable;
849+
_ -> experimental
850+
end,
851+
852+
FeatureProps1 = FeatureProps#{stability => Stability},
853+
FeatureProps2 = maps:remove(migration_fun, FeatureProps1),
854+
FeatureProps3 = maps:remove(callbacks, FeatureProps2),
855+
FeatureProps3
856+
end, FeatureFlags).
857+
759858
-spec list_feature_flags_enabled_somewhere(Inventory, HandleStateChanging) ->
760859
Ret when
761860
Inventory :: rabbit_feature_flags:cluster_inventory(),

‎deps/rabbit/src/rabbit_mnesia.erl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
is_registered_process_alive/1,
2626
cluster_nodes/1,
2727
node_type/0,
28+
is_virgin_node/0,
2829
dir/0,
2930
cluster_status_from_mnesia/0,
3031

0 commit comments

Comments
 (0)