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 2016/09/01 23:20:52 UTC
[kudu-CR] cache: dynamically determine the number of cache shards
Hello David Ribeiro Alves,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4292
to review the following change.
Change subject: cache: dynamically determine the number of cache shards
......................................................................
cache: dynamically determine the number of cache shards
I noticed in a heavily cache-bound workload on a 48-core machine that
there was a lot of contention on the cache shard spin locks. Manually
bumping the number of cache shards to 64 instead of the default 16
seemed to reduce the amount of contention significantly.
Change-Id: I2181ba4863065a554068a9c6d390b7f49e094abc
---
M src/kudu/util/cache.cc
1 file changed, 19 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/4292/1
--
To view, visit http://gerrit.cloudera.org:8080/4292
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2181ba4863065a554068a9c6d390b7f49e094abc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
[kudu-CR] cache: dynamically determine the number of cache shards
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: cache: dynamically determine the number of cache shards
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/3192/
--
To view, visit http://gerrit.cloudera.org:8080/4292
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2181ba4863065a554068a9c6d390b7f49e094abc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] cache: dynamically determine the number of cache shards
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: cache: dynamically determine the number of cache shards
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4292/1/src/kudu/util/cache.cc
File src/kudu/util/cache.cc:
Line 376: int bits = Bits::Log2Ceiling(base::NumCPUs());
Nit: you could CHECK that the number of bits is less than 32. I mean, that's a lot of cores, but, you know. :)
--
To view, visit http://gerrit.cloudera.org:8080/4292
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2181ba4863065a554068a9c6d390b7f49e094abc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] cache: dynamically determine the number of cache shards
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: cache: dynamically determine the number of cache shards
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4292/1/src/kudu/util/cache.cc
File src/kudu/util/cache.cc:
Line 376: int bits = Bits::Log2Ceiling(base::NumCPUs());
> Nit: you could CHECK that the number of bits is less than 32. I mean, that'
eh, I think enough other stuff would crash on a system like that that I'm not worried (eg striped locks which allocate an array based on the number of CPUs)
--
To view, visit http://gerrit.cloudera.org:8080/4292
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2181ba4863065a554068a9c6d390b7f49e094abc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] cache: dynamically determine the number of cache shards
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: cache: dynamically determine the number of cache shards
......................................................................
cache: dynamically determine the number of cache shards
I noticed in a heavily cache-bound workload on a 48-core machine that
there was a lot of contention on the cache shard spin locks. Manually
bumping the number of cache shards to 64 instead of the default 16
seemed to reduce the amount of contention significantly.
Change-Id: I2181ba4863065a554068a9c6d390b7f49e094abc
Reviewed-on: http://gerrit.cloudera.org:8080/4292
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/util/cache.cc
1 file changed, 19 insertions(+), 7 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4292
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2181ba4863065a554068a9c6d390b7f49e094abc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>