Skip to content

Commit 3b85d0f

Browse files
committed
Feature flags: Call the migration function after the flag is marked as enabled
This allows the migration function do perform some cleanup after the running code switched to the new thing. While here, fix a couple style issues in `rabbit_core_ff`.
1 parent 23a834e commit 3b85d0f

File tree

2 files changed

+40
-11
lines changed

2 files changed

+40
-11
lines changed

deps/rabbit/src/rabbit_core_ff.erl

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ quorum_queue_migration(_FeatureName, _FeatureProps, is_enabled) ->
8888
rabbit_table:wait(Tables, _Retry = true),
8989
Fields = amqqueue:fields(amqqueue_v2),
9090
mnesia:table_info(rabbit_queue, attributes) =:= Fields andalso
91-
mnesia:table_info(rabbit_durable_queue, attributes) =:= Fields.
91+
mnesia:table_info(rabbit_durable_queue, attributes) =:= Fields;
92+
quorum_queue_migration(_FeatureName, _FeatureProps, post_enabled_locally) ->
93+
ok.
9294

9395
stream_queue_migration(_FeatureName, _FeatureProps, _Enable) ->
9496
ok.
@@ -122,9 +124,12 @@ implicit_default_bindings_migration(FeatureName, _FeatureProps,
122124
rabbit_table:wait([rabbit_queue]),
123125
Queues = mnesia:dirty_all_keys(rabbit_queue),
124126
remove_explicit_default_bindings(FeatureName, Queues);
125-
implicit_default_bindings_migration(_Feature_Name, _FeatureProps,
127+
implicit_default_bindings_migration(_FeatureName, _FeatureProps,
126128
is_enabled) ->
127-
undefined.
129+
undefined;
130+
implicit_default_bindings_migration(_FeatureName, _FeatureProps,
131+
post_enabled_locally) ->
132+
ok.
128133

129134
remove_explicit_default_bindings(_FeatureName, []) ->
130135
ok;
@@ -150,7 +155,10 @@ virtual_host_metadata_migration(_FeatureName, _FeatureProps, enable) ->
150155
{aborted, Reason} -> {error, Reason}
151156
end;
152157
virtual_host_metadata_migration(_FeatureName, _FeatureProps, is_enabled) ->
153-
mnesia:table_info(rabbit_vhost, attributes) =:= vhost:fields(vhost_v2).
158+
mnesia:table_info(rabbit_vhost, attributes) =:= vhost:fields(vhost_v2);
159+
virtual_host_metadata_migration(
160+
_FeatureName, _FeatureProps, post_enabled_locally) ->
161+
ok.
154162

155163
%% -------------------------------------------------------------------
156164
%% Maintenance mode.
@@ -172,7 +180,10 @@ maintenance_mode_status_migration(FeatureName, _FeatureProps, enable) ->
172180
[Reason])
173181
end;
174182
maintenance_mode_status_migration(_FeatureName, _FeatureProps, is_enabled) ->
175-
rabbit_table:exists(rabbit_maintenance:status_table_name()).
183+
rabbit_table:exists(rabbit_maintenance:status_table_name());
184+
maintenance_mode_status_migration(
185+
_FeatureName, _FeatureProps, post_enabled_locally) ->
186+
ok.
176187

177188
%% -------------------------------------------------------------------
178189
%% User limits.
@@ -182,9 +193,14 @@ user_limits_migration(_FeatureName, _FeatureProps, enable) ->
182193
Tab = rabbit_user,
183194
rabbit_table:wait([Tab], _Retry = true),
184195
Fun = fun(Row) -> internal_user:upgrade_to(internal_user_v2, Row) end,
185-
case mnesia:transform_table(Tab, Fun, internal_user:fields(internal_user_v2)) of
196+
Ret = mnesia:transform_table(
197+
Tab, Fun, internal_user:fields(internal_user_v2)),
198+
case Ret of
186199
{atomic, ok} -> ok;
187200
{aborted, Reason} -> {error, Reason}
188201
end;
189202
user_limits_migration(_FeatureName, _FeatureProps, is_enabled) ->
190-
mnesia:table_info(rabbit_user, attributes) =:= internal_user:fields(internal_user_v2).
203+
mnesia:table_info(rabbit_user, attributes) =:=
204+
internal_user:fields(internal_user_v2);
205+
user_limits_migration(_FeatureName, _FeatureProps, post_enabled_locally) ->
206+
ok.

deps/rabbit/src/rabbit_feature_flags.erl

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@
7777

7878
-module(rabbit_feature_flags).
7979

80+
-include_lib("stdlib/include/assert.hrl").
81+
8082
-export([list/0,
8183
list/1,
8284
list/2,
@@ -1650,7 +1652,7 @@ mark_as_enabled(FeatureName, IsEnabled) ->
16501652
end.
16511653

16521654
-spec mark_as_enabled_locally(feature_name(), feature_state()) ->
1653-
any() | {error, any()} | no_return().
1655+
ok | {error, any()} | no_return().
16541656
%% @private
16551657

16561658
mark_as_enabled_locally(FeatureName, IsEnabled) ->
@@ -1673,9 +1675,20 @@ mark_as_enabled_locally(FeatureName, IsEnabled) ->
16731675
ok =:= try_to_write_enabled_feature_flags_list(
16741676
NewEnabledFeatureNames)
16751677
end,
1676-
initialize_registry(#{},
1677-
#{FeatureName => IsEnabled},
1678-
WrittenToDisk).
1678+
Ret = initialize_registry(#{},
1679+
#{FeatureName => IsEnabled},
1680+
WrittenToDisk),
1681+
case Ret of
1682+
ok when IsEnabled ->
1683+
?assert(is_enabled(FeatureName, non_blocking)),
1684+
spawn(
1685+
fun() ->
1686+
_ = run_migration_fun(FeatureName, post_enabled_locally)
1687+
end);
1688+
_ ->
1689+
ok
1690+
end,
1691+
Ret.
16791692

16801693
-spec mark_as_enabled_remotely(feature_name(), feature_state()) ->
16811694
any() | {error, any()} | no_return().

0 commit comments

Comments
 (0)