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 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/19237/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19237/1//COMMIT_MSG@15 PS1, Line 15: check flags consistency when : setting a flag One issue with this approach I can see is that once this consistency check is enabled, it will be impossible to change values for flags which must be set as a group. For example, take a look at --rpc_certificate_file, --rpc_private_key_file, --rpc_ca_certificate_file flags. They are not runtime as of now, but you can look at them just for an example. To overcome that limitation once the validation is enabled, I think it makes sense to introduce a new RPC to set multiple flags at once. That can be taken care of in a separate patch. http://gerrit.cloudera.org:8080/#/c/19237/1/src/kudu/util/flags.cc File src/kudu/util/flags.cc: http://gerrit.cloudera.org:8080/#/c/19237/1/src/kudu/util/flags.cc@551 PS1, Line 551: bool CheckFlagsInconsistence() { > This current code misses running the regular (i.e. non-custom) validators f After some consideration, I realized the standard validators defined by DEFINE_validator() are applied upon setting the flag upon calling google::SetCommandLineOption(), so those are not necessary to check here. -- 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: 1 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 12 Nov 2022 22:43:49 +0000 Gerrit-HasComments: Yes
