Skip to content

Remove deprecated mirrored queue majority check #1734

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

Closed
Zerpet opened this issue Sep 25, 2024 · 7 comments · Fixed by #1740
Closed

Remove deprecated mirrored queue majority check #1734

Zerpet opened this issue Sep 25, 2024 · 7 comments · Fixed by #1740
Milestone

Comments

@Zerpet
Copy link
Member

Zerpet commented Sep 25, 2024

fmt.Sprintf(" rabbitmq-upgrade await_online_quorum_plus_one -t %d &&"+
" rabbitmq-upgrade await_online_synchronized_mirror -t %d &&"+
" rabbitmq-upgrade drain -t %d",

The command rabbitmq-upgrade await_online_synchronized_mirror is removed in 4.0. In order to support 4.0, we need to remove this check from the pre-stop hook, otherwise, the hook will always fail, and Kubernetes will ungracefully kill the Pod.

From: https://kubernetes.io/docs/concepts/containers/container-lifecycle-hooks/#hook-handler-execution

If either a PostStart or PreStop hook fails, it kills the Container.

@Zerpet Zerpet added this to the v2.11.0 milestone Sep 25, 2024
@Zerpet
Copy link
Member Author

Zerpet commented Sep 25, 2024

@mkuratczyk @MirahImage this issue is a bit awkward, because nothing prevents our users from putting rabbitmq 4.0 in the spec.image. Simply removing the check is possible, but then we take away that safety check from 3.x versions.

Arguably, mirror queues are a deprecated feature, and users should be moving from it. This may be an "incentive" to move out from CMQ.

What do you think?

@mkuratczyk
Copy link
Collaborator

To be honest I thought we made this command a no-op in RMQ, rather than removing it. That would make the transition much easier. We can always put it back as a no-op if we want. Alternatively, something like rabbitmq-update await_online_synchronized_mirror || true

@jcamu
Copy link

jcamu commented Oct 3, 2024

It means for now, we are not able to use the cluster-operator to move from 3.13.7 to 4.X RabbitMQ version ?

@mkuratczyk
Copy link
Collaborator

I don't think it's ungracefully - I still see SIGTERM received - shutting down in RabbitMQ logs, so we just don't use the maintenance mode. Not great, but not terrible either.

@Zerpet
Copy link
Member Author

Zerpet commented Oct 7, 2024

It means for now, we are not able to use the cluster-operator to move from 3.13.7 to 4.X RabbitMQ version ?

Not at all. You will be able to move from 3.13.x to 4.0.x. Everything will continue to work. By removing this check, the Pod will not halt shut down if there are unsynchronised mirror, which will cause some unavailability of CMQ. We will continue to drain the node, alleviating this because the node drain moves queue leaders to other nodes.

@Zerpet
Copy link
Member Author

Zerpet commented Oct 7, 2024

To be honest I thought we made this command a no-op in RMQ, rather than removing it. That would make the transition much easier. We can always put it back as a no-op if we want. Alternatively, something like rabbitmq-update await_online_synchronized_mirror || true

The more I think about it, the more I lean towards: mirror queues are a deprecated feature, and users should be moving from it. This may be an "incentive" to move out from CMQ

We've been overly generous with CMQ support, given plenty of notices and time to migrate to QQs.

@mkuratczyk
Copy link
Collaborator

I've already submitted a PR that fixes this while still maintaining backwards compatibility with CMQs. Given how trivial it is, I don't mind solving the problem this way
#1740

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 a pull request may close this issue.

3 participants