You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2026/04/24 02:41:50 UTC
[kudu-CR] WIP: [rpc] use flat multiset for the service queue container
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24230
Change subject: WIP: [rpc] use flat_multiset for the service queue container
......................................................................
WIP: [rpc] use flat_multiset for the service queue container
WIP:
* collect initial feedback
* explore using priority queue instead of flat_multiset since
we only care about first/last elements in the queue from
the operational standpoint
While troubleshooting lock contention under heavy load, there was
a lot of contention in libtcmalloc, and it resulted in longer
times spent by reactor and RPC worker threads waiting on the lock
in RPC service queue when invoking LifoServiceQueue::Put() and
LifoServiceQueue::BlockingGet() correspondingly. The culprit seemed
to be a thread that would acquire LifoServiceQueue::lock_ while
executing LifoServiceQueue::Put() and then tried to add an element
into the queue. The latter lead to memory allocation/de-allocation
via tcmalloc that went through the central free list (probably,
due to exhaustion of the thread cache's free list). Meanwhile,
the central free list's lock had already been acquired by some other
thread performing memory (de)allocation.
So, it seems to be a prudent move to switch to a container that
doesn't to any dynamic memory allocations while adding and removing
elements to/from the LIFO service queue. Using the
boost::container::flat_multiset backed by vector of pre-allocated
capacity seems a good option, even if inserting and erasing elements
take linear time to the elements with bigger keys than the inserted
one (not counting logarithmic lookup times which is the same-O-scale
for dictionary containers, such as std::multiset that was used for
the queue before this update).
I'd use STL std::flat_multiset instead of the Boost's counterpart,
but it's only available since the C++23 standard. We can switch to
std::flat_multiset when switching Kudu's code from the C++17 to the
C++23 or newer C++ standard in the future.
Running the dedicated perf benchmark test on a 8 CPU-core machine
reported ~1.33x improvement even without introducing contention
in libtcmalloc:
./bin/service_queue-test --num_producers=3 --num_consumers=4
before:
Reqs/sec: 1680414
User CPU per req: 2.13097us
Sys CPU per req: 1.50995us
Avg rpc queue length: 5.81
Avg idle workers: 0.124
after:
Reqs/sec: 2238155
User CPU per req: 1.76033us
Sys CPU per req: 1.08875us
Avg rpc queue length: 6.29
Avg idle workers: 0.226
The default settings of the test (num_producers=4, num_consumers=20)
do not seem optimal for 8 CPU-core machine since there was extra context
switching at least, but even in that case there was a slight ~1.09x
improvement:
./bin/service_queue-test
before:
Reqs/sec: 1266376
User CPU per req: 1.80034us
Sys CPU per req: 3.74912us
Avg rpc queue length: 0.868
Avg idle workers: 8.1
after:
Reqs/sec: 1374688
User CPU per req: 1.71822us
Sys CPU per req: 3.43678us
Avg rpc queue length: 1.772
Avg idle workers: 7.75
[1] https://www.boost.org/doc/libs/latest/doc/html/doxygen/boost_container_header_reference/classboost_1_1container_1_1flat__multiset.html
Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
---
M src/kudu/rpc/service_queue.cc
M src/kudu/rpc/service_queue.h
2 files changed, 11 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/24230/1
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 4: Verified+1
Build Successful
http://jenkins.kudu.apache.org/job/pre_commit/2522/ : SUCCESS
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2026 18:02:04 +0000
Gerrit-HasComments: No
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Apr 2026 05:10:34 +0000
Gerrit-HasComments: No
[kudu-CR] WIP: [rpc] use flat multiset for the service queue container
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: WIP: [rpc] use flat_multiset for the service queue container
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24230/1/src/kudu/rpc/service_queue.h
File src/kudu/rpc/service_queue.h:
http://gerrit.cloudera.org:8080/#/c/24230/1/src/kudu/rpc/service_queue.h@216
PS1, Line 216: QueueType queue_;
Consider using boost::priority_queue with boost::pool_alloc to improve performance and avoid memory allocations.
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Apr 2026 13:08:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Kudu Jenkins, Joe McDonnell, Michael Smith,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24230
to look at the new patch set (#3).
Change subject: [rpc] use min priority queue for service queue container
......................................................................
[rpc] use min priority queue for service queue container
A lot of contention in libtcmalloc might result in longer times spent
by reactor and RPC worker threads waiting on the lock in RPC service
queue when invoking LifoServiceQueue::{Put,BlockingGet}(). Per analysis
of captured stack traces, the culprit seemed to be a thread that would
acquire LifoServiceQueue::lock_ while executing LifoServiceQueue::Put()
to add an element into the queue. The latter lead to memory
(de)allocations via tcmalloc that went through the central free list
perhaps due to exhaustion of the thread cache's free list. Meanwhile,
the central free list's lock had already been acquired by some other
thread performing memory (de)allocation.
It's prudent to switch to a container which doesn't do any dynamic
memory allocations while adding and removing elements to/from the
service queue. Using a pre-allocated std::vector instance to maintain
a min priority queue turns to be a good option. Instead of using direct
API of the std::priority_queue container adaptor, std::push_stack() and
std::pop_stack() are used on top of the underlying array/vector to
handle an edge case to evict an element while replacing it with the
newly arrived one.
Various scenarios of the dedicated perf benchmark run on a 8 CPU-core
machine reported improvement even without artificially induced
contention in libtcmalloc. The service queue length was set to 200.
With larger service queue the performance boost was even higher for
scenarios where the queue often stays full.
KUDU_ALLOW_SLOW_TESTS=1 ./bin/service_queue-test --num_producers=P --num_consumers=C --max_queue_size=200
Balanced P=4/C=4 scenario reported ~1.56x improvement:
before:
Reqs/sec: 1288901
User CPU per req: 2.89116us
Sys CPU per req: 2.71363us
Avg rpc queue length: 102.518
Avg idle workers: 0
after:
Reqs/sec: 2022126
User CPU per req: 1.9856us
Sys CPU per req: 1.78929us
Avg rpc queue length: 50.964
Avg idle workers: 0.03
Balanced P=3/C=3 scenario reported ~1.30x improvement:
before:
Reqs/sec: 1470446
User CPU per req: 2.3612us
Sys CPU per req: 1.37508us
Avg rpc queue length: 76.184
Avg idle workers: 0
after:
Reqs/sec: 1917650
User CPU per req: 1.97289us
Sys CPU per req: 1.0409us
Avg rpc queue length: 145.424
Avg idle workers: 0
Skewed P=4/C=2 scenario reported ~1.76x improvement:
before:
Reqs/sec: 1053219
User CPU per req: 3.59901us
Sys CPU per req: 1.5802us
Avg rpc queue length: 194.678
Avg idle workers: 0
after:
Reqs/sec: 1862593
User CPU per req: 2.32343us
Sys CPU per req: 0.823388us
Avg rpc queue length: 194.37
Avg idle workers: 0
Skewed P=4/C=3 scenario reported ~1.55x improvement:
before:
Reqs/sec: 1185783
User CPU per req: 3.08805us
Sys CPU per req: 2.16397us
Avg rpc queue length: 191.486
Avg idle workers: 0
after:
Reqs/sec: 1834414
User CPU per req: 2.35371us
Sys CPU per req: 1.32012us
Avg rpc queue length: 192.042
Avg idle workers: 0
Skewed P=3/C=5 scenario reported ~1.29x improvement:
before:
Reqs/sec: 1622451
User CPU per req: 2.081us
Sys CPU per req: 1.86138us
Avg rpc queue length: 5.072
Avg idle workers: 0.104
after:
Reqs/sec: 2095612
User CPU per req: 1.65551us
Sys CPU per req: 1.39116us
Avg rpc queue length: 6.296
Avg idle workers: 0.176
Skewed P=2/C=5 scenario reported ~1.23x improvement:
before:
Reqs/sec: 1168078
User CPU per req: 1.83038us
Sys CPU per req: 1.94243us
Avg rpc queue length: 0.85
Avg idle workers: 0.858
after:
Reqs/sec: 1436187
User CPU per req: 1.57106us
Sys CPU per req: 1.56673us
Avg rpc queue length: 1.078
Avg idle workers: 0.878
Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
---
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_queue-test.cc
M src/kudu/rpc/service_queue.cc
M src/kudu/rpc/service_queue.h
4 files changed, 102 insertions(+), 80 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/24230/3
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24230/3/src/kudu/rpc/service_queue-test.cc
File src/kudu/rpc/service_queue-test.cc:
http://gerrit.cloudera.org:8080/#/c/24230/3/src/kudu/rpc/service_queue-test.cc@72
PS3, Line 72: inprogress.fetch_add(1, std::memory_order_relaxed);
> Maybe comment that relaxed is ok, because there expected to be a lock in qu
Done
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2026 17:16:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 2:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2518/
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Sun, 26 Apr 2026 14:00:35 +0000
Gerrit-HasComments: No
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
[rpc] use min priority queue for service queue container
A lot of contention in libtcmalloc might result in longer times spent
by reactor and RPC worker threads waiting on the lock in RPC service
queue when invoking LifoServiceQueue::{Put,BlockingGet}(). Per analysis
of captured stack traces, the culprit seemed to be a thread that would
acquire LifoServiceQueue::lock_ while executing LifoServiceQueue::Put()
to add an element into the queue. The latter lead to memory
(de)allocations via tcmalloc that went through the central free list
perhaps due to exhaustion of the thread cache's free list. Meanwhile,
the central free list's lock had already been acquired by some other
thread performing memory (de)allocation.
It's prudent to switch to a container which doesn't do any dynamic
memory allocations while adding and removing elements to/from the
service queue. Using a pre-allocated std::vector instance to maintain
a min priority queue turns to be a good option. Instead of using direct
API of the std::priority_queue container adaptor, std::push_stack() and
std::pop_stack() are used on top of the underlying array/vector to
handle an edge case to evict an element while replacing it with the
newly arrived one.
Various scenarios of the dedicated perf benchmark run on a 8 CPU-core
machine reported improvement even without artificially induced
contention in libtcmalloc. The service queue length was set to 200.
With larger service queue the performance boost was even higher for
scenarios where the queue often stays full.
KUDU_ALLOW_SLOW_TESTS=1 ./bin/service_queue-test --num_producers=P --num_consumers=C --max_queue_size=200
Balanced P=4/C=4 scenario reported ~1.56x improvement:
before:
Reqs/sec: 1288901
User CPU per req: 2.89116us
Sys CPU per req: 2.71363us
Avg rpc queue length: 102.518
Avg idle workers: 0
after:
Reqs/sec: 2022126
User CPU per req: 1.9856us
Sys CPU per req: 1.78929us
Avg rpc queue length: 50.964
Avg idle workers: 0.03
Balanced P=3/C=3 scenario reported ~1.30x improvement:
before:
Reqs/sec: 1470446
User CPU per req: 2.3612us
Sys CPU per req: 1.37508us
Avg rpc queue length: 76.184
Avg idle workers: 0
after:
Reqs/sec: 1917650
User CPU per req: 1.97289us
Sys CPU per req: 1.0409us
Avg rpc queue length: 145.424
Avg idle workers: 0
Skewed P=4/C=2 scenario reported ~1.76x improvement:
before:
Reqs/sec: 1053219
User CPU per req: 3.59901us
Sys CPU per req: 1.5802us
Avg rpc queue length: 194.678
Avg idle workers: 0
after:
Reqs/sec: 1862593
User CPU per req: 2.32343us
Sys CPU per req: 0.823388us
Avg rpc queue length: 194.37
Avg idle workers: 0
Skewed P=4/C=3 scenario reported ~1.55x improvement:
before:
Reqs/sec: 1185783
User CPU per req: 3.08805us
Sys CPU per req: 2.16397us
Avg rpc queue length: 191.486
Avg idle workers: 0
after:
Reqs/sec: 1834414
User CPU per req: 2.35371us
Sys CPU per req: 1.32012us
Avg rpc queue length: 192.042
Avg idle workers: 0
Skewed P=3/C=5 scenario reported ~1.29x improvement:
before:
Reqs/sec: 1622451
User CPU per req: 2.081us
Sys CPU per req: 1.86138us
Avg rpc queue length: 5.072
Avg idle workers: 0.104
after:
Reqs/sec: 2095612
User CPU per req: 1.65551us
Sys CPU per req: 1.39116us
Avg rpc queue length: 6.296
Avg idle workers: 0.176
Skewed P=2/C=5 scenario reported ~1.23x improvement:
before:
Reqs/sec: 1168078
User CPU per req: 1.83038us
Sys CPU per req: 1.94243us
Avg rpc queue length: 0.85
Avg idle workers: 0.858
after:
Reqs/sec: 1436187
User CPU per req: 1.57106us
Sys CPU per req: 1.56673us
Avg rpc queue length: 1.078
Avg idle workers: 0.878
Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Reviewed-on: http://gerrit.cloudera.org:8080/24230
Tested-by: Kudu Jenkins
Reviewed-by: Zoltan Martonka <zm...@cloudera.com>
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
---
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_queue-test.cc
M src/kudu/rpc/service_queue.cc
M src/kudu/rpc/service_queue.h
4 files changed, 117 insertions(+), 88 deletions(-)
Approvals:
Kudu Jenkins: Verified
Zoltan Martonka: Looks good to me, but someone else must approve
Abhishek Chennaka: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 3:
> (1 comment)
>
> Is it really worth not just using an std::priority_queue?
> By preallocating, we save ~8-10 small memory allocation per service
> startup?
> Am I missing something?
You are missing one more reason that I pointed at in the description.
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2026 13:53:39 +0000
Gerrit-HasComments: No
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2518/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Sun, 26 Apr 2026 14:58:34 +0000
Gerrit-HasComments: No
[kudu-CR] WIP: [rpc] use flat multiset for the service queue container
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: WIP: [rpc] use flat_multiset for the service queue container
......................................................................
Patch Set 1: Verified+1
Build Successful
http://jenkins.kudu.apache.org/job/pre_commit/2514/ : SUCCESS
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Apr 2026 04:40:25 +0000
Gerrit-HasComments: No
[kudu-CR] WIP: [rpc] use flat multiset for the service queue container
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: WIP: [rpc] use flat_multiset for the service queue container
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24230/1/src/kudu/rpc/service_queue.h
File src/kudu/rpc/service_queue.h:
http://gerrit.cloudera.org:8080/#/c/24230/1/src/kudu/rpc/service_queue.h@216
PS1, Line 216: QueueType queue_;
> Consider using boost::priority_queue with boost::pool_alloc to improve perf
There is an inherent limit on the size of the queue -- it's specified as a parameter for the constructor. So, it's always possible to pre-allocate the whole container upfront and don't bother about memory allocations/re-allocations after if using a proper container type. I guess this is the most effective and straightforward way of memory management here, and this changelist uses this strategy as of PS1.
As for the amount of allocated memory, that's just 8 bytes per element. Even with a huge queue of 1024 elements (we usually don't recommend having more than 256 in Kudu) it would be just 8KiB total. Do you think we should use boost::pool_alloc for such amount of pre-allocated memory?
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Apr 2026 01:47:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Kudu Jenkins, Joe McDonnell, Michael Smith,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24230
to look at the new patch set (#2).
Change subject: [rpc] use min priority queue for service queue container
......................................................................
[rpc] use min priority queue for service queue container
A lot of contention in libtcmalloc might resulted in longer times spent
by reactor and RPC worker threads waiting on the lock in RPC service
queue when invoking LifoServiceQueue::{Put,BlockingGet}(). Per analysis
of captured stack traces, the culprit seemed to be a thread that would
acquire LifoServiceQueue::lock_ while executing LifoServiceQueue::Put()
to add an element into the queue. The latter lead to memory
(de)allocations via tcmalloc that went through the central free list
perhaps due to exhaustion of the thread cache's free list. Meanwhile,
the central free list's lock had already been acquired by some other
thread performing memory (de)allocation.
It's prudent to switch to a container which doesn't do any dynamic
memory allocations while adding and removing elements to/from the
service queue. Using a pre-allocated std::vector instance to maintain
a min priority queue turns to be a good option. Instead of using direct
API of the std::priority_queue container adaptor, std::push_stack() and
std::pop_stack() are used on top of the underlying array/vector to
handle an edge case to evict an element while replacing it with the
newly arrived one.
Various scenarios of the dedicated perf benchmark run on a 8 CPU-core
machine reported improvement even without artificially induced
contention in libtcmalloc. The service queue length was set to 200.
With larger service queue the performance boost was even higher for
scenarios where the queue often stays full.
KUDU_ALLOW_SLOW_TESTS=1 ./bin/service_queue-test --num_producers=P --num_consumers=C --max_queue_size=200
Balanced P=4/C=4 scenario reported ~1.56x improvement:
before:
Reqs/sec: 1288901
User CPU per req: 2.89116us
Sys CPU per req: 2.71363us
Avg rpc queue length: 102.518
Avg idle workers: 0
after:
Reqs/sec: 2022126
User CPU per req: 1.9856us
Sys CPU per req: 1.78929us
Avg rpc queue length: 50.964
Avg idle workers: 0.03
Balanced P=3/C=3 scenario reported ~1.30x improvement:
before:
Reqs/sec: 1470446
User CPU per req: 2.3612us
Sys CPU per req: 1.37508us
Avg rpc queue length: 76.184
Avg idle workers: 0
after:
Reqs/sec: 1917650
User CPU per req: 1.97289us
Sys CPU per req: 1.0409us
Avg rpc queue length: 145.424
Avg idle workers: 0
Skewed P=4/C=2 scenario reported ~1.76x improvement:
before:
Reqs/sec: 1053219
User CPU per req: 3.59901us
Sys CPU per req: 1.5802us
Avg rpc queue length: 194.678
Avg idle workers: 0
after:
Reqs/sec: 1862593
User CPU per req: 2.32343us
Sys CPU per req: 0.823388us
Avg rpc queue length: 194.37
Avg idle workers: 0
Skewed P=4/C=3 scenario reported ~1.55x improvement:
before:
Reqs/sec: 1185783
User CPU per req: 3.08805us
Sys CPU per req: 2.16397us
Avg rpc queue length: 191.486
Avg idle workers: 0
after:
Reqs/sec: 1834414
User CPU per req: 2.35371us
Sys CPU per req: 1.32012us
Avg rpc queue length: 192.042
Avg idle workers: 0
Skewed P=3/C=5 scenario reported ~1.29x improvement:
before:
Reqs/sec: 1622451
User CPU per req: 2.081us
Sys CPU per req: 1.86138us
Avg rpc queue length: 5.072
Avg idle workers: 0.104
after:
Reqs/sec: 2095612
User CPU per req: 1.65551us
Sys CPU per req: 1.39116us
Avg rpc queue length: 6.296
Avg idle workers: 0.176
Skewed P=2/C=5 scenario reported ~1.23x improvement:
before:
Reqs/sec: 1168078
User CPU per req: 1.83038us
Sys CPU per req: 1.94243us
Avg rpc queue length: 0.85
Avg idle workers: 0.858
after:
Reqs/sec: 1436187
User CPU per req: 1.57106us
Sys CPU per req: 1.56673us
Avg rpc queue length: 1.078
Avg idle workers: 0.878
Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
---
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_queue-test.cc
M src/kudu/rpc/service_queue.cc
M src/kudu/rpc/service_queue.h
4 files changed, 102 insertions(+), 80 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/24230/2
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[kudu-CR] WIP: [rpc] use flat multiset for the service queue container
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: WIP: [rpc] use flat_multiset for the service queue container
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2514/
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 24 Apr 2026 02:41:58 +0000
Gerrit-HasComments: No
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Zoltan Martonka (Code Review)" <ge...@cloudera.org>.
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Apr 2026 06:13:19 +0000
Gerrit-HasComments: No
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2026 05:41:07 +0000
Gerrit-HasComments: No
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Zoltan Martonka, Kudu Jenkins, Joe McDonnell, Michael Smith,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24230
to look at the new patch set (#4).
Change subject: [rpc] use min priority queue for service queue container
......................................................................
[rpc] use min priority queue for service queue container
A lot of contention in libtcmalloc might result in longer times spent
by reactor and RPC worker threads waiting on the lock in RPC service
queue when invoking LifoServiceQueue::{Put,BlockingGet}(). Per analysis
of captured stack traces, the culprit seemed to be a thread that would
acquire LifoServiceQueue::lock_ while executing LifoServiceQueue::Put()
to add an element into the queue. The latter lead to memory
(de)allocations via tcmalloc that went through the central free list
perhaps due to exhaustion of the thread cache's free list. Meanwhile,
the central free list's lock had already been acquired by some other
thread performing memory (de)allocation.
It's prudent to switch to a container which doesn't do any dynamic
memory allocations while adding and removing elements to/from the
service queue. Using a pre-allocated std::vector instance to maintain
a min priority queue turns to be a good option. Instead of using direct
API of the std::priority_queue container adaptor, std::push_stack() and
std::pop_stack() are used on top of the underlying array/vector to
handle an edge case to evict an element while replacing it with the
newly arrived one.
Various scenarios of the dedicated perf benchmark run on a 8 CPU-core
machine reported improvement even without artificially induced
contention in libtcmalloc. The service queue length was set to 200.
With larger service queue the performance boost was even higher for
scenarios where the queue often stays full.
KUDU_ALLOW_SLOW_TESTS=1 ./bin/service_queue-test --num_producers=P --num_consumers=C --max_queue_size=200
Balanced P=4/C=4 scenario reported ~1.56x improvement:
before:
Reqs/sec: 1288901
User CPU per req: 2.89116us
Sys CPU per req: 2.71363us
Avg rpc queue length: 102.518
Avg idle workers: 0
after:
Reqs/sec: 2022126
User CPU per req: 1.9856us
Sys CPU per req: 1.78929us
Avg rpc queue length: 50.964
Avg idle workers: 0.03
Balanced P=3/C=3 scenario reported ~1.30x improvement:
before:
Reqs/sec: 1470446
User CPU per req: 2.3612us
Sys CPU per req: 1.37508us
Avg rpc queue length: 76.184
Avg idle workers: 0
after:
Reqs/sec: 1917650
User CPU per req: 1.97289us
Sys CPU per req: 1.0409us
Avg rpc queue length: 145.424
Avg idle workers: 0
Skewed P=4/C=2 scenario reported ~1.76x improvement:
before:
Reqs/sec: 1053219
User CPU per req: 3.59901us
Sys CPU per req: 1.5802us
Avg rpc queue length: 194.678
Avg idle workers: 0
after:
Reqs/sec: 1862593
User CPU per req: 2.32343us
Sys CPU per req: 0.823388us
Avg rpc queue length: 194.37
Avg idle workers: 0
Skewed P=4/C=3 scenario reported ~1.55x improvement:
before:
Reqs/sec: 1185783
User CPU per req: 3.08805us
Sys CPU per req: 2.16397us
Avg rpc queue length: 191.486
Avg idle workers: 0
after:
Reqs/sec: 1834414
User CPU per req: 2.35371us
Sys CPU per req: 1.32012us
Avg rpc queue length: 192.042
Avg idle workers: 0
Skewed P=3/C=5 scenario reported ~1.29x improvement:
before:
Reqs/sec: 1622451
User CPU per req: 2.081us
Sys CPU per req: 1.86138us
Avg rpc queue length: 5.072
Avg idle workers: 0.104
after:
Reqs/sec: 2095612
User CPU per req: 1.65551us
Sys CPU per req: 1.39116us
Avg rpc queue length: 6.296
Avg idle workers: 0.176
Skewed P=2/C=5 scenario reported ~1.23x improvement:
before:
Reqs/sec: 1168078
User CPU per req: 1.83038us
Sys CPU per req: 1.94243us
Avg rpc queue length: 0.85
Avg idle workers: 0.858
after:
Reqs/sec: 1436187
User CPU per req: 1.57106us
Sys CPU per req: 1.56673us
Avg rpc queue length: 1.078
Avg idle workers: 0.878
Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
---
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_queue-test.cc
M src/kudu/rpc/service_queue.cc
M src/kudu/rpc/service_queue.h
4 files changed, 117 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/24230/4
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 4:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2522/
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2026 17:16:46 +0000
Gerrit-HasComments: No
[kudu-CR] [rpc] use min priority queue for service queue container
Posted by "Zoltan Martonka (Code Review)" <ge...@cloudera.org>.
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24230 )
Change subject: [rpc] use min priority queue for service queue container
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
Is it really worth not just using an std::priority_queue?
By preallocating, we save ~8-10 small memory allocation per service startup?
Am I missing something?
http://gerrit.cloudera.org:8080/#/c/24230/3/src/kudu/rpc/service_queue-test.cc
File src/kudu/rpc/service_queue-test.cc:
http://gerrit.cloudera.org:8080/#/c/24230/3/src/kudu/rpc/service_queue-test.cc@72
PS3, Line 72: inprogress.fetch_add(1, std::memory_order_relaxed);
Maybe comment that relaxed is ok, because there expected to be a lock in queue->Put(), therefore the compiler/cpu can't move this after the put call.
Because for a possibly lock-free queue, relaxed would be unsufiicient.
--
To view, visit http://gerrit.cloudera.org:8080/24230
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d95828177ae6458b427ce43dd292877d03065cf
Gerrit-Change-Number: 24230
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2026 12:16:59 +0000
Gerrit-HasComments: Yes