Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15161 )
Change subject: [clock] introduce time source auto selection ...................................................................... Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/15161/8/src/kudu/clock/hybrid_clock.cc File src/kudu/clock/hybrid_clock.cc: http://gerrit.cloudera.org:8080/#/c/15161/8/src/kudu/clock/hybrid_clock.cc@433 PS8, Line 433: if (clock::BuiltInNtp::auto_configuration_enabled()) { > I still don't see the use case for --time_source=auto with --builtin_ntp_en I think current code guarantees that only one InstanceDetector is used: the non-default constructor of BuiltinNtp class doesn't activate the InstanceDetector when the set of servers is non-empty, so that question shouldn't bother us too much. So, the important piece is about the use cases we want to support. I agree that the case of '--time_source=auto --builtin_ntp_enable_auto_config=false' doesn't seem to be important. However, there is a use case of '--time_source=builtin --builtin_ntp_enable_auto_config=true', which starts playing once switching the time source to 'builtin'. Does this use case seem useful? I think so, but I can be convinced otherwise. If you agree that the mentioned case is useful then I think we should keep both flags, right? And if keeping both flags, why to impose some implicit dependency, and, consequently, prohibiting the '--time_source=auto --builtin_ntp_enable_auto_config=false' case? -- To view, visit http://gerrit.cloudera.org:8080/15161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9f2bc94f4a1ad32fe365d4f6f0707913fb476a4 Gerrit-Change-Number: 15161 Gerrit-PatchSet: 8 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin <[email protected]> Gerrit-Comment-Date: Wed, 12 Feb 2020 07:14:06 +0000 Gerrit-HasComments: Yes
