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 2020/04/28 00:50:29 UTC

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15828


Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................

[master-test] added GetTableSchema() microbenchmarks

Added a couple of GetTableSchema() microbenchmark tests.  These are to
gauge the performance of GetTableSchema() RPCs and the direct calls
to the catalog manager with authz enabled and disabled.

Results for a debug build:
  authz disabled: completed 27309 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 24699 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 548249 GetTableSchema calls in 5.000s
  authz enabled: completed 141795 GetTableSchema calls in 5.000s

Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
---
M src/kudu/master/master-test.cc
1 file changed, 141 insertions(+), 2 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#5).

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................

[master-test] added GetTableSchema() microbenchmarks

Added a couple of GetTableSchema() microbenchmark tests.  These are to
gauge the performance of GetTableSchema() RPCs and the direct calls
to the catalog manager with authz enabled and disabled.

Results for a RELEASE build:
  authz disabled: completed 208055 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 205599 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 5715530 GetTableSchema function calls in 5.000s
  authz enabled: completed 306410 GetTableSchema function calls in 5.000s

Results for a DEBUG build:
  authz disabled: completed 27309 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 24699 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 548249 GetTableSchema calls in 5.000s
  authz enabled: completed 141795 GetTableSchema calls in 5.000s

Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
---
M src/kudu/master/master-test.cc
1 file changed, 161 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15828/5
-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1084
PS1, Line 1084: FLAGS_rpc_service_queue_length
Probably, this should be min(FLAGS_rpc_service_queue_length, the number of CPU cores)


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1137
PS1, Line 1137: 200
Probably, this should depend on the number of CPU cores.



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 28 Apr 2020 00:59:29 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................

[master-test] added GetTableSchema() microbenchmarks

Added a couple of GetTableSchema() microbenchmark tests.  These are to
gauge the performance of GetTableSchema() RPCs and the direct calls
to the catalog manager with authz enabled and disabled.

Results for a RELEASE build:
  authz disabled: completed 208055 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 205599 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 5715530 GetTableSchema function calls in 5.000s
  authz enabled: completed 306410 GetTableSchema function calls in 5.000s

Results for a DEBUG build:
  authz disabled: completed 27309 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 24699 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 548249 GetTableSchema calls in 5.000s
  authz enabled: completed 141795 GetTableSchema calls in 5.000s

Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
---
M src/kudu/master/master-test.cc
1 file changed, 159 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15828/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 5: Verified+1

Unrelated LSAN warning:

 ==14215==ERROR: LeakSanitizer: detected memory leaks                            
                                                                                
Direct leak of 2184 byte(s) in 1 object(s) allocated from:                      
    #0 0x554e61 in malloc /home/jenkins-slave/workspace/kudu-master/0/thirdparty/src/llvm-9.0.0.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:145
    #1 0x7f81f9529db2 in CRYPTO_malloc (/lib/x86_64-linux-gnu/libcrypto.so.1.0.0+0x5fdb2)


-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Sun, 03 May 2020 21:03:33 +0000
Gerrit-HasComments: No

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG@13
PS1, Line 13: debug
Are such wide disparities there in release mode as well?


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1110
PS1, Line 1110: if (resp.has_error()) {
              :           LOG(WARNING) << SecureDebugString(resp);
              :           continue;
              :         }
Do you think it's worth failing this test on error? That way we can more easily not use bogus results?


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1139
PS1, Line 1139:   const bool supports_authz = GetParam();
              :   FLAGS_master_support_authz_tokens = supports_authz;
              : 
              :   for (auto idx = 0; idx < kNumTables; ++idx) {
              :     auto table_name = Substitute("testtb_$0", idx);
              :     EXPECT_OK(CreateTable(table_name, kTableSchema));
              :   }
nit: consider putting these in some common SetUp()?


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1180
PS1, Line 1180:   auto stopper_cleanup = MakeScopedCleanup([&]() {
              :     for (auto& t : caller_threads) {
              :       t.join();
              :     }
              :   });
nit: just FYI, you can use this too if you'd like, especially if you don't need to cancel the cleanup -- may be slightly more ergonomic:

 SCOPED_CLEANUP({ /* do stuff */ })


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1191
PS1, Line 1191: calls
nit: maybe "function calls"? an RPC is also a "call", so it wasn't quite clear to me at first what the difference was from the commit message.



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 28 Apr 2020 01:13:44 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG@13
PS1, Line 13: debug
> Are such wide disparities there in release mode as well?
yea I think quoting release build numbers is probably more interesting



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 28 Apr 2020 03:51:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................

[master-test] added GetTableSchema() microbenchmarks

Added a couple of GetTableSchema() microbenchmark tests.  These are to
gauge the performance of GetTableSchema() RPCs and the direct calls
to the catalog manager with authz enabled and disabled.

Results for a RELEASE build:
  GetTableSchema RPC: 45645 req/sec (authz disabled)
  GetTableSchema RPC: 43675.2 req/sec (authz enabled)
  GetTableSchema function: 1245196.8 req/sec (authz disabled)
  GetTableSchema function: 62765.6 req/sec (authz enabled)

Results for a DEBUG build:
  GetTableSchema RPC: 5283 req/sec (authz disabled)
  GetTableSchema RPC: 4973.8 req/sec (authz enabled)
  GetTableSchema function: 91688 req/sec (authz disabled)
  GetTableSchema function: 27960.4 req/sec (authz enabled)

Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Reviewed-on: http://gerrit.cloudera.org:8080/15828
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/master/master-test.cc
1 file changed, 161 insertions(+), 2 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 3: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1139
PS1, Line 1139:   const auto errors = accumulate(error_counters.begin(), error_counters.end(), 0UL);
              :   if (errors != 0) {
              :     FAIL() << Substitute("detected $0 errors", errors);
              :   }
              : 
              :   const auto total = accumulate(call_counters.begin(), call_counters.end(), 0UL);
              :   L
> Moved setting supports_authz_ into the constructor.
Ack


http://gerrit.cloudera.org:8080/#/c/15828/3/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/15828/3/src/kudu/master/master-test.cc@1123
PS3, Line 1123:         if (resp.has_error()) {
nit: consider logging the error too.


http://gerrit.cloudera.org:8080/#/c/15828/3/src/kudu/master/master-test.cc@1187
PS3, Line 1187:           ++error_counters[idx];
nit: same here



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 30 Apr 2020 22:14:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15828/5//COMMIT_MSG
Commit Message:

PS5: 
I guess I need to update the stats to reflect the new way of reporting: requests/second.



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 05 May 2020 02:54:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 7:

> (1 comment)

An update: I updated the benchmark to better simulate multiple clients environment.  It turns out that the difference in performance for RPC is about 3.5 times. See https://gerrit.cloudera.org/#/c/15894/


-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 11 May 2020 18:16:34 +0000
Gerrit-HasComments: No

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG@13
PS4, Line 13: Results for a RELEASE build:
> The numbers were significantly lower than these numbers (the information is
Gotcha. Yeah, I was wondering whether the low throughput they saw could have been caused by other load on the system, but it seems like there wasn't much beyond GetTableSchema() calls.


http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG@14
PS4, Line 14:   authz disabled: completed 208055 GetTableSchema RPC requests in 5.000s
> This is a good idea.  I think it's easier to go with that, and start right 
Ack



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 01 May 2020 23:25:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15828/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/6//COMMIT_MSG@14
PS6, Line 14:   GetTableSchema RPC: 45645 req/sec (authz disabled)
            :   GetTableSchema RPC: 43675.2 req/sec (authz enabled)
            :   GetTableSchema function: 1245196.8 req/sec (authz disabled)
            :   GetTableSchema function: 62765.6 req/sec (authz enabled)
            : 
Given the pretty huge disparity between RPC and direct function call, does this imply that most of the latency is coming from the RPC subsystem?



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 07 May 2020 05:54:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 07 May 2020 05:53:22 +0000
Gerrit-HasComments: No

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 4: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG@13
PS4, Line 13: Results for a RELEASE build:
Curious whether these numbers are at all close to the numbers in the cluster you saw. I also wonder whether the cluster was getting hit with many alter table requests to drop/add range partitions. Would that lead to a large number of OpenTable() calls, in the same way that altering schemas does?


http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG@14
PS4, Line 14:   authz disabled: completed 208055 GetTableSchema RPC requests in 5.000s
nit: at some point, we'll probably want to track requests/sec so we can modify the timings without a significant change in values.



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 01 May 2020 18:28:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG@13
PS1, Line 13: debug
> yea I think quoting release build numbers is probably more interesting
Added stats for RELEASE build as well.  Interestingly enough, in the case of RELEASE build the difference is huge for function call counters.


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1084
PS1, Line 1084: FLAGS_rpc_service_queue_length
> Probably, this should be min(FLAGS_rpc_service_queue_length, the number of 
From the other side, it's desirable to test with as many threads as possible.  The only requirement is to avoid RPC queue overflows.


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1110
PS1, Line 1110: if (resp.has_error()) {
              :           LOG(WARNING) << SecureDebugString(resp);
              :           continue;
              :         }
> Do you think it's worth failing this test on error? That way we can more ea
Indeed, that's a better approach.  Done.


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1137
PS1, Line 1137: 200
> Probably, this should depend on the number of CPU cores.
From the other side, it's desirable to test with many threads each representing a client calling GetTableSchema().


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1139
PS1, Line 1139:   const bool supports_authz = GetParam();
              :   FLAGS_master_support_authz_tokens = supports_authz;
              : 
              :   for (auto idx = 0; idx < kNumTables; ++idx) {
              :     auto table_name = Substitute("testtb_$0", idx);
              :     EXPECT_OK(CreateTable(table_name, kTableSchema));
              :   }
> nit: consider putting these in some common SetUp()?
Moved setting supports_authz_ into the constructor.

In different scenarios kNumTables is different, so SetUp() would not fit for the piece of the creation of tables, IMHO.


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1180
PS1, Line 1180:   auto stopper_cleanup = MakeScopedCleanup([&]() {
              :     for (auto& t : caller_threads) {
              :       t.join();
              :     }
              :   });
> nit: just FYI, you can use this too if you'd like, especially if you don't 
Thanks!  I always forget about those useful macros.


http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1191
PS1, Line 1191: calls
> nit: maybe "function calls"? an RPC is also a "call", so it wasn't quite cl
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 28 Apr 2020 08:21:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15828/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/6//COMMIT_MSG@14
PS6, Line 14:   GetTableSchema RPC: 45645 req/sec (authz disabled)
            :   GetTableSchema RPC: 43675.2 req/sec (authz enabled)
            :   GetTableSchema function: 1245196.8 req/sec (authz disabled)
            :   GetTableSchema function: 62765.6 req/sec (authz enabled)
            : 
> Given the pretty huge disparity between RPC and direct function call, does 
I guess so.  While serving PRCs, a lot of extra work is done to serialize the request, send it over the network (in case of loopback interfaces that's only about copying from userspace to kernel space and then back), handle the request to the servicing thread, de-serialize the request and then serialize the response, sending it back over the network, receive the data, and de-serialize the resonse at the client side.

All these add context switching, so the end results doesn't show the order of magnitude difference after all.



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 07 May 2020 15:08:11 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#6).

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................

[master-test] added GetTableSchema() microbenchmarks

Added a couple of GetTableSchema() microbenchmark tests.  These are to
gauge the performance of GetTableSchema() RPCs and the direct calls
to the catalog manager with authz enabled and disabled.

Results for a RELEASE build:
  GetTableSchema RPC: 45645 req/sec (authz disabled)
  GetTableSchema RPC: 43675.2 req/sec (authz enabled)
  GetTableSchema function: 1245196.8 req/sec (authz disabled)
  GetTableSchema function: 62765.6 req/sec (authz enabled)

Results for a DEBUG build:
  GetTableSchema RPC: 5283 req/sec (authz disabled)
  GetTableSchema RPC: 4973.8 req/sec (authz enabled)
  GetTableSchema function: 91688 req/sec (authz disabled)
  GetTableSchema function: 27960.4 req/sec (authz enabled)

Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
---
M src/kudu/master/master-test.cc
1 file changed, 161 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15828/6
-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................

[master-test] added GetTableSchema() microbenchmarks

Added a couple of GetTableSchema() microbenchmark tests.  These are to
gauge the performance of GetTableSchema() RPCs and the direct calls
to the catalog manager with authz enabled and disabled.

Results for a RELEASE build:
  authz disabled: completed 208055 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 205599 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 5715530 GetTableSchema function calls in 5.000s
  authz enabled: completed 306410 GetTableSchema function calls in 5.000s

Results for a DEBUG build:
  authz disabled: completed 27309 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 24699 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 548249 GetTableSchema calls in 5.000s
  authz enabled: completed 141795 GetTableSchema calls in 5.000s

Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
---
M src/kudu/master/master-test.cc
1 file changed, 161 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15828/4
-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................

[master-test] added GetTableSchema() microbenchmarks

Added a couple of GetTableSchema() microbenchmark tests.  These are to
gauge the performance of GetTableSchema() RPCs and the direct calls
to the catalog manager with authz enabled and disabled.

Results for a RELEASE build:
  authz disabled: completed 208055 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 205599 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 5715530 GetTableSchema function calls in 5.000s
  authz enabled: completed 306410 GetTableSchema function calls in 5.000s

Results for a DEBUG build:
  authz disabled: completed 27309 GetTableSchema RPC requests in 5.000s
  authz enabled: completed 24699 GetTableSchema RPC requests in 5.000s
  authz disabled: completed 548249 GetTableSchema calls in 5.000s
  authz enabled: completed 141795 GetTableSchema calls in 5.000s

Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
---
M src/kudu/master/master-test.cc
1 file changed, 156 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15828/2
-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [master-test] added GetTableSchema() microbenchmarks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )

Change subject: [master-test] added GetTableSchema() microbenchmarks
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG@13
PS4, Line 13: Results for a RELEASE build:
> Curious whether these numbers are at all close to the numbers in the cluste
The numbers were significantly lower than these numbers (the information is based on RPC request counting metrics) in the cluster I looked at.

As for the amount of AlterTable() calls, the number of those calls was negligible compared with the number of GetTableSchema()/GetTableLocations() requests (i.e. 1 request/minute for AlterTable vs 2K requests/second for GetTableSchema()).  At least that's what I saw in the recently collected diagnostics logs.  I'll check the very first one, though.

Does this answer your question?


http://gerrit.cloudera.org:8080/#/c/15828/4//COMMIT_MSG@14
PS4, Line 14:   authz disabled: completed 208055 GetTableSchema RPC requests in 5.000s
> nit: at some point, we'll probably want to track requests/sec so we can mod
This is a good idea.  I think it's easier to go with that, and start right away.  I'll update the patch accordingly.



-- 
To view, visit http://gerrit.cloudera.org:8080/15828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9
Gerrit-Change-Number: 15828
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 01 May 2020 20:54:12 +0000
Gerrit-HasComments: Yes