You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Wang Xixu (Code Review)" <ge...@cloudera.org> on 2022/05/27 08:07:56 UTC

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Wang Xixu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18572


Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got worsed.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
4 files changed, 244 insertions(+), 7 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 1
Gerrit-Owner: Wang Xixu <14...@qq.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 239 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/18
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 18
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 335 insertions(+), 80 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/27
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 27
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 34: Verified+1 Code-Review+2

Unrelated test failure in LogRollingITest.TestLogCleanupOnStartup.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 34
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 13:43:28 +0000
Gerrit-HasComments: No

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 2:

(1 comment)

> Patch Set 1:
> 
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/18572/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/18572/1/src/kudu/tools/kudu-admin-test.cc@3043
PS1, Line 3043:                                 const string& tables = "") {
> warning: the parameter 'tables' is copied for each invocation but only used
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Comment-Date: Fri, 27 May 2022 08:42:47 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18572/3/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/3/src/kudu/tools/master_rebuilder.cc@138
PS3, Line 138:       const std::set<string>& filter_tables = strings::Split(FLAGS_tables, ",",
Better to obtain filter_tables only once out of this loop.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sun, 29 May 2022 12:29:24 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 239 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/17
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 17
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 298 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/22
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 22
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 33:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18572/33/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/33/src/kudu/tools/master_rebuilder.cc@500
PS33, Line 500: fake_map
nit: remove this fake_map.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 33
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 12:43:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 10:

> Uploaded patch set 10.

How you replied this comment ?https://gerrit.cloudera.org/c/18572/3/src/kudu/tools/master_rebuilder.cc#404


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sun, 26 Jun 2022 08:39:36 +0000
Gerrit-HasComments: No

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 345 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/30
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 30
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 258 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/20
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 20
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 4:

(1 comment)

> Patch Set 3:
> 
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/18572/3/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/3/src/kudu/tools/master_rebuilder.cc@138
PS3, Line 138:         default:
> Better to obtain filter_tables only once out of this loop.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 04 Jun 2022 02:49:06 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
6 files changed, 192 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/11
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
6 files changed, 191 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/12
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 12
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 345 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/32
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 32
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 24:

> Patch Set 23:
> 
> (1 comment)

done


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 24
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Jul 2022 10:10:02 +0000
Gerrit-HasComments: No

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got worsed.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
4 files changed, 246 insertions(+), 7 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 34:

(1 comment)

> Patch Set 33:
> 
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/18572/33/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/33/src/kudu/tools/master_rebuilder.cc@500
PS33, Line 500: me_to_ad
> nit: remove this fake_map.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 34
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 12:49:55 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 234 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/14
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 14
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 18:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/master.h@102
PS18, Line 102:   void close_fs_manager_for_test() { fs_manager_ = nullptr;}
nit: better to define as private accessibility


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/sys_catalog.h@104
PS18, Line 104: SimpleTableVisitor
It's suggest to naming a more meaningful name.


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@110
PS18, Line 110: strings
nit: alignment


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@429
PS18, Line 429:  = ""
nit: can be omit.


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@428
PS18, Line 428: bool find_same_table_flag = false;
              :   string old_table_name = "";
              :   string new_table_name = "";
emm, better to move closer to the place where it used.


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@430
PS18, Line 430:  = ""
nit: can be omit.


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@433
PS18, Line 433: for (const auto& table : visitor.tables) {
You can define TableVisitor.tables as a associative or hash container, then you can reduce the time complex from O(n) to O(log(n)) or O(1).


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@437
PS18, Line 437: table_entry.second->metadata().ReadLock();
              :       new_table_name = table_entry.second->metadata().state().name();
              :       table_entry.second->metadata().ReadUnlock();
new_table_name can be obtained out of the loop, can't it?


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@446
PS18, Line 446:     if (!find_same_table_flag) {
find_same_table_flag can be removed if you do what I suggest above.


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@452
PS18, Line 452: const auto& table = table_entry.second;
              :     table->GetAllTablets(&tablets);
              :     TableMetadataLock l_table(table.get(), LockMode::WRITE);
              :     TabletMetadataGroupLock l_tablets(LockMode::RELEASED);
              :     l_tablets.AddMutableInfos(tablets);
              :     l_tablets.Lock(LockMode::WRITE);
              :     SysCatalogTable::Actions actions;
              :     actions.table_to_update = table;
              :     actions.tablets_to_update = tablets;
              :     RETURN_NOT_OK_PREPEND(sys_catalog.Write(actions),
              :                           Substitute("unable to write metadata for table $0 to sys_catalog",
              :                                      table_entry.first));
Could you please define a function for these code, and then you can reuse it in 3 places at least.


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/tool_action_master.cc@910
PS18, Line 910: create
Some of the help info should be updated.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 18
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Jul 2022 15:50:11 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 298 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/23
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 23
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 25:

(10 comments)

> Patch Set 24:
> 
> (9 comments)

http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3139
PS23, Line 3139: ASSERT_OK(sys_catalog.tablet_replica()->consensus()->WaitUntilLeader(kLeaderTimeout));
               :   TableInfoLoader table_info_loader;
               :   sys_catalog.VisitTables(&table_info_loader);
> move closer to the place where you use them.
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3148
PS23, Line 3148:     table_info = table;
> meanless, remove it.
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3160
PS23, Line 3160:  tablets.push_back(tablet);
> comments need update.
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3162
PS23, Line 3162:   }
> Could you wrap these code to a function?
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3165
PS23, Line 3165: bleMetadataLock l_table(table_info.get(), LockMode::WRITE);
               :   TabletMetadataGroupLock l_tablets(LockMode::RELEASED);
               :   l_t
> assign directly? they are both std::vector<std::string>
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@406
PS23, Line 406:     return Status::Corruption("inconsistent replica: partition mismatch");
> Seems set is enough, no need map.
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@431
PS23, Line 431: 
> warning: method 'DoWrite' can be made static [readability-convert-member-fu
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@432
PS23, Line 432: 
> nit: how about table_by_name?
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@433
PS23, Line 433: alog, tables_by_name_,
> it's just an enum, const reference is no needed.
Done


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@434
PS23, Line 434:                       SysCatalogTable::SysCatalogOperation::ADD));
              :   return Status::OK();
> if map_tables is empty, you can skip it.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 25
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Jul 2022 11:24:11 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18572/3/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/3/src/kudu/tools/master_rebuilder.cc@138
PS3, Line 138:       const std::set<string>& filter_tables = strings::Split(FLAGS_tables, ",",
nit: you can omit std namespace like string.


http://gerrit.cloudera.org:8080/#/c/18572/3/src/kudu/tools/master_rebuilder.cc@404
PS3, Line 404: Status MasterRebuilder::UpdateSysCatalog() {
Seems most code is similar to WriteSysCatalog, how about reuse it?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sun, 29 May 2022 09:54:48 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 345 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/33
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 33
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 21:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h@104
PS21, Line 104: class TableInfoVisitor : public TableVisitor {
Seems similar to class TestTableLoader in src/kudu/master/sys_catalog-test.cc, could you do some refactor to reuse it?


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h@415
PS21, Line 415:   TableInfoVisitor table_info_loader_;
I don't think it's necessary to make it as a member, isn't it?


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.cc@1165
PS21, Line 1165:                                       const SysTablesEntryPB& metadata) {
nit: alignment


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3143
PS21, Line 3143:                                           /*is_secure*/false, /*log_to_stderr*/true, part_tables),
nit: alignment


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3157
PS21, Line 3157: Rebuild 3 tables in add mode.
Have you miss this test case?


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3177
PS21, Line 3177: foo1
Should be 'default.foo1'? And it's better to use kTable1 instead.


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3187
PS21, Line 3187:       }
should break?


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3191
PS21, Line 3191:   NO_FATALS(master.close_fs_manager_for_test());
You can destroy 'master' then you don't have to close_fs_manager_for_test manually.
Or you can close fs_manager in master::Shutdown().


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc@411
PS21, Line 411: tables_map_to_update
Naming it as <value>_by_<key> is a good practice in Kudu, e.g. std::unordered_map<string, TabletSizeStats> size_stats_by_table_id in src/kudu/tools/tool_action_local_replica.cc, it's suggest to rename these variables here.


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc@425
PS21, Line 425: map_tables_in_sys_catalog.find(new_table_name) !=
              :           map_tables_in_sys_catalog.end()
Could you plz use ContainsKey in src/kudu/gutil/map-util.h to simplify code?


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/tool_action_master.cc@931
PS21, Line 931:         "- The parameter 'tables' can be used to rebuild part of tables, when these tables "
Add a blank space before '-'



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 21
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Jul 2022 07:30:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 22:

(11 comments)

> Patch Set 21:
> 
> (11 comments)

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h@104
PS21, Line 104: class TableInfoLoader : public TableVisitor {
> Seems similar to class TestTableLoader in src/kudu/master/sys_catalog-test.
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h@415
PS21, Line 415: 
> I don't think it's necessary to make it as a member, isn't it?
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.cc@1165
PS21, Line 1165:                                    const SysTablesEntryPB& metadata) {
> nit: alignment
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3143
PS21, Line 3143: 
> nit: alignment
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3157
PS21, Line 3157: sterVerifier cv1(cluster_.get
> Have you miss this test case?
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3177
PS21, Line 3177: fo> 
> Should be 'default.foo1'? And it's better to use kTable1 instead.
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3187
PS21, Line 3187:     ASSERT_NE(nullptr, table_info);
> should break?
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3191
PS21, Line 3191:     vector<scoped_refptr<TabletInfo>> tablets;
> You can destroy 'master' then you don't have to close_fs_manager_for_test m
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc@411
PS21, Line 411: 
> Naming it as <value>_by_<key> is a good practice in Kudu, e.g. std::unorder
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc@425
PS21, Line 425: 
              :   RETURN_NOT_OK(DoWrite(&sys_catalog, tab
> Could you plz use ContainsKey in src/kudu/gutil/map-util.h to simplify code
Done


http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/tool_action_master.cc@931
PS21, Line 931:         " - The parameter 'tables' can be used to rebuild part of tables, when these tables "
> Add a blank space before '-'
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 22
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 08:54:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 12:

(6 comments)

> Patch Set 10:
> 
> (8 comments)

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.h@415
PS10, Line 415:   SimpleTableVisitor simple_table_loader_;
> nit: use snake case.
Done


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.cc@1155
PS10, Line 1155:                                       const SysTablesEntryPB& metadata) {
> nit: alignment
Done


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/kudu-admin-test.cc@3132
PS10, Line 3132:                                           /*is_secure*/false, /*log_to_stderr*/true, part_tables),
> nit: alignment
Done


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc@81
PS10, Line 81: using std::set;
             : using std::stri
> nit: keep them in lexicographical order.
Done


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc@130
PS10, Line 130:       const auto& table_name = tablet_status_pb.table_name();
> If the table you specified in FLAGS_tables is not found in replicas, report
If a table is not found in replicas, tables_by_name_ will be empty, it will not update the sys_catalog.


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc@167
PS10, Line 167: 
> nit: better to judge on filter_tables, since there maybe some invalid chara
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 12
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Jun 2022 02:55:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 239 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/16
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 16
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 10:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.h@415
PS10, Line 415:   SimpleTableVisitor simpleTableLoader_;
nit: use snake case.


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/master/sys_catalog.cc@1155
PS10, Line 1155:                                      const SysTablesEntryPB& metadata) {
nit: alignment


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/kudu-admin-test.cc@3132
PS10, Line 3132:                                           /*is_secure*/false, /*log_to_stderr*/true, part_tables),
nit: alignment


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/kudu-admin-test.cc@3137
PS10, Line 3137:   ASSERT_STR_CONTAINS(stdout, "Rebuilt from 2 replicas, of which 0 had errors");
After master been rebuilt, it's better to restart the cluster and check everything running as expect.


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/kudu-admin-test.cc@3151
PS10, Line 3151:   ASSERT_STR_CONTAINS(stdout, "Rebuilt from 3 replicas, of which 0 had errors");
same


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc@81
PS10, Line 81: using std::string;
             : using std::set;
nit: keep them in lexicographical order.


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc@130
PS10, Line 130:       if (!filter_tables.empty() && !ContainsKey(filter_tables, table_name)) continue;
If the table you specified in FLAGS_tables is not found in replicas, report an error?


http://gerrit.cloudera.org:8080/#/c/18572/10/src/kudu/tools/master_rebuilder.cc@167
PS10, Line 167: FLAGS_tables
nit: better to judge on filter_tables, since there maybe some invalid characters in FLAGS_tables before strings::Split



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sun, 26 Jun 2022 08:38:44 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got worsed.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
4 files changed, 247 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/4
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 28:

(1 comment)

> Patch Set 26:
> 
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/18572/26/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/26/src/kudu/tools/master_rebuilder.cc@473
PS26, Line 473: ptr<master::TableInfo>> table_by_name_to_del
> It seems this UPDATE operation wouldn't take effect on master's SysCatalogT
That's true. I will delete a table and it's tablet in syscatalog when a table with the sanme name exists in syscatalog and then rebuild it.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 28
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 19 Jul 2022 06:52:40 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Reviewed-on: http://gerrit.cloudera.org:8080/18572
Reviewed-by: Yifan Zhang <ch...@163.com>
Tested-by: Yifan Zhang <ch...@163.com>
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 343 insertions(+), 81 deletions(-)

Approvals:
  Yifan Zhang: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 35
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 26:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18572/26/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/26/src/kudu/tools/master_rebuilder.cc@473
PS26, Line 473: SysCatalogTable::SysCatalogOperation::UPDATE
It seems this UPDATE operation wouldn't take effect on master's SysCatalogTable, it would be treated as an empty update in SysCatalogTable::ReqUpdateTable().

On second thought, it doesn't seem to make sense to update the metadata directly. In this case, perhaps we should return an AlreadyPresent ERROR and tell the user that a table already exists and that if they really want to rebuild the table, they can manually delete it and run the tool again.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 26
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 18 Jul 2022 13:17:48 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 31: Code-Review+1

(2 comments)

Overall looks good now, just a few more nits.

http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/master_rebuilder.cc@100
PS31, Line 100: tablet_by_name
nit: maybe make it a default parameter with the default value 'nullptr', and then 'fake_map' will not be needed.


http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/tool_action_master.cc@931
PS31, Line 931:         " - The parameter 'tables' can be used to rebuild part of tables, when these tables "
nit: I think we should highlight the difference between with and without '--tables' in the tool description. 
- Without '--tables', the tool will build a brand new sys.catalog table based on tablet data on tablet server metadata. 
- With '--tables', the tool will update specific tables in current sys.catalog.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 31
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 09:21:45 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 310 insertions(+), 80 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/25
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 25
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 306 insertions(+), 80 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/26
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 26
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 261 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/21
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 21
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 32:

(2 comments)

> Patch Set 31: Code-Review+1
> 
> (2 comments)
> 
> Overall looks good now, just a few more nits.

http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/master_rebuilder.cc@100
PS31, Line 100: tablet_by_name
> nit: maybe make it a default parameter with the default value 'nullptr', an
Done


http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/18572/31/src/kudu/tools/tool_action_master.cc@931
PS31, Line 931:         " - Without '--tables', the tool will build a brand new syscatalog table based on "
> nit: I think we should highlight the difference between with and without '-
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 32
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Jul 2022 10:02:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 24:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3139
PS23, Line 3139: string stdout;
               :   string stderr;
               :   const string& part_tables = Substitute("$0,$1", kTable1, kTable2);
move closer to the place where you use them.


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3148
PS23, Line 3148: ASSERT_STR_NOT_CONTAINS(stderr, "must be empty");
meanless, remove it.


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3160
PS23, Line 3160: Rebuild 1 table in add mode.
comments need update.


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3162
PS23, Line 3162:   {
Could you wrap these code to a function?


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/kudu-admin-test.cc@3165
PS23, Line 3165: for (const string& data_dir : cluster_->master()->data_dirs()) {
               :       opts.fs_opts.data_roots.push_back(data_dir);
               :     }
assign directly? they are both std::vector<std::string>


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@406
PS23, Line 406:   }
Seems set is enough, no need map.


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@432
PS23, Line 432: 
nit: how about table_by_name?


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@433
PS23, Line 433: ogTable::SysCatalogOperati
it's just an enum, const reference is no needed.


http://gerrit.cloudera.org:8080/#/c/18572/23/src/kudu/tools/master_rebuilder.cc@434
PS23, Line 434: return Status::OK();
              : }
if map_tables is empty, you can skip it.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 24
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Jul 2022 10:12:55 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 306 insertions(+), 80 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/24
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 24
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 344 insertions(+), 80 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/29
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 29
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
4 files changed, 247 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
6 files changed, 171 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/9
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 9
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18572/5/src/kudu/tools/master_rebuilder.h
File src/kudu/tools/master_rebuilder.h:

http://gerrit.cloudera.org:8080/#/c/18572/5/src/kudu/tools/master_rebuilder.h@113
PS5, Line 113:   Status UpdateSysCatalog();
nit: Would be better to leave a message describing this new method.


http://gerrit.cloudera.org:8080/#/c/18572/5/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/5/src/kudu/tools/master_rebuilder.cc@141
PS5, Line 141:       if (!filter_tables.empty() && !ContainsKey(filter_tables, table_name)) continue;
nit: Maybe move this to L129? So we will not get 'Skipping replica ...' messages of filtered tables.


http://gerrit.cloudera.org:8080/#/c/18572/5/src/kudu/tools/master_rebuilder.cc@427
PS5, Line 427: table_to_update
Why we should update table/tablet entry? What if the entry doesn't exist before we recover it from tservers?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 17:26:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has removed a vote on this change.

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 34
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 335 insertions(+), 80 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/28
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 28
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 346 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/31
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 31
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 342 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/6
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
6 files changed, 172 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/7
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 239 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/15
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 15
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 343 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/34
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 34
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 35: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 35
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Jul 2022 10:20:56 +0000
Gerrit-HasComments: No

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got worsed.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
4 files changed, 246 insertions(+), 7 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
9 files changed, 255 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
6 files changed, 171 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/10
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Yingchun Lai, Yifan Zhang, Kudu Jenkins, 

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

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

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................

[Tools] Rebuild master according to part of tables not all tables

Current function unsafe_rebuild will get all tables's meta data
from tservers, and rebuild a new sys table. The cost is very
high when only part of tables got damaged.

This patch will rebuild master with configured tables. That
means if some tables meta data get damaged, it only needs to
repair these damaged tables meta data.

Change-Id: I45d812c8a16228ac937603872161969eb05136eb
---
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/master_rebuilder.h
M src/kudu/tools/tool_action_master.cc
7 files changed, 256 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18572/19
-- 
To view, visit http://gerrit.cloudera.org:8080/18572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 19
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [Tools] Rebuild master according to part of tables not all tables

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

Change subject: [Tools] Rebuild master according to part of tables not all tables
......................................................................


Patch Set 20:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/master.h@102
PS18, Line 102:   // Get the RPC and HTTP addresses for this master instance.
> nit: better to define as private accessibility
It can not be private, because it is used in outside. See kudu-admin-test.cc Line 3191


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/master/sys_catalog.h@104
PS18, Line 104: TableInfoVisitor :
> It's suggest to naming a more meaningful name.
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc
File src/kudu/tools/master_rebuilder.cc:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@110
PS18, Line 110: rings::
> nit: alignment
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@429
PS18, Line 429: bles_
> nit: can be omit.
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@428
PS18, Line 428:     } else {
              :         InsertOrDie(&tables_map_to_add, table_entry.first, table_entry.second);
              :       }
> emm, better to move closer to the place where it used.
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@430
PS18, Line 430: 
> nit: can be omit.
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@433
PS18, Line 433: TURN_NOT_OK(DoWrite(sys_catalog, tables_ma
> You can define TableVisitor.tables as a associative or hash container, then
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@437
PS18, Line 437: 
              : Status MasterRebuilder::DoWrite(SysCatalogTable sys_catalog,
              :                                 const map<string, 
> new_table_name can be obtained out of the loop, can't it?
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@446
PS18, Line 446:     TabletMetadataGroupLock l_tablets(LockMode::RELEASED);
> find_same_table_flag can be removed if you do what I suggest above.
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/master_rebuilder.cc@452
PS18, Line 452:     actions.table_to_add = table;
              :         actions.tablets_to_add = tablets;
              :         break;
              :       }
              :       case SysCatalogOperation::UPDATE: {
              :         actions.table_to_update = table;
              :         actions.tablets_to_update = tablets;
              :         break;
              :       }
              :       case SysCatalogOperation::DELETE: {
              :         actions.table_to_delete = table;
              :         actions.tablets_to_delete = tablets;
> Could you please define a function for these code, and then you can reuse i
Done


http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/18572/18/src/kudu/tools/tool_action_master.cc@910
PS18, Line 910: create
> Some of the help info should be updated.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb
Gerrit-Change-Number: 18572
Gerrit-PatchSet: 20
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jul 2022 11:14:27 +0000
Gerrit-HasComments: Yes