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 2018/06/19 22:11:16 UTC
[kudu-CR] KUDU-2480: tsan failure of master-stress-test
Hello Hao Hao,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10770
to review the following change.
Change subject: KUDU-2480: tsan failure of master-stress-test
......................................................................
KUDU-2480: tsan failure of master-stress-test
This is an attempt to fix the race reported in KUDU-2480. I've been
unable to reproduce the race on dist-test, however after auditing the
code mentioned the the stack traces I saw this bit of fishy code. My
current theory is that it's illegal to call
Synchronizer::AsStatusCallback twice on one synchronizer, which was
accidentally happening due to some dead code. This may or may not fix
the race, but it's a mistake worth fixing nonetheless.
Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
---
M src/kudu/master/hms_notification_log_listener.cc
1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/10770/1
--
To view, visit http://gerrit.cloudera.org:8080/10770
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
Gerrit-Change-Number: 10770
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
[kudu-CR] KUDU-2480: tsan failure of master-stress-test
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10770 )
Change subject: KUDU-2480: tsan failure of master-stress-test
......................................................................
Patch Set 1: Code-Review+2
I don't see why it would be a problem to call std::bind(&Synchronizer::StatusCB, this, std::placeholders::_1) twice on the same synchronizer, but this is dead code so why not remove it.
--
To view, visit http://gerrit.cloudera.org:8080/10770
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
Gerrit-Change-Number: 10770
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:40:22 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2480: tsan failure of master-stress-test
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10770 )
Change subject: KUDU-2480: tsan failure of master-stress-test
......................................................................
KUDU-2480: tsan failure of master-stress-test
This is an attempt to fix the race reported in KUDU-2480. I've been
unable to reproduce the race on dist-test, however after auditing the
code mentioned the the stack traces I saw this bit of fishy code. My
current theory is that it's illegal to call
Synchronizer::AsStatusCallback twice on one synchronizer, which was
accidentally happening due to some dead code. This may or may not fix
the race, but it's a mistake worth fixing nonetheless.
Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
Reviewed-on: http://gerrit.cloudera.org:8080/10770
Reviewed-by: Mike Percy <mp...@apache.org>
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Dan Burkert <da...@apache.org>
---
M src/kudu/master/hms_notification_log_listener.cc
1 file changed, 0 insertions(+), 2 deletions(-)
Approvals:
Mike Percy: Looks good to me, approved
Hao Hao: Looks good to me, but someone else must approve
Dan Burkert: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10770
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
Gerrit-Change-Number: 10770
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-2480: tsan failure of master-stress-test
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10770 )
Change subject: KUDU-2480: tsan failure of master-stress-test
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/10770
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
Gerrit-Change-Number: 10770
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:44:33 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2480: tsan failure of master-stress-test
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10770 )
Change subject: KUDU-2480: tsan failure of master-stress-test
......................................................................
Patch Set 1: Verified+1
Looks like an unrelated flake (non-HMS test).
--
To view, visit http://gerrit.cloudera.org:8080/10770
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
Gerrit-Change-Number: 10770
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:52:13 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2480: tsan failure of master-stress-test
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has removed a vote on this change.
Change subject: KUDU-2480: tsan failure of master-stress-test
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10770
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I6b5103242aa24ffa81813d8e6ab5df630c69c8cf
Gerrit-Change-Number: 10770
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>