Add set_default_queue_type migration option#86
Draft
lukebakken wants to merge 5 commits into
Draft
Conversation
Add a `set_default_queue_type` field to the `migration_opts` record and parse it from the start-migration request body. Valid values are `quorum` and `classic`; any other value is rejected. Absent leaves the vhost default queue type untouched (the default). This commit only wires up parsing and validation; nothing applies the value yet. Setting the vhost default queue type on successful migration follows in a later commit for gh-79.
When `set_default_queue_type` is given, set the virtual host's default queue type (to `quorum` or `classic`) after the migration completes successfully, via `rabbit_vhost:update_metadata`. This is the feature behind gh-79: clients that omit `x-queue-type` and rely on the vhost default then resolve to the migrated queue type. The step is strictly best-effort and never fails the migration, which has already completed and restored normal operations by this point. Both an `{error, _}` return and any raised exception (update_metadata throws if its Mnesia transaction fails) are caught and logged; the migration result is unaffected.
Cover the new option in unit_SUITE: `quorum` and `classic` parse to the expected value, an absent option leaves the key unset, and an invalid value (e.g. `stream`, `bogus`) is rejected.
Add a global `--set-default-queue-type=quorum|classic` argument that any test can pass. The value rides along on whatever migration the test triggers via QueueMigrationClient, without changing the startMigration call signatures. When the option is set, the end-to-end test verifies the vhost default queue type was set to the requested value after a successful migration, reading it from the management API (which reports "undefined" when unset). The end-to-end Makefile target exercises this with `quorum`.
Describe the new option on the start endpoint (accepted values, that it overwrites the existing default, affects only newly declared or redeclared queues, and is best-effort so it never fails the migration), add a curl example, and note that the check endpoint accepts and validates it but a check does not modify the vhost.
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
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.
Summary
Adds an opt-in
set_default_queue_typeoption to the start-migration request. When set to"quorum"or"classic", the virtual host's default queue type is set to that value after a successful migration. When unset, the vhost default is left unchanged.This addresses the redeclaration failure described in #79: after migrating classic queues to quorum, clients that declare queues without an explicit
x-queue-typeand rely on the vhost default fail to redeclare the migrated queues with a 406. Setting the vhost default toquorummakes the broker injectx-queue-type: quoruminto such declarations so they match the migrated queue.Closes #79.
Decisions vs the issue's open questions
The issue left several choices open. This PR resolves them as follows:
set_default_queue_typeaccepting"quorum"or"classic", rather than the booleanset_default_queue_type_quorum. The string form is more flexible and any other value is rejected with a 400.It only affects newly declared or redeclared queues that omit
x-queue-type; it does not change any existing queue.Not included
The web UI checkbox/selector suggested in the issue is not part of this PR. This change covers the HTTP API, engine, and tests.
Implementation
rqm_mgmtparses and strictly validates the option (invalid value returns 400).rqmsets the vhost default viarabbit_vhost:update_metadata/3afterpost_migration_stats, using the plugin's existinginternal_useracting user, wrapped so no failure can propagate.Testing
quorum/classicaccepted, absent leaves it unset, invalid rejected).make ct-unitgreen.--set-default-queue-type=quorum|classicargument was added to the integration harness so any test can request it; the end-to-end test verifies the vhost default was set after migration, and the end-to-end Makefile target exercises it withquorum.quorumas requested.Stacked PR
This is stacked on #85 (
allow_message_ttl). The base branch isfeature/allow-message-ttl-gh-80; please merge #85 first. The diff shown here is only theset_default_queue_typecommits.