You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2019/05/10 04:26:56 UTC

[kudu-CR](branch-1.9.x) [ksck] Make ksck tool more concurrent 

Hello Kudu Jenkins, Adar Dembo,

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

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

to review the following change.


Change subject: [ksck] Make ksck tool more concurrent
......................................................................

[ksck] Make ksck tool more concurrent

Though ksck fetches tserver info concurrently now, it's still slow when
fetching info from clusters that have thousands of tables when the CLI
and the cluster are in different regions.
This patch make it more concurrent when fetching master info and table
info.

Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Reviewed-on: http://gerrit.cloudera.org:8080/13024
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
(cherry picked from commit d17f9fce684272df28e6291839f162b33796bc5c)
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/tool_action_cluster.cc
6 files changed, 191 insertions(+), 109 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................


Patch Set 2: Code-Review+2

Could you add to the commit message a bit about how this isn't relevant for master because of Yingchun's commit?


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 10 May 2019 05:15:29 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins, Adar Dembo, 

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

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

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

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................

[tools] fix race in FetchInfoFromTabletServers()

Fixed race in FetchInfoFromTabletServers() while fetching information
from multiple tablet servers.

Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
---
M src/kudu/tools/ksck.cc
1 file changed, 1 insertion(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................


Patch Set 3: Verified+1

known flake in RemoteKsckTest.TestClusterWithLocation


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 10 May 2019 15:13:10 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.9.x) [ksck] Make ksck tool more concurrent

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [ksck] Make ksck tool more concurrent
......................................................................


Patch Set 1: Code-Review+2

Was this really the path of least resistance to fix the race in Ksck::FetchInfoFromTabletServers? I'm a little hesitant here because the patch being cherry-picked hasn't yet received a ton of testing.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 10 May 2019 04:50:14 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins, Adar Dembo, 

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

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

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

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................

[tools] fix race in FetchInfoFromTabletServers()

Fixed race in FetchInfoFromTabletServers() while fetching information
from multiple tablet servers.

In the master branch, the race is fixed with changelist
d17f9fce684272df28e6291839f162b33796bc5c, which also brings improvements
for concurrency while running ksck and other related CLI tools.  This
patch is targeted for the stable branch, so it only fixes the race
condition.

Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
---
M src/kudu/tools/ksck.cc
1 file changed, 1 insertion(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................

[tools] fix race in FetchInfoFromTabletServers()

Fixed race in FetchInfoFromTabletServers() while fetching information
from multiple tablet servers.

In the master branch, the race is fixed with changelist
d17f9fce684272df28e6291839f162b33796bc5c, which also brings improvements
for concurrency while running ksck and other related CLI tools.  This
patch is targeted for the stable branch, so it only fixes the race
condition.

Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Reviewed-on: http://gerrit.cloudera.org:8080/13301
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/tools/ksck.cc
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................


Patch Set 2: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 10 May 2019 05:29:56 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................


Patch Set 3:

> Could you add to the commit message a bit about how this isn't
 > relevant for master because of Yingchun's commit?

Done.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 10 May 2019 06:30:29 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.9.x) [ksck] Make ksck tool more concurrent

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [ksck] Make ksck tool more concurrent
......................................................................


Patch Set 1:

> Was this really the path of least resistance to fix the race in
 > Ksck::FetchInfoFromTabletServers? I'm a little hesitant here
 > because the patch being cherry-picked hasn't yet received a ton of
 > testing.

I prepared one-liner as well.  But I thought this one looked good enough and it allows for more concurrency while running ksck and related tools.

All right, I agree that the one-liner patch might be a better option if we want to play safe.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 10 May 2019 05:07:12 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.9.x) [tools] fix race in FetchInfoFromTabletServers()

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13301 )

Change subject: [tools] fix race in FetchInfoFromTabletServers()
......................................................................


Patch Set 3: Code-Review+2

Carrying over Adar's +2 from PS2.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.9.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4
Gerrit-Change-Number: 13301
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 10 May 2019 16:43:16 +0000
Gerrit-HasComments: No