[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
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 LipconTested-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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
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 BurkertGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
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 BurkertGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
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 BurkertGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
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 BurkertGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
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 BurkertGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [maintenance manager] fix op scheduling lock contention, thread ID tweak
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 BurkertGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[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 BurkertGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon