state_changing crash in classic queue when enabling feature flag stream_queues

[gomoripeti]

We are seeing a few of these crashes while enabling the feature flag stream_queues after a RabbitMQ upgrade (3.8.14 -> 3.10.1)

** Generic server <0.990.0> terminating
** Last message in was {'$gen_cast',
                           {run_backing_queue,rabbit_variable_queue,
...
** Reason for termination ==
** {{case_clause,state_changing},
    [{rabbit_classic_queue,confirm_to_sender,3,
                           [{file,"rabbit_classic_queue.erl"},{line,512}]},
     {maps,fold_1,3,[{file,"maps.erl"},{line,233}]},
     {rabbit_amqqueue_process,confirm_messages,3,
                              [{file,"rabbit_amqqueue_process.erl"},
                               {line,615}]},
...
Restarting crashed queue '<queue_name>' in vhost '<vhost_name>'.

I wonder if this "let it crash" behaviour is intentional for the short period while the feature flag is in a transitional state? Also it looks like it is harmless as the queue is restarted and continues without issue. Could you confirm there is no data loss in this case? We don't see any more consequences of this, but I am wondering could this cause more major crashes if eg too many queues hit this transitional state and some supervisor hits max intensity?

thanks in advance for the insights

[michaelklishin]

It would be nice to handle the exception, the function is fairly trivial:

confirm_to_sender(Pid, QName, MsgSeqNos) ->
    %% the stream queue included the queue type refactoring and thus requires
    %% a different message format
    Evt = case rabbit_ff_registry:is_enabled(stream_queue) of
              true ->
                  {queue_event, QName, {confirm, MsgSeqNos, self()}};
              false ->
                  {confirm, MsgSeqNos, self()}
          end,
    gen_server2:cast(Pid, Evt).

[michaelklishin]

We can treat "state changing" state the same way as false.

[dumbbell]

As discussed with @michaelklishin, calling rabbit_ff_registry:is_enabled/1 is incorrect: it is internal to the feature flags subsystem and may lead to various crashes very difficult to debug every time a feature flag is enabled.

The correct function is rabbit_feature_flags:is_enabled/1. This one will always return a boolean. It does take care of the wait if there is a need for it.

[dumbbell]

I submitted a patch in #5022 to fix it. The commit message contains a bit more details about the reasons why rabbit_ff_registry should not be used outside of the feature flags subsystem.

[gomoripeti]

thanks for the explanation, that's what I was missing as I couldn't find any other example of rabbit_ff_registry:is_enabled/1 in the code-base which was surprising. Makes sense now.