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/10 00:01:27 UTC
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Hello Mike Percy, Andrew Wong,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/15701
to review the following change.
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
client/tserver: add support for connecting over unix domain sockets
This adds new experiental flags -rpc_listen_on_unix_domain_socket and
-client_use_unix_domain_sockets. The former makes the RPC server bind to
a unix socket and advertise this to the kudu master as part of the TS
registration. The latter makes the client attempt to connect via a
domain socket when it sees such a socket path advertised.
I perf tested by scanning an int32 column from a table with 800M rows
and using 'perf stat -a -r10' to look at total CPU consumption across
the tserver and system. There's a fair amount of variability here due to
inconsistent scheduling to cores/numa nodes, but seems like the unix
socket on average is 10% faster or so in terms of total cycles.
TCP sockets:
Performance counter stats for 'system wide' (10 runs):
148,367.78 msec cpu-clock # 87.755 CPUs utilized ( +- 4.82% )
101,755 context-switches # 0.686 K/sec ( +- 9.03% )
866 cpu-migrations # 0.006 K/sec ( +- 6.42% )
21,440 page-faults # 0.145 K/sec ( +- 19.32% )
43,847,792,445 cycles # 0.296 GHz ( +- 3.77% ) (1.01%)
50,668,281,554 instructions # 1.16 insn per cycle ( +- 1.80% ) (1.11%)
7,676,337,185 branches # 51.739 M/sec ( +- 4.61% ) (0.85%)
69,634,718 branch-misses # 0.91% of all branches ( +- 4.72% ) (0.84%)
1.6907 +- 0.0811 seconds time elapsed ( +- 4.80% )
Unix sockets:
Performance counter stats for 'system wide' (10 runs):
136,877.86 msec cpu-clock # 87.638 CPUs utilized ( +- 2.67% )
77,376 context-switches # 0.565 K/sec ( +- 14.16% )
846 cpu-migrations # 0.006 K/sec ( +- 6.58% )
23,430 page-faults # 0.171 K/sec ( +- 39.77% )
39,106,012,185 cycles # 0.286 GHz ( +- 4.26% ) (0.99%)
48,957,283,894 instructions # 1.25 insn per cycle ( +- 2.24% ) (1.08%)
7,635,756,771 branches # 55.785 M/sec ( +- 3.54% ) (0.83%)
69,900,882 branch-misses # 0.92% of all branches ( +- 5.14% ) (0.82%)
1.5619 +- 0.0415 seconds time elapsed ( +- 2.66% )
Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
---
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/net/sockaddr.h
11 files changed, 99 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/15701/1
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Andrew Wong, Kudu Jenkins, Andrew Wong, Grant Henke, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15701
to look at the new patch set (#5).
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
client/tserver: add support for connecting over unix domain sockets
This adds new experiental flags -rpc_listen_on_unix_domain_socket and
-client_use_unix_domain_sockets. The former makes the RPC server bind to
a unix socket and advertise this to the kudu master as part of the TS
registration. The latter makes the client attempt to connect via a
domain socket when it sees such a socket path advertised.
Note that this makes one behavioral change even when those flags are not
enabled: we now consider any tablet server with a loopback IP to be
"local" (and thus a candidate for unix domain socket connection). This
mostly affects the MiniCluster where tablet servers register using
various IPs in the loopback range 127.0.0.0/8, and was necessary in
order to test unix socket connections from the client.
I perf tested by scanning an int32 column from a table with 800M rows
and using 'perf stat -a -r10' to look at total CPU consumption across
the tserver and system. There's a fair amount of variability here due to
inconsistent scheduling to cores/numa nodes, but seems like the unix
socket on average is 10% faster or so in terms of total cycles.
TCP sockets:
Performance counter stats for 'system wide' (10 runs):
148,367.78 msec cpu-clock # 87.755 CPUs utilized ( +- 4.82% )
101,755 context-switches # 0.686 K/sec ( +- 9.03% )
866 cpu-migrations # 0.006 K/sec ( +- 6.42% )
21,440 page-faults # 0.145 K/sec ( +- 19.32% )
43,847,792,445 cycles # 0.296 GHz ( +- 3.77% ) (1.01%)
50,668,281,554 instructions # 1.16 insn per cycle ( +- 1.80% ) (1.11%)
7,676,337,185 branches # 51.739 M/sec ( +- 4.61% ) (0.85%)
69,634,718 branch-misses # 0.91% of all branches ( +- 4.72% ) (0.84%)
1.6907 +- 0.0811 seconds time elapsed ( +- 4.80% )
Unix sockets:
Performance counter stats for 'system wide' (10 runs):
136,877.86 msec cpu-clock # 87.638 CPUs utilized ( +- 2.67% )
77,376 context-switches # 0.565 K/sec ( +- 14.16% )
846 cpu-migrations # 0.006 K/sec ( +- 6.58% )
23,430 page-faults # 0.171 K/sec ( +- 39.77% )
39,106,012,185 cycles # 0.286 GHz ( +- 4.26% ) (0.99%)
48,957,283,894 instructions # 1.25 insn per cycle ( +- 2.24% ) (1.08%)
7,635,756,771 branches # 55.785 M/sec ( +- 3.54% ) (0.83%)
69,900,882 branch-misses # 0.92% of all branches ( +- 5.14% ) (0.82%)
1.5619 +- 0.0415 seconds time elapsed ( +- 2.66% )
Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
15 files changed, 163 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/15701/5
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15701 )
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 16 Apr 2020 07:09:24 +0000
Gerrit-HasComments: No
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15701 )
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
client/tserver: add support for connecting over unix domain sockets
This adds new experiental flags -rpc_listen_on_unix_domain_socket and
-client_use_unix_domain_sockets. The former makes the RPC server bind to
a unix socket and advertise this to the kudu master as part of the TS
registration. The latter makes the client attempt to connect via a
domain socket when it sees such a socket path advertised.
Note that this makes one behavioral change even when those flags are not
enabled: we now consider any tablet server with a loopback IP to be
"local" (and thus a candidate for unix domain socket connection). This
mostly affects the MiniCluster where tablet servers register using
various IPs in the loopback range 127.0.0.0/8, and was necessary in
order to test unix socket connections from the client.
I perf tested by scanning an int32 column from a table with 800M rows
and using 'perf stat -a -r10' to look at total CPU consumption across
the tserver and system. There's a fair amount of variability here due to
inconsistent scheduling to cores/numa nodes, but seems like the unix
socket on average is 10% faster or so in terms of total cycles.
TCP sockets:
Performance counter stats for 'system wide' (10 runs):
148,367.78 msec cpu-clock # 87.755 CPUs utilized ( +- 4.82% )
101,755 context-switches # 0.686 K/sec ( +- 9.03% )
866 cpu-migrations # 0.006 K/sec ( +- 6.42% )
21,440 page-faults # 0.145 K/sec ( +- 19.32% )
43,847,792,445 cycles # 0.296 GHz ( +- 3.77% ) (1.01%)
50,668,281,554 instructions # 1.16 insn per cycle ( +- 1.80% ) (1.11%)
7,676,337,185 branches # 51.739 M/sec ( +- 4.61% ) (0.85%)
69,634,718 branch-misses # 0.91% of all branches ( +- 4.72% ) (0.84%)
1.6907 +- 0.0811 seconds time elapsed ( +- 4.80% )
Unix sockets:
Performance counter stats for 'system wide' (10 runs):
136,877.86 msec cpu-clock # 87.638 CPUs utilized ( +- 2.67% )
77,376 context-switches # 0.565 K/sec ( +- 14.16% )
846 cpu-migrations # 0.006 K/sec ( +- 6.58% )
23,430 page-faults # 0.171 K/sec ( +- 39.77% )
39,106,012,185 cycles # 0.286 GHz ( +- 4.26% ) (0.99%)
48,957,283,894 instructions # 1.25 insn per cycle ( +- 2.24% ) (1.08%)
7,635,756,771 branches # 55.785 M/sec ( +- 3.54% ) (0.83%)
69,900,882 branch-misses # 0.92% of all branches ( +- 5.14% ) (0.82%)
1.5619 +- 0.0415 seconds time elapsed ( +- 2.66% )
Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Reviewed-on: http://gerrit.cloudera.org:8080/15701
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
15 files changed, 163 insertions(+), 9 deletions(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Andrew Wong, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15701
to look at the new patch set (#2).
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
client/tserver: add support for connecting over unix domain sockets
This adds new experiental flags -rpc_listen_on_unix_domain_socket and
-client_use_unix_domain_sockets. The former makes the RPC server bind to
a unix socket and advertise this to the kudu master as part of the TS
registration. The latter makes the client attempt to connect via a
domain socket when it sees such a socket path advertised.
I perf tested by scanning an int32 column from a table with 800M rows
and using 'perf stat -a -r10' to look at total CPU consumption across
the tserver and system. There's a fair amount of variability here due to
inconsistent scheduling to cores/numa nodes, but seems like the unix
socket on average is 10% faster or so in terms of total cycles.
TCP sockets:
Performance counter stats for 'system wide' (10 runs):
148,367.78 msec cpu-clock # 87.755 CPUs utilized ( +- 4.82% )
101,755 context-switches # 0.686 K/sec ( +- 9.03% )
866 cpu-migrations # 0.006 K/sec ( +- 6.42% )
21,440 page-faults # 0.145 K/sec ( +- 19.32% )
43,847,792,445 cycles # 0.296 GHz ( +- 3.77% ) (1.01%)
50,668,281,554 instructions # 1.16 insn per cycle ( +- 1.80% ) (1.11%)
7,676,337,185 branches # 51.739 M/sec ( +- 4.61% ) (0.85%)
69,634,718 branch-misses # 0.91% of all branches ( +- 4.72% ) (0.84%)
1.6907 +- 0.0811 seconds time elapsed ( +- 4.80% )
Unix sockets:
Performance counter stats for 'system wide' (10 runs):
136,877.86 msec cpu-clock # 87.638 CPUs utilized ( +- 2.67% )
77,376 context-switches # 0.565 K/sec ( +- 14.16% )
846 cpu-migrations # 0.006 K/sec ( +- 6.58% )
23,430 page-faults # 0.171 K/sec ( +- 39.77% )
39,106,012,185 cycles # 0.286 GHz ( +- 4.26% ) (0.99%)
48,957,283,894 instructions # 1.25 insn per cycle ( +- 2.24% ) (1.08%)
7,635,756,771 branches # 55.785 M/sec ( +- 3.54% ) (0.83%)
69,900,882 branch-misses # 0.92% of all branches ( +- 5.14% ) (0.82%)
1.5619 +- 0.0415 seconds time elapsed ( +- 2.66% )
Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
---
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/net/sockaddr.h
11 files changed, 101 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/15701/2
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Andrew Wong, Kudu Jenkins, Andrew Wong, Grant Henke, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15701
to look at the new patch set (#4).
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
client/tserver: add support for connecting over unix domain sockets
This adds new experiental flags -rpc_listen_on_unix_domain_socket and
-client_use_unix_domain_sockets. The former makes the RPC server bind to
a unix socket and advertise this to the kudu master as part of the TS
registration. The latter makes the client attempt to connect via a
domain socket when it sees such a socket path advertised.
Note that this makes one behavioral change even when those flags are not
enabled: we now consider any tablet server with a loopback IP to be
"local" (and thus a candidate for unix domain socket connection). This
mostly affects the MiniCluster where tablet servers register using
various IPs in the loopback range 127.0.0.0/8, and was necessary in
order to test unix socket connections from the client.
I perf tested by scanning an int32 column from a table with 800M rows
and using 'perf stat -a -r10' to look at total CPU consumption across
the tserver and system. There's a fair amount of variability here due to
inconsistent scheduling to cores/numa nodes, but seems like the unix
socket on average is 10% faster or so in terms of total cycles.
TCP sockets:
Performance counter stats for 'system wide' (10 runs):
148,367.78 msec cpu-clock # 87.755 CPUs utilized ( +- 4.82% )
101,755 context-switches # 0.686 K/sec ( +- 9.03% )
866 cpu-migrations # 0.006 K/sec ( +- 6.42% )
21,440 page-faults # 0.145 K/sec ( +- 19.32% )
43,847,792,445 cycles # 0.296 GHz ( +- 3.77% ) (1.01%)
50,668,281,554 instructions # 1.16 insn per cycle ( +- 1.80% ) (1.11%)
7,676,337,185 branches # 51.739 M/sec ( +- 4.61% ) (0.85%)
69,634,718 branch-misses # 0.91% of all branches ( +- 4.72% ) (0.84%)
1.6907 +- 0.0811 seconds time elapsed ( +- 4.80% )
Unix sockets:
Performance counter stats for 'system wide' (10 runs):
136,877.86 msec cpu-clock # 87.638 CPUs utilized ( +- 2.67% )
77,376 context-switches # 0.565 K/sec ( +- 14.16% )
846 cpu-migrations # 0.006 K/sec ( +- 6.58% )
23,430 page-faults # 0.171 K/sec ( +- 39.77% )
39,106,012,185 cycles # 0.286 GHz ( +- 4.26% ) (0.99%)
48,957,283,894 instructions # 1.25 insn per cycle ( +- 2.24% ) (1.08%)
7,635,756,771 branches # 55.785 M/sec ( +- 3.54% ) (0.83%)
69,900,882 branch-misses # 0.92% of all branches ( +- 5.14% ) (0.82%)
1.5619 +- 0.0415 seconds time elapsed ( +- 2.66% )
Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
15 files changed, 164 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/15701/4
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15701 )
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/15701/2//COMMIT_MSG
Commit Message:
PS2:
Do you think it's worth adding a test to enable these in a mini cluster and client?
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.h@128
PS2, Line 128: boost::optional<std::string> unix_domain_socket_path_;
nit: should comment that we need to check that we're local before using this. Or only set this if we're local.
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.cc@83
PS2, Line 83: thta
nit: typo
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.cc@151
PS2, Line 151: << "Tablet server " << hp.ToString() << "(" << uuid_ << ") "
: << " reported an invalid unix domain socket path " << *unix_domain_socket_path_;
nit: extra space after the parens. Prefer Substitute() for readability? Same below
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.cc@203
PS2, Line 203: // Don't add unix domain sockets to the list of HostPorts.
: if (!addr.is_ip()) continue;
Do you think we'll ever want to have master-client transfer over domain sockets? Maybe consider a version of this that fills in parts of a ServerRegistrationPB
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.proto@101
PS2, Line 101: an
nit: capitalize
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/master/master.proto@436
PS2, Line 436: an
nit: capitalize
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/rpc_server.cc
File src/kudu/server/rpc_server.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/rpc_server.cc@178
PS2, Line 178: if (server_state_ != INITIALIZED) {
: return Status::IllegalState("must add bound addresses between Init() and Bind()");
: }
nit: if this is a programmer error, consider CHECK_NE?
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/server_base.cc@541
PS2, Line 541: CHECK_OK(addr.ParseUnixDomainPath(
: Substitute("@kudu-$0", fs_manager_->uuid())));
: CHECK_OK(rpc_server_->AddBindAddress(addr));
Why not RETURN_NOT_OK for these?
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Sun, 12 Apr 2020 08:20:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15701 )
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
Patch Set 3:
(9 comments)
> Patch Set 3:
>
> Client in this context could be the master or API client, right?
>
Client here is always the API client. The master doesn't use the src/client/* stuff to talk to tablet servers (just the underlying RPC client).
> API client (Impala for instance) is more likely to be running and communicating with the tablet server on the same machine, right?
Yea, it's common for clients to be on the same machine as the tablet server (Impala schedules for locality, for example, as does Spark/Presto/whatever)
http://gerrit.cloudera.org:8080/#/c/15701/2//COMMIT_MSG
Commit Message:
PS2:
> Do you think it's worth adding a test to enable these in a mini cluster and
Done
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.h@128
PS2, Line 128: boost::optional<std::string> unix_domain_socket_path_;
> nit: should comment that we need to check that we're local before using thi
Done
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.cc@83
PS2, Line 83: thta
> nit: typo
Done
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/client/meta_cache.cc@151
PS2, Line 151: << "Tablet server " << hp.ToString() << "(" << uuid_ << ") "
: << " reported an invalid unix domain socket path " << *unix_domain_socket_path_;
> nit: extra space after the parens. Prefer Substitute() for readability? Sam
Done
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.cc@203
PS2, Line 203: if (!addr.is_ip()) continue;
: HostPortPB* pb = pbs->Add();
> Do you think we'll ever want to have master-client transfer over domain soc
I don't think there's much benefit in that -- the master is rarely collocated with clients and the amount of data transfer is pretty low.
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/common/wire_protocol.proto@101
PS2, Line 101: an
> nit: capitalize
Done
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/master/master.proto@436
PS2, Line 436: an
> nit: capitalize
Done
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/rpc_server.cc
File src/kudu/server/rpc_server.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/rpc_server.cc@178
PS2, Line 178: if (server_state_ != INITIALIZED) {
: return Status::IllegalState("must add bound addresses between Init() and Bind()");
: }
> nit: if this is a programmer error, consider CHECK_NE?
Done
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/15701/2/src/kudu/server/server_base.cc@541
PS2, Line 541: CHECK_OK(rpc_server_->AddBindAddress(addr));
: }
:
> Why not RETURN_NOT_OK for these?
Done
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 15 Apr 2020 17:32:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] client/tserver: add support for connecting over unix domain sockets
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15701 )
Change subject: client/tserver: add support for connecting over unix domain sockets
......................................................................
Patch Set 3:
Client in this context could be the master or API client, right?
API client (Impala for instance) is more likely to be running and communicating with the tablet server on the same machine, right?
--
To view, visit http://gerrit.cloudera.org:8080/15701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c390b4209ac7e08cd45239c49499fb0b96405d0
Gerrit-Change-Number: 15701
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 15 Apr 2020 01:20:07 +0000
Gerrit-HasComments: No