Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19237 )
Change subject: [Flag] Check flags consistency when setting a flag ...................................................................... Patch Set 6: Code-Review+1 (4 comments) Overall looks good to me! Just a few nits to fix wording in comments and messages. http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/server/generic_service.cc File src/kudu/server/generic_service.cc: http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/server/generic_service.cc@163 PS6, Line 163: "Roll back flag: $0 to: $1. Please set it manually." nit: not sure the advice to set flag manually is actionable; how about "failed to roll back flag '$0' to previous value '$1'" http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/server/server_base.proto File src/kudu/server/server_base.proto: http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/server/server_base.proto@80 PS6, Line 80: Check all flags for consistency when a flag is set. nit: how about Check all flags for consistency upon setting a new value for a flag. http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/tools/tool_action_common.cc File src/kudu/tools/tool_action_common.cc: http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/tools/tool_action_common.cc@104 PS6, Line 104: If true, kudu will check all " : "flags consistency. If it is inconsistent, set a flag will failed. nit: how about If true, Kudu server checks all flags for consistency upon setting a flag. In this mode, the server rolls the flag back to its previous value and sends corresponding error response if an inconsistency is detected. http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/util/flags.h File src/kudu/util/flags.h: http://gerrit.cloudera.org:8080/#/c/19237/6/src/kudu/util/flags.h@60 PS6, Line 60: // Check unsafe flags, experimental flags, flag group validators. : // If true, the system is in flags consistent. nit: how about Check for unsafe and experimental flags, and run all group flag validators. Returns 'true' if all checks pass and no inconsistencies are detected, 'false' otherwise. -- To view, visit http://gerrit.cloudera.org:8080/19237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e6e37e0f4e72a2cc3d2316248961315f1757ebc Gerrit-Change-Number: 19237 Gerrit-PatchSet: 6 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Sun, 20 Nov 2022 23:26:27 +0000 Gerrit-HasComments: Yes
