Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24475 )
Change subject: KUDU-3785: Fix CLI flag validator failure ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/24475/4/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/24475/4/src/kudu/tools/kudu-tool-test.cc@11160 PS4, Line 11160: Edge case: > Why is it important to not use SetCommandLineOption(), but use an assignmen The fact that it is a 'nit' comment and I have already +1'ed, clearly says that this is not important, in case you missed that. The point of doing so is to ensure that there is no difference in gflag behavior (i.e. outcome of google::GetCommandLineFlagInfoOrDie("rpc_max_message_size").is_default) between when the flag is set to default value a) via SetCommandLineOption() and b) via FLAGS_rpc_max_message_size assignment directly. This point was clearly stated in one of my earlier comments here: https://gerrit.cloudera.org/#/c/24475/3/src/kudu/tools/tool_action_common.cc@558 I just did a quick verification check of gflag behavior if flag is set via variable assignment - turns out, it fails because the gflag code sets 'modified_' to true only when current value is not same as default value: +++ void CommandLineFlag::UpdateModifiedBit() { // Update the "modified" bit in case somebody bypassed the // Flags API and wrote directly through the FLAGS_name variable. if (!modified_ && !current_->Equal(*defvalue_)) { modified_ = true; } } +++ Even with the failure, it should be ok to go ahead with the change because such direct assignments in the code are not usual scenarios in the deployments, probably just limited to the tests. With that info, it should be ok to document this in the form of either comments here or any other way that seems fit. -- To view, visit http://gerrit.cloudera.org:8080/24475 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd24ec605bd14ce90ed8a5705230ff96d77d0d64 Gerrit-Change-Number: 24475 Gerrit-PatchSet: 4 Gerrit-Owner: Marton Greber <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Tue, 23 Jun 2026 05:26:51 +0000 Gerrit-HasComments: Yes
