Modify default queue type injection logic #13837
Merged
+79
−16
Conversation
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
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).
Rather than injecting node-level DQT when exporting definitions, inject it into vhost's metadata when a vhost is created.
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.
|
@lukebakken @mkuratczyk and I have discussed this in detail, there is also at least one related change in With this change, the injection of the DQT moves from definition export time to virtual host creation time, which is easier to reason about and avoids inconsistent or surprising behavior in a number of scenarios. |
|
@mkuratczyk FTR, the |
dcorbacho
approved these changes
May 5, 2025
mkuratczyk
changed the title
michaelklishin
approved these changes
May 5, 2025
michaelklishin
changed the title
michaelklishin
added a commit
that referenced
this pull request
May 5, 2025
Modify default queue type injection logic (backport #13837)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.