-
Notifications
You must be signed in to change notification settings - Fork 0
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
Lt 5705 rabbit mq broker add quorum queues support for nl queues #63
Conversation
…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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
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.