Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14866 )
Change subject: [utility] auto-detection of cloud VM instance type ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/14866/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14866/2//COMMIT_MSG@9 PS2, Line 9: it > Nit: a Done http://gerrit.cloudera.org:8080/#/c/14866/2/src/kudu/util/cloud/instance_detector.h File src/kudu/util/cloud/instance_detector.h: http://gerrit.cloudera.org:8080/#/c/14866/2/src/kudu/util/cloud/instance_detector.h@66 PS2, Line 66: : > Could you use our built-in synchronization primitives? Then you can use Mon Done http://gerrit.cloudera.org:8080/#/c/14866/2/src/kudu/util/cloud/instance_detector.h@68 PS2, Line 68: > For production code use kudu::Thread which will be included in metrics and Done http://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.h File src/kudu/util/cloud/instance_metadata.h: PS1: > Continuing the conversation, are you going to attempt to persist the result Probably, but I think that's better to done in a separate changelist. >From the other hand, an extra second on start of kudu-{master,tserver} is >nothing compared with hours loading the on-disk data structures and >bootstrapping tablets. In addition, it would be necessary to provide an extra >flag/tool to clean the persisted information. So, I'm still weighing pros and >contras. http://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.cc File src/kudu/util/cloud/instance_metadata.cc: http://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.cc@86 PS1, Line 86: "169.254.169.123" > Personally I don't mind the hardcoding. These addresses have been defined b Thank you for the feedback. It seems some extra clarification is needed here: I'll re-read the docs making sure there is a clear contract for keeping these parameters constant. -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Greg Solovyev <gsolov...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin <verjov...@cloudera.com> Gerrit-Comment-Date: Sat, 18 Jan 2020 05:04:47 +0000 Gerrit-HasComments: Yes
