Skip to content

Commit c239399

Browse files
dumbbelldumbbell
authored
Merge pull request #6791 from rabbitmq/enable-remote-ff-on-virgin-node
rabbit_feature_flags: Sync enabled feature flags differently on virgin node
2 parents 0ffe61c + 2f301b4 commit c239399

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: `~ts` 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)