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

Lt 5705 rabbit mq broker add quorum queues support for nl queues #63

Conversation

tarurar
Copy link
Collaborator

@tarurar tarurar commented Sep 18, 2024

This change introduces a more secure approach to running the subscriber. Previously, a single channel was used to configure the queue and later to consume the queue. However, if an application level exception occurs during configuration, the channel is automatically closed. This may require a new channel to be created and queue configuration to continue before the queue is consumed, not to mention the risk to the subscriber who cannot consume on a closed channel, so the channel factory is now passed to the configurator instead of the channel itself.

This update is part of the Quorum Queue Migration Initiative.

atarutin added 2 commits September 18, 2024 17:09
…bscriber

this is safer approach since the original channel can be closed while configuring the queue due to application-level exceptions. So consumer channel shouldn't depend on configuration issues.
…ead of passing channel itself

(LT-5705): application-level exceptions can happen during channel configuration which leads to channel closure. In this case configurator might need to create another channel to proceed with configuration. Also this update separates subscription channel from configuration channel which is much safer.

_consumer = GetOrCreateConsumer(_channel);

var queueName = MessageReadStrategy.Configure(_settings, _channel);
var queueName = MessageReadStrategy.Configure(_settings, CreateConfiguratorChannel);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the configurator channel created after the consumer channel? It seems that the configurator channel is short-lived, and it would make more sense to run it first, before the long-lived consumer channel.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It, actually, doesn't make any difference.
Channel for subscriber and channel for configuration are different and independent. Channels are cheap objects and TTL can be up to connection's TTL.
Key point to understand here is that there is no channel configuration (except from QOS value for consumer channel). Channel is only a transport abstraction which is used to communicate with a broker. So called short-lived configuration channel is used to configure queue on broker side.
Weather I create consumer or configuration channel first doesn't change anything unless consumer starts listening. The listening itself should be started after queue configuration of course. And it is there, on line 177.
To sum it up: I can switch consumer and configuration channel creation, but this really doesn't change anything. Connections by default support up to 256 channels at the same time.

}

public IModel CreateModel()
{
LatestChannel = new FakeChannel();
return LatestChannel;
// Consumer channel is the one requested first by subscriber
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's better not to rely on the implementation details here, if possible ofc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
Lykke.RabbitMqBroker 36% 30% 906
Summary 36% (722 / 1979) 30% (182 / 610) 906

@tarurar tarurar merged commit da3c61e into master Sep 19, 2024
1 check passed
@tarurar tarurar deleted the LT-5705-rabbit-mq-broker-add-quorum-queues-support-for-nl-queues branch September 19, 2024 10:02
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.

2 participants