Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24475 )

Change subject: KUDU-3785: Fix CLI flag validator failure
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24475/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

http://gerrit.cloudera.org:8080/#/c/24475/1/src/kudu/tools/tool_main.cc@300
PS1, Line 300:   CHECK_NE("",  google::SetCommandLineOptionWithMode(
             :       "tablet_transaction_memory_limit_mb",
             :       "-1",
             :       google::SET_FLAGS_VALUE));
I don't think it's a good idea to explicitly set flags via 
google::SET_FLAGS_VALUE here, at least because of overriding the 
--tablet_transaction_memory_limit_mb settings that the test harness uses when 
starting kudu-master and kudu-tserver via the CLI tool.  Contrary to that, 
using SET_FLAGS_DEFAULT is generally safe because it doesn't override the 
explicitly set values.

This is the case when it's desirable to have some sort of behavior that's 
similar to SET_FLAG_DEFAULT, but applied to a group of flags.  I guess it might 
be a separate task on its own to implement it as a generic/library routine.

I think it's OK to implement such logic for this particular situation only: add 
an extra code here to set the new defaults for 
--tablet_transaction_memory_limit_mb and --rpc_max_message_size via 
SET_FLAGS_DEFAULT only if neither of the flags has been explicitly set already.



--
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: 1
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: Wed, 17 Jun 2026 22:46:22 +0000
Gerrit-HasComments: Yes

Reply via email to