Skip to content

Conversation

@dumbbell
Copy link
Collaborator

@dumbbell dumbbell commented Dec 15, 2022

So far, when a new node is started, the feature flags v1 code is used to enable all stable feature flags (the default behavior). In the list, there would be feature_flags_v2, so this one would be enabled first and all others would be using the v2 code path.

If we mark feature_flags_v2 as required, the v2 code path didn't handle this so far. This patch adds this missing code. Now, a virgin node's feature flags can be enabled fully using the v2 code path.

As part of that, the get_forced_feature_flag_names() function is moved from rabbit_feature_flags to rabbit_ff_controller.

It depends on #6791.

Update: The patch was split into two parts, this one (which was the initial scope) and #6810.

@dumbbell dumbbell added this to the 3.12.0 milestone Dec 15, 2022
@dumbbell dumbbell self-assigned this Dec 15, 2022
@dumbbell dumbbell force-pushed the add-feature-flags-v2-virgin-node-init-support branch from bf973c2 to 14ae1b5 Compare December 18, 2022 18:29
@dumbbell dumbbell changed the title rabbit_feature_flags: Support virgin node init with feature flags v2 rabbit_feature_flags: keep v2 code path only and drop v1 Dec 18, 2022
@dumbbell
Copy link
Collaborator Author

Note that I widened the scope of this pull request to make the v2 code path required. The description was updated accordingly.

@dumbbell dumbbell force-pushed the add-feature-flags-v2-virgin-node-init-support branch 3 times, most recently from 4cddbee to 3470fcf Compare December 18, 2022 20:36
@mergify mergify bot added the bazel label Dec 18, 2022
@dumbbell dumbbell changed the title rabbit_feature_flags: keep v2 code path only and drop v1 rabbit_feature_flags: keep v2 code path only (drop v1) Dec 18, 2022
@dumbbell dumbbell force-pushed the add-feature-flags-v2-virgin-node-init-support branch 2 times, most recently from 8121f1d to de54251 Compare December 19, 2022 14:15
@dumbbell dumbbell marked this pull request as ready for review December 19, 2022 14:33
@dumbbell dumbbell marked this pull request as draft December 22, 2022 14:29
@dumbbell
Copy link
Collaborator Author

I marked this pull request as draft again because I would like to sort out the problem I describe in #6701 (comment) first.

@dumbbell dumbbell force-pushed the add-feature-flags-v2-virgin-node-init-support branch from de54251 to 0d3639d Compare January 5, 2023 07:40
So far, when a new node is started, the feature flags v1 code is used to
enable all stable feature flags (the default behavior). In the list,
there would be `feature_flags_v2`, so this one would be enabled first
and all others would be using the v2 code path.

If we mark `feature_flags_v2` as required, the v2 code path didn't
handle this so far. This patch adds this missing code. Now, a virgin
node's feature flags can be enabled fully using the v2 code path.

As part of that, th `get_forced_feature_flag_names()` function is moved
from `rabbit_feature_flags` to `rabbit_ff_controller`.
@dumbbell dumbbell force-pushed the add-feature-flags-v2-virgin-node-init-support branch from 0d3639d to bb501a0 Compare January 5, 2023 15:42
…nal use

The two exported functions are used by the testsuite. They should be
exported when `TEST` is defined only, but the way mixed-version testing
is implemented in Bazel forces us to export them.
@dumbbell dumbbell force-pushed the add-feature-flags-v2-virgin-node-init-support branch from bb501a0 to 80da804 Compare January 5, 2023 21:54
@dumbbell dumbbell changed the title rabbit_feature_flags: keep v2 code path only (drop v1) Feature flags: Add virgin node init support to v2 code path Jan 5, 2023
@dumbbell dumbbell marked this pull request as ready for review January 6, 2023 11:02
@dumbbell dumbbell merged commit 478d9ee into main Jan 6, 2023
@dumbbell dumbbell deleted the add-feature-flags-v2-virgin-node-init-support branch January 9, 2023 21:31
dumbbell added a commit that referenced this pull request Jan 9, 2023
Feature flags: Add virgin node init support to v2 code path (backport #6682)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants