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

Avoid ETS lookup if no extra_bcc queue set #4787

Merged
merged 1 commit into from May 11, 2022
Merged

Conversation

ansd
Copy link
Member

@ansd ansd commented May 10, 2022

A queue (Q1) can have an extra_bcc queue (Q2).
Whenever a message is routed to Q1, it must also be routed to Q2.

Commit fc2d37e (in 3.10.x, but not in 3.9.x) puts the logic to determine extra_bcc queues into rabbit_exchange:route/2.
That is functionally correct because it ensures that messages being dead lettered to target queues will also route to the target queues' extra_bcc queues. For every message being routed, that commit uses ets:lookup/2 just to check for an extra_bcc queue (blue rectangle D below).

Technically, that commit is not a regression because it does not slow down the common case where a message is routed to a single target queue because before that commit rabbit_channel:deliver_to_queues/3 used two ets:lookup/2 calls (blue rectangles A and B below).

However we can do better by avoiding the ets:lookup/2 for the common case where there is no extra_bcc queue set.

One option is to use ets:lookup_element/3 to only fetch the queue options field as demonstrated here.

A better option (implemented in this PR) is determining whether to send to an extra_bcc queue in the rabbit_channel and in the at-most and at-least once dead lettering modules where the queue records have already been looked up.

Performance

Single node RabbitMQ cluster and latest Java perf-test client running both on the same Intel NUC11 with 8 CPUs, 32GB RAM, Ubuntu 22.04, Erlang OTP 25.0-rc3.

RabbitMQ server started via

make run-broker LEAVE_PLUGINS_DISABLED=1 RABBITMQ_CONFIG_FILE="advanced.config" RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS="+JPperf true +S 4"
advanced.config
[
 {rabbit,[
  {vm_memory_high_watermark,{absolute,12_000_000_000}},
  {credit_flow_default_credit, {1600, 800}}
 ]}
].

2 publishers sending to a stream:

-x 2 -y 0 -qa x-queue-type=stream -ad false -f persistent -u some-long-queue-name-to-copy-more-memory-from-ets -z 15

Sending rate avg (msgs/s):
3.9.17: 252k - 260k
3.10.1: 250k - 262k
PR: 259k - 273k

CPU Flame Graphs

3.9.17
A is this line
B is this line
3-9-17

3.10.1
C is this line
D is this line
3-10-1

This PR
No additional ETS lookup and therefore faster routing.
PR

@ansd ansd mentioned this pull request May 10, 2022
1 task
@ansd ansd changed the title WIP Avoid ETS lookup if not extra_bcc queue set WIP Avoid ETS lookup if no extra_bcc queue set May 10, 2022
@ansd ansd force-pushed the extra-bcc branch 3 times, most recently from 6bab52c to 652e4f5 Compare May 10, 2022 21:58
A queue (Q1) can have an extra_bcc queue (Q2).
Whenever a message is routed to Q1, it must also be routed to Q2.

Commit fc2d37e
puts the logic to determine extra_bcc queues into
rabbit_exchange:route/2.
That is functionally correct because it ensures that messages being dead
lettered to target queues will also route to the target queues'
extra_bcc queues.
For every message being routed, that commit uses ets:lookup/2
just to check for an extra_bcc queue.

(Technically, that commit is not a regression because it does not slow
down the common case where a message is routed to a single target queue
because before that commit rabbit_channel:deliver_to_queues/3
used two ets:lookup/2 calls.)

However we can do better by avoiding the ets:lookup/2 for the common
case where there is no extra_bcc queue set.

One option is to use ets:lookup_element/3 to only fetch the queue
'options' field.

A better option (implemented in this commit) is determining whether to
send to an extra_bcc queue in the rabbit_channel and in the at-most
and at-least once dead lettering modules where the queue records
are already looked up.

This commit speeds up sending throughput by a few thousand messages per
second.
@ansd ansd changed the title WIP Avoid ETS lookup if no extra_bcc queue set Avoid ETS lookup if no extra_bcc queue set May 11, 2022
@ansd ansd marked this pull request as ready for review May 11, 2022 09:11
@michaelklishin michaelklishin merged commit 5c6ecdc into master May 11, 2022
@michaelklishin michaelklishin deleted the extra-bcc branch May 11, 2022 13:14
@michaelklishin michaelklishin added this to the 3.10.2 milestone May 11, 2022
michaelklishin added a commit that referenced this pull request May 11, 2022
Avoid ETS lookup if no extra_bcc queue set (backport #4787)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants