Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup shovels deleted from another node #11101

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

mkuratczyk
Copy link
Contributor

There are two ways to delete a shovel - rabbitmqctl delete_shovel and rabbitmqctl clear_parameter. The former works correctly (to the extent I tried), the latter doesn't clean up the shovel status correctly when the shovel is deleted from a another node. The shovel process is actually stopped, but it's not removed from the ETS table and is therefore returned in rabbitmqctl shovel_status, Management UI, etc.

The problems was that rabbit_shovel_status:remove(Name) was called on the node where the command was executed, rather than on the node running the shovel. This PR addresses this by calling this function from the shovel (worker) process itself, upon termination.

To reproduce the problem:

  1. start a cluster without this PR
  2. declare the queues, for example: perf-test -qq -u src -C 1; perf-test -qq -u dst -C 1
  3. declare the shovel:
rabbitmqctl -n rabbit-1 set_parameter shovel test '{"ack-mode":"on-confirm","dest-add-forward-headers":false,"dest-protocol":"amqp091","dest-queue":"dst","dest-uri":"amqp://","src-delete-after":"never","src-protocol":"amqp091","src-queue":"src","src-uri":"amqp://"}'
  1. delete the shovel by clearing the parameter from another node:
rabbitmqctl -n rabbit-2 clear_parameter shovel test
  1. You will see rabbitmqctl -n rabbit-1 shovel_status still reporting this shovel as running, even though it doesn't exist at all (Management UI also shows this shovel)

With this PR, the last command should return an empty result.

Copy link
Member

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how minimalistic the solution ended up being (compared to some alternative ideas suggested at first) 👍

@michaelklishin michaelklishin merged commit 97b26a8 into main Apr 26, 2024
16 checks passed
@michaelklishin michaelklishin deleted the md-shovel-status-cleanup branch April 26, 2024 23:47
michaelklishin added a commit that referenced this pull request Apr 27, 2024
Cleanup shovels deleted from another node (backport #11101)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants