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

Set the QoS on the channel that is created for the consumer #224

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

abompard
Copy link
Member

@abompard abompard commented Jul 29, 2020

According to the RabbitMQ docs, they changed the semantics of the global QoS parameter, and setting it to True as we've done does not enforce the prefetch limit on all channels in the connection but rather on all consumers in the channel. There is no way to have connection-wide limits with RabbitMQ.

Since we create a new channel for each consumer, we must set the QoS in this channel and not in the main channel, otherwise it'll always be limitless.

Fixes #223

According to [the RabbitMQ
docs](https://www.rabbitmq.com/consumer-prefetch.html), they changed the
semantics of the `global` QoS parameter, and setting it to `True` as
we've done does not enforce the prefetch limit on all channels in the
connection but rather on all consumers in the channel. There is no way
to have connection-wide limits with RabbitMQ.

Since we create a new channel for each consumer, we must set the QoS in
this channel and not in the main channel, otherwise it'll always be
limitless.

Fixes fedora-infra#223

Signed-off-by: Aurélien Bompard <aurelien@bompard.org>
@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2020

Codecov Report

Merging #224 into stable will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           stable     #224   +/-   ##
=======================================
  Coverage   95.68%   95.68%           
=======================================
  Files          13       13           
  Lines        1343     1344    +1     
  Branches      166      166           
=======================================
+ Hits         1285     1286    +1     
  Misses         43       43           
  Partials       15       15           
Impacted Files Coverage Δ
fedora_messaging/twisted/protocol.py 93.22% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3faef19...44417a5. Read the comment docs.

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.

The prefetch_count parameter is not applied
2 participants