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

POC Make all queues on Rabbitmq quorum queue when quorum option is en… #2065

Closed
wants to merge 2 commits into from

Conversation

Arsnael
Copy link
Contributor

@Arsnael Arsnael commented Feb 28, 2024

…abled

@Arsnael Arsnael self-assigned this Feb 28, 2024
Comment on lines +58 to 59
QueueArguments.Builder builder = configuration.workQueueArgumentsBuilder();
configuration.getQueueTTL().ifPresent(builder::queueTTL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are those options compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems not, good catch

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@quantranhong1999 quantranhong1999 left a comment

Choose a reason for hiding this comment

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

read it

Copy link
Contributor

@vttranlina vttranlina left a comment

Choose a reason for hiding this comment

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

read it

Those parameters are not compatible with quorum queues
.autoDelete(AUTO_DELETE)
.autoDelete(!AUTO_DELETE)
Copy link
Contributor

Choose a reason for hiding this comment

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

the key queue seems volatile and can accept message lost to me -> Maybe we do not need to apply quorum queue on key queues?

After James restarts, a key queue would not be consumed anymore. So I think we should keep autoDelete if possible to avoid flushing RabbitMQ with unused key queues.

Copy link
Contributor

Choose a reason for hiding this comment

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

But thinking more about that, not applying quorum queue to the key queues could again break James if a RabbitMQ node is down...

Copy link
Contributor

Choose a reason for hiding this comment

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

Likely we can write a task to clean up inactive RabbitMQ queues (no consumers)

Copy link
Contributor

Choose a reason for hiding this comment

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

@chibenwa
Copy link
Contributor

chibenwa commented Apr 8, 2024

the key queue seems volatile and can accept message lost to me -> Maybe we do not need to apply quorum queue on key queues?
After James restarts, a key queue would not be consumed anymore. So I think we should keep autoDelete if possible to avoid flushing RabbitMQ with unused key queues.

Do not Guess. Do not use the word maybe.

If needed experiment. How does this break things? Reject upon publish? Loose the notification? Are we able to recover connection loss afterward?

RabbitMQClusterTest might prove valuable...

Hmm normally key queues should be cleaned up by James upon James shutdown

How about "not clean shutdown?"

Likely we can write a task to clean up inactive RabbitMQ queues (no consumers)

In the best of the world I would prefer we do not come down to that...

Also keep in mind that RabbitMQ pubsub architecture (keys) could be replaced by redis. Do that work presently allows RabbitHA when using Redis on the pubsub infrastructure?

@Arsnael
Copy link
Contributor Author

Arsnael commented Apr 9, 2024

Work moved here => #2191

Closing this.

@Arsnael Arsnael closed this Apr 9, 2024
@quantranhong1999
Copy link
Contributor

How about "not clean shutdown?"

Queue TTL still can apply on quorum queues -> dangling key queues would be cleared by RabbitMQ.

I just checked: we already apply the queue TTL on the ephemeral queues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants