is it only when it is disabled (-1) that it cannot be updated from a policy?
can it be updated eg from 20 to 40? if it can then this change will prevent the periodic policy repair to notice such change?

I really did not investigate thoroughly to find out what prevents the disabled delivery limit to be changed by a policy but this code part seems to allow it https://github.com/rabbitmq/rabbitmq-server/blob/v4.2.0-beta.2/deps/rabbit/src/rabbit_fifo.erl#L227.

isn't to problem of continuous update_config that rabbit_fifo converts a negative delivery limit to undefined so when maybe_apply_policies compares current config with new config, the former contains undefined while the later contains a negative value for the same unchanged "disabled" delivery limit?

I will shortly link a commit to show what I mean. (Sorry I just came back from vacation and couldn't look at this issue earlier)

is it only when it is disabled (-1) that it cannot be updated from a policy?
can it be updated eg from 20 to 40? if it can then this change will prevent the periodic policy repair to notice such change?

yes good point, this will probably disable "valid" repairs. that said with the defaulting logic I feel it is probably safest to exclude this key irrespectively.

This change f855eb5 prevents update_config every tick if delivery-limit is negative, but still allows the periodic check to detect when delivery-limit is changed via policy. What do you think?

This change f855eb5 prevents update_config every tick if delivery-limit is negative, but still allows the periodic check to detect when delivery-limit is changed via policy. What do you think?

yes I think this is fine - do we need the not IsQueueDeclaration guard?

do we need the not IsQueueDeclaration guard?

Not necessarily. Just wanted to highlight that the conversion is necessary only for the comparison, for the actual config update it does not matter.
Does this conversion (https://github.com/rabbitmq/rabbitmq-server/blob/v4.2.0-beta.2/deps/rabbit/src/rabbit_fifo.erl#L203-L205) still needed if the guard is removed and the fifo_client passes the disabled value as undefined? (I guess yes because of mixed version clusters or when applying older raft logs)

I will add tests and submit a PR early next week.

I have been doing some testing and your change allows the delivery_limit to be disable after it has been active, this didn't use to be the case and was a deliberate choice. We could ofc extract the delivery_limit key in the comparison and compare them using adjusted logic but I'm not convinced it is worth it. Especially as we are probably going to make some adjustments to deliver limit defaulting once we introduce Ra v3 (Which is better able to cope with repeated returns).