You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yuqi Du (Code Review)" <ge...@cloudera.org> on 2023/01/04 08:00:53 UTC
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Yuqi Du has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19398
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
[compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
Now the gflags FLAGS_enable_maintenance_manager can be on/off, it should set
before start. So if we want to change it, we must restart tserver. But
restart tserver sometimes very slow.
This patch support change FLAGS_enable_maintenance_manager at runtime, it
can avoid troublesome restart operation when administrators need do this.
Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 7 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/19398/1
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 1
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19398
to look at the new patch set (#6).
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
[compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
Now the gflags FLAGS_enable_maintenance_manager can be on/off, it should set
before start. So if we want to change it, we must restart tservers. But
restarting a tablet server can sometimes be quite time consuming due to
various factors.
This patch adds ability to change FLAGS_enable_maintenance_manager at runtime,
that can help in avoiding slow restart operations caused due to external factors.
Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 15 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/19398/6
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 6
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 3:
(1 comment)
The test failure in TSAN mode seems related.
http://gerrit.cloudera.org:8080/#/c/19398/3/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19398/3/src/kudu/util/maintenance_manager.cc@311
PS3, Line 311: if (shutdown_) {
Seems we need to acquire the 'lock_' before using 'shutdown_' in this place.
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Feb 2023 14:37:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 2:
(2 comments)
Good advices. Thank you.
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG@10
PS1, Line 10: But
: restart tserver sometimes very slow.
> Is this slowness because of maintenance ops?
The problem 'tserver bootstrap slowly' does not matter with maintenance ops.
This is another problem, its main reason: replaying LMB' metadata/data and replaying wals.
I mean the problem may cause some other maintenance cost.
eg, the patch: https://gerrit.cloudera.org/c/18569/, it's purpuse is that speed tserver's bootstrap
http://gerrit.cloudera.org:8080/#/c/19398/1/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19398/1/src/kudu/util/maintenance_manager.cc@312
PS1, Line 312: if (shutdown_) {
> It would be nice if you give advice how to enable it, like how we do in htt
Done
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Jan 2023 10:49:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 2: Code-Review+1
> Patch Set 2: Verified+1 Code-Review+1
>
> LGTM, but I'm curious what's use case of it, this flag seems only useful in tests.
Yes. You are right.
I also think that this patch is not important, the scenarios of using it may be rare, only some special scenarios should change the flag for administrators.
But the purpose of provided the this flag, it should support change it.
So I decide to submit this patch.
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Jan 2023 07:28:35 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 4:
(1 comment)
> Patch Set 3:
>
> (1 comment)
>
> The test failure in TSAN mode seems related.
Your advices are very useful, I fix it again.
Thanks a lot.
http://gerrit.cloudera.org:8080/#/c/19398/3/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19398/3/src/kudu/util/maintenance_manager.cc@311
PS3, Line 311: MaintenanceOp* op = nullptr;
> Seems we need to acquire the 'lock_' before using 'shutdown_' in this place
You are right
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 4
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 06 Feb 2023 06:42:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG@10
PS1, Line 10: But
: restart tserver sometimes very slow.
Is this slowness because of maintenance ops?
Is there any specific scenario that could cause slowness in tserver restart?
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 1
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 04 Jan 2023 13:58:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Ashwani Raina, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19398
to look at the new patch set (#2).
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
[compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
Now the gflags FLAGS_enable_maintenance_manager can be on/off, it should set
before start. So if we want to change it, we must restart tserver. But
restart tserver sometimes very slow.
This patch support change FLAGS_enable_maintenance_manager at runtime, it
can avoid troublesome restart operation when administrators need do this.
Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 10 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/19398/2
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19398
to look at the new patch set (#5).
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
[compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
Now the gflags FLAGS_enable_maintenance_manager can be on/off, it should set
before start. So if we want to change it, we must restart tservers. But
restarting a tablet server can sometimes be quite time consuming due to
various factors.
This patch adds ability to change FLAGS_enable_maintenance_manager at runtime,
that can help in avoiding slow restart operations caused due to external factors.
Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 13 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/19398/5
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 5
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Feb 2023 14:32:45 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has removed a vote on this change.
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Removed Code-Review+1 by Yifan Zhang <ch...@163.com>
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 6
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 21 Feb 2023 14:41:36 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19398/1/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19398/1/src/kudu/util/maintenance_manager.cc@312
PS1, Line 312: KLOG_EVERY_N(INFO, 1200) << "Maintenance manager is disabled.";
It would be nice if you give advice how to enable it, like how we do in https://github.com/apache/kudu/blob/master/src/kudu/tablet/tablet_replica_mm_ops.cc#L164
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 1
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 04 Jan 2023 15:38:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
[compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
Now the gflags FLAGS_enable_maintenance_manager can be on/off, it should set
before start. So if we want to change it, we must restart tservers. But
restarting a tablet server can sometimes be quite time consuming due to
various factors.
This patch adds ability to change FLAGS_enable_maintenance_manager at runtime,
that can help in avoiding slow restart operations caused due to external factors.
Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Reviewed-on: http://gerrit.cloudera.org:8080/19398
Tested-by: Kudu Jenkins
Reviewed-by: Yingchun Lai <la...@apache.org>
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 15 insertions(+), 5 deletions(-)
Approvals:
Kudu Jenkins: Verified
Yingchun Lai: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has removed a vote on this change.
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 2: -Code-Review
(4 comments)
> Patch Set 2:
>
> (4 comments)
I have fixed them according to your advices.
Thanks a lot.
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG@10
PS1, Line 10: But
: restart tserver sometimes very slow.
> I understand. So, this change is just to add support to change FLAGS_enable
Yes, you are right.
http://gerrit.cloudera.org:8080/#/c/19398/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19398/2//COMMIT_MSG@11
PS2, Line 11: tserver sometimes very slow
> nit: How about this. - "of a tablet server can sometimes be quite time cons
Done
http://gerrit.cloudera.org:8080/#/c/19398/2//COMMIT_MSG@13
PS2, Line 13: This patch support change FLAGS_enable_maintenance_manager at runtime, it
: can avoid troublesome restart operation when administrators need do this.
> How about -
Done
http://gerrit.cloudera.org:8080/#/c/19398/2/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19398/2/src/kudu/util/maintenance_manager.cc@312
PS2, Line 312: if (shutdown_) {
: return;
: }
> shutdown_ check seems to be already present on line num 336 below.
What Yifan said, that's ok. I change it.
As Ashwanl said, L336 is not enough, because when FLAGS_enable_maintenance_manager is false, the thread 'RunSchedulerThread()' may not stop when shutdown.
The idea that moving L336 the beginning of 'while (true)', I think that's ok.
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Feb 2023 03:57:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19398
to look at the new patch set (#3).
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
[compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
Now the gflags FLAGS_enable_maintenance_manager can be on/off, it should set
before start. So if we want to change it, we must restart tservers. But
restarting a tablet server can sometimes be quite time consuming due to
various factors.
This patch adds ability to change FLAGS_enable_maintenance_manager at runtime,
that can help in avoiding slow restart operations caused due to external factors.
Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 11 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/19398/3
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Ashwani Raina, Yingchun Lai, Yifan Zhang, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19398
to look at the new patch set (#4).
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
[compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
Now the gflags FLAGS_enable_maintenance_manager can be on/off, it should set
before start. So if we want to change it, we must restart tservers. But
restarting a tablet server can sometimes be quite time consuming due to
various factors.
This patch adds ability to change FLAGS_enable_maintenance_manager at runtime,
that can help in avoiding slow restart operations caused due to external factors.
Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
---
M src/kudu/util/maintenance_manager.cc
1 file changed, 13 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/19398/4
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 4
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 2: Verified+1 Code-Review+1
LGTM, but I'm curious what's use case of it, this flag seems only useful in tests.
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sun, 15 Jan 2023 14:38:23 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19398/2/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19398/2/src/kudu/util/maintenance_manager.cc@312
PS2, Line 312: if (shutdown_) {
: return;
: }
nit: maybe move this out of the 'if (!FLAGS_enable_maintenance_manager)...' block?
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Jan 2023 08:02:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] support turn on/off FLAGS enable maintenance manager at runtime
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/19398 )
Change subject: [compaction] support turn on/off FLAGS_enable_maintenance_manager at runtime
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19398/1//COMMIT_MSG@10
PS1, Line 10: But
: restart tserver sometimes very slow.
> The problem 'tserver bootstrap slowly' does not matter with maintenance ops
I understand. So, this change is just to add support to change FLAGS_enable_maintenance_manager at runtime, which may indirectly help in cases, where tserver restart could become time consuming (due to other factors), by totally avoiding restart, that was required earlier to bring flag into effect.
http://gerrit.cloudera.org:8080/#/c/19398/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19398/2//COMMIT_MSG@11
PS2, Line 11: tserver sometimes very slow
nit: How about this. - "of a tablet server can sometimes be quite time consuming due to various factors" ?
http://gerrit.cloudera.org:8080/#/c/19398/2//COMMIT_MSG@13
PS2, Line 13: This patch support change FLAGS_enable_maintenance_manager at runtime, it
: can avoid troublesome restart operation when administrators need do this.
How about -
"
This patch adds ability to change FLAGS_enable_maintenance_manager at runtime,
that can help in avoiding slow restart operations caused due to external factors.
"
http://gerrit.cloudera.org:8080/#/c/19398/2/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/19398/2/src/kudu/util/maintenance_manager.cc@312
PS2, Line 312: if (shutdown_) {
: return;
: }
> nit: maybe move this out of the 'if (!FLAGS_enable_maintenance_manager)...'
shutdown_ check seems to be already present on line num 336 below.
Maybe moving the same check from line 336 to outside of "if" could be an option if there is a legit reason for the same.
--
To view, visit http://gerrit.cloudera.org:8080/19398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7029b22a4c8ce58094501e71a6c22271d4f0b2
Gerrit-Change-Number: 19398
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Jan 2023 15:33:23 +0000
Gerrit-HasComments: Yes