You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/08/08 22:30:23 UTC
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Hello Jean-Daniel Cryans, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/7621
to review the following change.
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
[maintenance manager] fix op scheduling lock contention, thread ID tweak
943b1ae26e62a0c82 introduced a lock acquisition on the MM global lock
when a worker thread begins running an op. This resulted in measurable
lock contention with many MM threads, for instance the dense_node-itest
which creates 100 MM threads. This commit introduces a finer grained
lock around the currently running ops container, which reduces the
contention on the global lock.
I've also snuck in a change to make the thread ID correspond to the
system thread ID, instead of a synthetic C++ stdlib ID.
Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
---
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
M src/kudu/util/maintenance_manager.proto
3 files changed, 30 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/7621/1
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7621
to look at the new patch set (#2).
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
[maintenance manager] fix op scheduling lock contention, thread ID tweak
943b1ae26e62a0c82 introduced a lock acquisition on the MM global lock
when a worker thread begins running an op. This resulted in measurable
lock contention with many MM threads, for instance the dense_node-itest
which creates 100 MM threads. This commit introduces a finer grained
lock around the currently running ops container, which reduces the
contention on the global lock.
I've also snuck in a change to make the thread ID correspond to the
system thread ID, instead of a synthetic C++ stdlib ID.
Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
---
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
M src/kudu/util/maintenance_manager.proto
3 files changed, 30 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/7621/2
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
[maintenance manager] fix op scheduling lock contention, thread ID tweak
943b1ae26e62a0c82 introduced a lock acquisition on the MM global lock
when a worker thread begins running an op. This resulted in measurable
lock contention with many MM threads, for instance the dense_node-itest
which creates 100 MM threads. This commit introduces a finer grained
lock around the currently running ops container, which reduces the
contention on the global lock.
I've also snuck in a change to make the thread ID correspond to the
system thread ID, instead of a synthetic C++ stdlib ID.
Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Reviewed-on: http://gerrit.cloudera.org:8080/7621
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
M src/kudu/util/maintenance_manager.proto
3 files changed, 30 insertions(+), 17 deletions(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7621/1/src/kudu/util/maintenance_manager.proto
File src/kudu/util/maintenance_manager.proto:
Line 35: required sfixed64 thread_id = 1;
why fixed?
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7621/1/src/kudu/util/maintenance_manager.proto
File src/kudu/util/maintenance_manager.proto:
Line 35: required sfixed64 thread_id = 1;
> Weak justification: thread IDs tend to be large numbers, so a fixed size in
weak rebuttal: thread IDs are usually <64k in the default OS config, so they'd fit in a 3-byte varint vs 8-byte fixed :)
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7621/1/src/kudu/util/maintenance_manager.proto
File src/kudu/util/maintenance_manager.proto:
Line 35: required sfixed64 thread_id = 1;
> why fixed?
Weak justification: thread IDs tend to be large numbers, so a fixed size integer might be smaller on average than a varint.
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [maintenance manager] fix op scheduling lock contention, thread ID tweak
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7621/1/src/kudu/util/maintenance_manager.proto
File src/kudu/util/maintenance_manager.proto:
Line 35: required sfixed64 thread_id = 1;
> weak rebuttal: thread IDs are usually <64k in the default OS config, so the
These will be synthetic thread IDs (probably a memory address) on OS X, but I think you've made a good argument, I've changed it to int64.
--
To view, visit http://gerrit.cloudera.org:8080/7621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I018ebe65c71344d8911ff66848478f75fef085af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes