Skip to content

Conversation

@dumbbell
Copy link
Collaborator

Why

Up-to RabbitMQ 3.13.x, there was a case where if:

  1. you enabled a plugin
  2. you enabled its feature flags
  3. you disabled the plugin
  4. you restarted a node (or upgraded it)

... the node could crash on startup because it had a feature flag marked as enabled that it didn't know about:

error:{badmatch,#{feature_flags => ...

    rabbit_ff_controller:-check_one_way_compatibility/2-fun-0-/3, line 514
    lists:all_1/2, line 1520
    rabbit_ff_controller:are_compatible/2, line 496
    rabbit_ff_controller:check_node_compatibility_task1/4, line 437
    rabbit_db_cluster:check_compatibility/1, line 376

This was "fixed" by the new way of keeping the registry in memory (#10988) because it introduces a slight change of behavior. Indeed, the old way walked through the FeatureFlags map and looked up the state in the FeatureStates map to create the is_enabled/1 function. The new way just looks up the state in FeatureStates.

How

The new testcase succeeds on 4.0.x and main, but would fail on 3.13.x with the aforementionne crash.

[Why]
Up-to RabbitMQ 3.13.x, there was a case where if:
1. you enabled a plugin
2. you enabled its feature flags
3. you disabled the plugin
4. you restarted a node (or upgraded it)

... the node could crash on startup because it had a feature flag marked
as enabled that it didn't know about:

    error:{badmatch,#{feature_flags => ...

        rabbit_ff_controller:-check_one_way_compatibility/2-fun-0-/3, line 514
        lists:all_1/2, line 1520
        rabbit_ff_controller:are_compatible/2, line 496
        rabbit_ff_controller:check_node_compatibility_task1/4, line 437
        rabbit_db_cluster:check_compatibility/1, line 376

This was "fixed" by the new way of keeping the registry in memory
(#10988) because it introduces a slight change of behavior. Indeed, the
old way walked through the `FeatureFlags` map and looked up the state in
the `FeatureStates` map to create the `is_enabled/1` function. The new
way just looks up the state in `FeatureStates`.

[How]
The new testcase succeeds on 4.0.x and `main`, but would fail on 3.13.x
with the aforementionne crash.
@dumbbell dumbbell force-pushed the add-feature-flags-testcase-for-issue-12963 branch from c7afc29 to ea2c8db Compare December 19, 2024 15:34
@dumbbell dumbbell marked this pull request as ready for review December 19, 2024 16:28
@dumbbell dumbbell merged commit f9257e8 into main Dec 19, 2024
273 checks passed
@dumbbell dumbbell deleted the add-feature-flags-testcase-for-issue-12963 branch December 19, 2024 16:28
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