[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..

meta_cache: improve multi-threaded scalability

When profiling a multi-threaded 'kudu perf loadgen' I found the
bottleneck was on the rw_spinlock acquisition in
MetaCache::LookupTabletByKeyFastPath as well as some contention in
RemoteTablet::stale().

This fixes the first by switching to a percpu_rwlock and the second by
using an atomic bool instead of a spinlock-protected bool.

This moved LookupTabletByKeyFastPath from the first line of the profile
(14% of CPU) down to the 8th line (3% of CPU).

Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Reviewed-on: http://gerrit.cloudera.org:8080/9760
Reviewed-by: Adar Dembo 
Tested-by: Todd Lipcon 
---
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/util/locks.h
3 files changed, 25 insertions(+), 17 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Todd Lipcon: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 4: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:56:59 +
Gerrit-HasComments: No

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Todd Lipcon has removed a vote on this change.

Change subject: meta_cache: improve multi-threaded scalability
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] meta cache: improve multi-threaded scalability

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 29 Mar 2018 20:36:26 +
Gerrit-HasComments: No

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: meta_cache: improve multi-threaded scalability
..

meta_cache: improve multi-threaded scalability

When profiling a multi-threaded 'kudu perf loadgen' I found the
bottleneck was on the rw_spinlock acquisition in
MetaCache::LookupTabletByKeyFastPath as well as some contention in
RemoteTablet::stale().

This fixes the first by switching to a percpu_rwlock and the second by
using an atomic bool instead of a spinlock-protected bool.

This moved LookupTabletByKeyFastPath from the first line of the profile
(14% of CPU) down to the 8th line (3% of CPU).

Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
---
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/util/locks.h
3 files changed, 25 insertions(+), 17 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h@261
PS1, Line 261:   std::atomic stale_;
> Previous discussion: https://gerrit.cloudera.org/c/1842/
Only thing i really dislike about std::atomic is that the API allows for 
Release_Load and Acquire_Store as follows:

std::atomic x;
int v = x.load(std::memory_order_release);
x.store(v, std::memory_order_acquire);

... but it turns out that the C++ memory model prohibits such things from 
existing. Rather than crash or give a warning or somesuch, it silently allows 
them without emitting the memory barriers required by x86 (which are emitted by 
our own Release_Load and Acquire_Store macros). Granted, those two operations 
are somewhat more esoteric, but they're useful for various low-level things 
like seq-locks. It appears we only use them in a handful of places in Kudu and 
some of them are potentially not even correct usage :)

Anyway, I feel like we should probably move to std::atomic for most use cases, 
and perhaps see if we can write a clang-tidy checker for the above incorrect 
usages and contribute it upstream.


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h
File src/kudu/util/locks.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@142
PS1, Line 142: // Usage:
 : //   percpu_rwlock mylock;
 : //
 : //   // Lock shared:
 : //   {
 : // boost::shared_lock lock(mylock.get_lock());
 : // ...
 : //   }
 : //
 : //   // Lock exclusive:
 : //
 : //   {
 : // boost::lock_guard lock(mylock);
 : // ...
 : //   }
> Mind updating this with the lock guards we actually use now?
Done


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@142
PS1, Line 142: // Usage:
 : //   percpu_rwlock mylock;
 : //
 : //   // Lock shared:
 : //   {
 : // boost::shared_lock lock(mylock.get_lock());
 : // ...
 : //   }
 : //
 : //   // Lock exclusive:
 : //
 : //   {
 : // boost::lock_guard lock(mylock);
 : // ...
 : //   }
> I think you missed this one.
sorry, my network crapped out mid-push and apparently I pushed an old version. 
take 2...


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@239
PS1, Line 239: rw_spinlock lock;
> Gotcha. Could you add a blurb about this optimization to the class comment?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 29 Mar 2018 16:42:14 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h
File src/kudu/util/locks.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@142
PS1, Line 142: // This means that in the read-mostly case, different readers 
will not cause any
 : // cacheline contention.
 : //
 : // Usage:
 : //   percpu_rwlock mylock;
 : //
 : //   // Lock shared:
 : //   {
 : // boost::shared_lock lock(mylock.get_lock());
 : // ...
 : //   }
 : //
 : //   // Lock exclusive:
 : //
 : //   {
> Mind updating this with the lock guards we actually use now?
I think you missed this one.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 29 Mar 2018 02:10:22 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: meta_cache: improve multi-threaded scalability
..

meta_cache: improve multi-threaded scalability

When profiling a multi-threaded 'kudu perf loadgen' I found the
bottleneck was on the rw_spinlock acquisition in
MetaCache::LookupTabletByKeyFastPath as well as some contention in
RemoteTablet::stale().

This fixes the first by switching to a percpu_rwlock and the second by
using an atomic bool instead of a spinlock-protected bool.

This moved LookupTabletByKeyFastPath from the first line of the profile
(14% of CPU) down to the 8th line (3% of CPU).

Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
---
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/util/locks.h
3 files changed, 23 insertions(+), 15 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] meta cache: improve multi-threaded scalability

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h@261
PS1, Line 261:   std::atomic stale_;
> Found the Chromium reference: https://bugs.chromium.org/p/chromium/issues/d
Previous discussion: https://gerrit.cloudera.org/c/1842/



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 24 Mar 2018 11:07:05 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h
File src/kudu/util/locks.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@239
PS1, Line 239: rw_spinlock lock;
> Cuz a node can get pre-empted off its CPU while it holds the lock, it's han
Gotcha. Could you add a blurb about this optimization to the class comment?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 23 Mar 2018 19:07:24 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h@261
PS1, Line 261:   std::atomic stale_;
> On older compilers I think there were some issues with optimizations or som
Found the Chromium reference: 
https://bugs.chromium.org/p/chromium/issues/detail?id=592903

Seems like gcc 4.8 probably is OK, but will look into it a bit before 
suggesting we remove atomicops entirely



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 23 Mar 2018 18:57:25 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h@261
PS1, Line 261:   std::atomic stale_;
> Hmm, weren't you telling me in the past that std::atomic wasn't as performa
On older compilers I think there were some issues with optimizations or 
something -- the issue I remember citing was when Chromium converted SpinLock 
to std::atomic it caused a regression so they backed it out. We probably should 
re-evaluate on our current generation of compilers. But either way I think this 
is a big improvement from spinlock-protected bool.


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h
File src/kudu/util/locks.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@239
PS1, Line 239: rw_spinlock lock;
> I'm curious why this uses a rw_spinlock under the hood and not a simple_spi
Cuz a node can get pre-empted off its CPU while it holds the lock, it's handy 
that it's a reader-writer lock so the next thread on that same CPU can still 
acquire that slot.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 23 Mar 2018 18:52:46 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h@261
PS1, Line 261:   std::atomic stale_;
Hmm, weren't you telling me in the past that std::atomic wasn't as performant 
as util/atomic?


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h
File src/kudu/util/locks.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@142
PS1, Line 142: // Usage:
 : //   percpu_rwlock mylock;
 : //
 : //   // Lock shared:
 : //   {
 : // boost::shared_lock lock(mylock.get_lock());
 : // ...
 : //   }
 : //
 : //   // Lock exclusive:
 : //
 : //   {
 : // boost::lock_guard lock(mylock);
 : // ...
 : //   }
Mind updating this with the lock guards we actually use now?


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@239
PS1, Line 239: rw_spinlock lock;
I'm curious why this uses a rw_spinlock under the hood and not a 
simple_spinlock? It seems like it could work either way; the usage is either 
"acquire all locks for writing" (exclusive) or "acquire my CPU's lock for 
reading" (shared). The class comment talks about mutexes without specifying 
what kind they are.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 23 Mar 2018 18:44:09 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 23 Mar 2018 15:23:46 +
Gerrit-HasComments: No

[kudu-CR] meta cache: improve multi-threaded scalability

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9760/1//COMMIT_MSG@17
PS1, Line 17: This moved LookupTabletByKeyFastPath from the first line of the 
profile
: (14% of CPU) down to the 8th line (3% of CPU).
Wow, that's nice.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 22 Mar 2018 23:17:30 +
Gerrit-HasComments: Yes

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert,

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

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

to review the following change.


Change subject: meta_cache: improve multi-threaded scalability
..

meta_cache: improve multi-threaded scalability

When profiling a multi-threaded 'kudu perf loadgen' I found the
bottleneck was on the rw_spinlock acquisition in
MetaCache::LookupTabletByKeyFastPath as well as some contention in
RemoteTablet::stale().

This fixes the first by switching to a percpu_rwlock and the second by
using an atomic bool instead of a spinlock-protected bool.

This moved LookupTabletByKeyFastPath from the first line of the profile
(14% of CPU) down to the 8th line (3% of CPU).

Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
---
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/util/locks.h
3 files changed, 17 insertions(+), 12 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert