Race condition when declaring Queues and Policies programmatically (via Topology Operator)

[ikavgo]

First discovered in KNative integration pipelines and described here: https://github.com/knative-sandbox/eventing-rabbitmq/issues/792

Discussed with @dcorbacho.

How this race happens: When we create a queue rabbit first amend its spec with matching policy and then really declares putting into mnesia index. So if policy affecting queue is created between these two steps (and policy creates as insert policy into mnesia, then find all concerned queues), the queue stays without policy at all.

Per slack with @dcorbacho:

index 6e6aed9ab7..379dcc7e4f 100644
--- a/deps/rabbit/src/rabbit_queue_type.erl
+++ b/deps/rabbit/src/rabbit_queue_type.erl
@@ -239,7 +239,20 @@ is_enabled(Type) ->
 declare(Q0, Node) ->
     Q = rabbit_queue_decorator:set(rabbit_policy:set(Q0)),
     Mod = amqqueue:get_type(Q),
-    Mod:declare(Q, Node).
+    case Mod:declare(Q, Node) of
+        {'new', Q1} ->
+            Q2 = rabbit_policy:set(Q1),
+            case Q1 =/= Q2 of
+                true ->
+                    rabbit_amqqueue:ensure_rabbit_queue_record_is_initialized(Q2),
+                    Mod:policy_changed(Q2),
+                    {'new', Q2};
+                false ->
+                    {'new', Q1}
+            end;
+        Reply ->
+            Reply
+    end.

Diana writes:

This might do. Once the queue is declared, we check if the policy has changed. If it has, we store the new record and notify the changes. I haven’t tested it :grin: 
I think it’s a better option than the one huge transaction. This should rarely happen 
Credit to @dumbbell  for the idea

[ikavgo]

@dcorbacho @dumbbell I also looked at exchange - https://github.com/rabbitmq/rabbitmq-server/blob/master/deps/rabbit/src/rabbit_exchange.erl#L114 and I see the same pattern here: decorator call; ...; mnesia operation.

Looks like another race condition possible here?

[dumbbell]

Yes, it looks like the pattern is the same. We can probably apply the same solution in this case.