Modify default queue type injection logic (backport #13837)#13854
Merged
michaelklishin merged 7 commits intov4.1.xfrom May 5, 2025
Merged
Modify default queue type injection logic (backport #13837)#13854michaelklishin merged 7 commits intov4.1.xfrom
michaelklishin merged 7 commits intov4.1.xfrom
Conversation
The correct place for the `default_queue_type` property is inside the `metadata` block. However, right now we'd always export the value outside of `metadata` AND only export it inside `metadata`, if it was not `undefined`. This value outside of `metadata` was just misleading: if a user exported the definitins from a fresh node, changed `classic` to `quorum` and imported such modified values, the DQT would still be `classic`, because RMQ looks for the value inside `metadata`. Just to make it more confusing, if the DQT was changed successfully one way or another, the value outside of `metadata` would reflect that (it always shows the correct value, but is ignored on import). (cherry picked from commit 73da2a3)
(cherry picked from commit 5eb65f5)
Rather than injecting node-level DQT when exporting definitions, inject it into vhost's metadata when a vhost is created. (cherry picked from commit 3c95bf3)
(cherry picked from commit 0e743b5)
Vhosts that currently don't have their own default queue type, now inherit it from the node configuration and store it in their metadata going forward. (cherry picked from commit 9d0f01b)
(cherry picked from commit 9bd11b4)
(cherry picked from commit f61b9d9)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There were a few issues:
rabbitmqctl export definitions -would export thedefault_queue_typewithin vhost's metadata if set explicitly (correct) but also outside of themetadatablock even if not set explicitly for the vhost (not correct)rabbitmqadminwould always export the default queue type in both placesmetadatablock is just redundantmetadatablock is inconsistent withrabbitmqctl export_definitions(the difference stems from the fact that CTL only exports existing vhosts' metadata, while HTTP API injects the configuration-level DQT into each vhost'smetadataupon export)This PR changes the DQT logic: no DQT is injected upon export because it's injected/inherited from the node's configuration into vhost's metadata when a vhost is declared. Therefore each vhost has its own DQT explicitly defined and it's always present in any export (both CTL and HTTP API).
The downsides of this approach is that export/import between systems with a different configuration-level DQT will now require adjusting the DQT in the JSON file
However the logic is should be easier now and more consistent with how queue-level DQT works: when a queue is declared without a type, the default queue type is injected. Going forward, DQT changes don't affect the queue. Similarly, now if a vhost with no DQT is declared, configuration-level DQT is injected and configuration changes don't affect existing vhosts.
This is an automatic backport of pull request #13837 done by Mergify.