-
Notifications
You must be signed in to change notification settings - Fork 137
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
Configure consumer tag names/prefixes #265
Configure consumer tag names/prefixes #265
Conversation
@olleolleolle thoughts? This looks reasonable to me. |
Generate with | ||
|
||
0. `yard doc lib/hutch/config.rb` | ||
0. Copy the _Configuration_ section from `doc/Hutch/Config.html` here, with the anchor tags stripped. |
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.
Thank you!
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.
(This is you coming back to improve the README: 🎩 !)
The feature is an improvement! So many thumbs: 👍 The failing test is about supporting MarchHare + Bunny: JRuby support.
In order to get that working, perhaps we need to not use |
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.
Hm, MarchHare perhaps has an alternate way of expressing this?
|
||
def consumer_tag(queue) | ||
prefix = Hutch::Config[:consumer_tag_prefix] | ||
queue.channel.generate_consumer_tag(prefix) |
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.
So, MarchHare does not have the same interface:
https://github.com/ruby-amqp/march_hare/search?utf8=%E2%9C%93&q=generate_consumer_tag
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.
Ah, it does exist, just another way of expressing it: https://github.com/ruby-amqp/march_hare/pull/93/files
Oh I understand why |
@@ -45,9 +45,17 @@ | |||
end | |||
|
|||
it 'sets up a subscription' do | |||
expect(queue).to receive(:subscribe).with(manual_ack: true) | |||
expect(queue).to receive(:subscribe).with(consumer_tag: %r(^hutch\-.{36}$), manual_ack: true) |
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.
Do I need to double check AMQP/RabbitMQ spec for the maximum available length of the consumer tag name?
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.
That would be a cool touch.
(Then we could figure out ways to validate configuration options. Example addition to the Config options' DSL: max_length: 34
.)
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 "depends", it seems: https://www.rabbitmq.com/amqp-0-9-1-reference.html#basic.consume.consumer-tag it is a shortstr
, defined as "shortstr shortstr [short string]
"
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.
Seems like it's 255 bytes max: https://github.com/rabbitmq/rabbitmq-common/blob/rabbitmq_v3_6_0/codegen.py#L178-L179 ... I hope that's the same in 3.4.x as well :)
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.
So, now we know that the longest prefix that can be used is: 256 - 1 - 36 = 219 bytes.
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.
Ideally Hutch should loudly complain about prefixes that are longer, e.g. log an error and trim the prefix or fail to start.
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.
Opted for an "on-create" validation as validating the actual prefix length is weird (why 219 bytes?) and coupled to the implementation.
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.
Is the on-create validation sufficient/ok?
def unique_consumer_tag | ||
prefix = Hutch::Config[:consumer_tag_prefix] | ||
unique_part = SecureRandom.uuid | ||
"#{prefix}-#{unique_part}" |
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.
I ended up generating this myself. Quote from the commit:
Due to the interface differences between
bunny
(used for CRuby/MRI)
andmarch_hare
(used for JRuby), it's not possible to consistently use
the generator functionqueue.channel.generate_consumer_tag
for the
unique part of the consumer tags.
Instead we can generate the unique part ourselves without relying on
internals of these gems. UsingSecureRandom.uuid
seems like a good
idea.
Is this assumption valid? Is it okay to use SecureRandom
here?
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 is. SecureRandom
is available on JRuby and produces values of a known fixed length (36 bytes).
⌛️ I also need to test these with my local RabbitMQ install to ensure it's not breaking on some other integration problems. |
Allows us to configure the consumer names (tags) that are visible in RabbitMQ. Tagging with specific names is useful for us because we have over 100 queues and lots of applications communicating. If we aim to be easily identifiable, we can do these things: - define a routing key convention that refers to the application publishing the message - define a queue name convention that identifies the consumers - identify consumers by tag names Out of the above three we cannot do the last one. So this changeset enables that configuration :) Note: this changes the default tag name from `bunny` to `hutch`.
Due to the interface differences between `bunny` (used for CRuby/MRI) and `march_hare` (used for JRuby), it's not possible to consistently use the generator function `queue.channel.generate_consumer_tag` for the unique part of the consumer tags. Instead we can generate the unique part ourselves without relying on internals of these gems. Using `SecureRandom.uuid` seems like a good idea.
[According to the AMQP specification][amqp-spec] the field is a `shortstr`, which is [255 bytes long][codegen]. [amqp-spec]: https://www.rabbitmq.com/amqp-0-9-1-reference.html#basic.consume.consumer-tag [codegen]: https://github.com/rabbitmq/rabbitmq-common/blob/rabbitmq_v3_6_0/codegen.py#L178-L179
Consumer tags do not change and are rarely used in delivery handlers (probably never in Hutch).
… On 28 Nov 2016, at 13:49, Dávid Lantos ***@***.***> wrote:
@sldblog commented on this pull request.
In spec/hutch/worker_spec.rb:
> @@ -45,9 +45,17 @@
end
it 'sets up a subscription' do
- expect(queue).to receive(:subscribe).with(manual_ack: true)
+ expect(queue).to receive(:subscribe).with(consumer_tag: %r(^hutch\-.{36}$), manual_ack: true)
Is the on-create validation sufficient/ok?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@michaelklishin Does that mean it's okay to validate them when creating the subscriptions? |
It does to me. |
🎉 🙇 |
@michaelklishin Would it be possible to release this as 0.24? If it's blocked on something, can I help unblocking it? |
Yes. |
|
❤️ |
Allows us to configure the consumer names (tags) that are visible in RabbitMQ.
Tagging with specific names is useful for us because we have over 100 queues and lots of applications communicating. If we aim to be easily identifiable, we can do these things:
Out of the above three we cannot do the last one. So this changeset enables that configuration :)
How-to
Notes
The default tag name changes from
bunny
tohutch
with this change.Bonus
Explains on how to (still manually) update the generated configuration section in the readme.
Examples
Default (before the PR)
Default (after the PR)
Custom prefix