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 <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Mon, 16 Jan 2023 15:33:23 +0000
Gerrit-HasComments: Yes

Reply via email to