You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2020/04/12 05:34:14 UTC

[kudu-CR] rpc: avoid an extra epoll cycle when data can be sent immediately

Hello Andrew Wong,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/15716

to review the following change.


Change subject: rpc: avoid an extra epoll cycle when data can be sent immediately
......................................................................

rpc: avoid an extra epoll cycle when data can be sent immediately

Prior to this patch, when queueing outbound data in the reactor thread,
we would always enqueue the transfer, set the epoll watcher on the
socket to wait for the "writable" state, and then go back into the epoll
loop. In most cases, there is sufficient buffer space, so the next epoll
loop would return immediately. We'd then perform the write, see nothing
left in the transfer queue, and disable the "write" watcher again.

This whole dance would result in a sequence of epoll_ctl / epolL_wait /
send / epoll_ctl for every outbound message.

This patch changes the behavior so that, when we enqueue a transfer, if
the transfer queue was previously empty (ie we hadn't gotten blocked on
another transfer), we now try writing it immediately. Only if it fails
to fully write do we enable the epoll watching. This saves a pair of
epoll_ctl calls and an epoll_wait for both the request and response
sides of every RPC.

I tested this using a new benchmark I'm working on which sends 20480
synchronous RPCs from a single thread. I used 'perf trace' to count
syscalls:

Without the patch:
  Performance counter stats for 'build/latest/bin/rpc_stub-test
  --gtest_filter=*Trans* --benchmark_total_mb=40960
  --benchmark_method_pattern=fd*mmap*reuse*reuse)
  -benchmark-setup-pattern=all-same-*':

              81,924      syscalls:sys_enter_epoll_ctl  (4 per RPC)
             122,886      syscalls:sys_enter_epoll_wait (6 per RPC)

With the patch:
  Performance counter stats for 'build/latest/bin/rpc_stub-test
  --gtest_filter=*Trans* --benchmark_total_mb=40960
  --benchmark_method_pattern=fd*mmap*reuse*reuse)
  -benchmark-setup-pattern=all-same-*':

                  6      syscalls:sys_enter_epoll_ctl  (0 per RPC)
             81,927      syscalls:sys_enter_epoll_wait (4 per RPC)

I also benchmarked with:
  rpc-bench --gtest_filter=\*Calls -client-threads=1 \
    -server-reactors=1 --gtest_repeat=10 2>&1 | grep Reqs/sec

and ran a t-test on the results:

  	Welch Two Sample t-test

  data:  before and after
  t = -5.5671, df = 12.065, p-value = 0.00012
  alternative hypothesis: true difference in means is not equal to 0
  95 percent confidence interval:
   -2700.794 -1182.046
  sample estimates:
  mean of x mean of y
   25353.48  27294.90

so 95% confidence interval this improves throughput between 5 and 10%.

Change-Id: I30af102224d5db2cb526b4c2ae981d6e9defd82a
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
2 files changed, 23 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/15716/1
-- 
To view, visit http://gerrit.cloudera.org:8080/15716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I30af102224d5db2cb526b4c2ae981d6e9defd82a
Gerrit-Change-Number: 15716
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>