Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14866 )
Change subject: [utility] auto-detection of cloud VM instance type ...................................................................... Patch Set 4: Code-Review+1 (6 comments) http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector-test.cc File src/kudu/util/cloud/instance_detector-test.cc: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector-test.cc@83 PS4, Line 83: FromMicroseconds To reduce possibility of flaky tests, can we go lower than this, FromNanoseconds()? http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h File src/kudu/util/cloud/instance_detector.h: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@38 PS4, Line 38: running running-> run http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@56 PS4, Line 56: autodetection auto-detection http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@60 PS4, Line 60: cloude typo http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@70 PS4, Line 70: result_metadata_ I can't find this field in this change. Perhaps it was renamed. http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.cc File src/kudu/util/cloud/instance_detector.cc: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.cc@84 PS4, Line 84: result_detector_idx_ == kNoIdx An instance can't be running on multiple cloud vendors, so the first one to update result_detector_idx_ wins the race and breaks the loop in case of spurious wake ups for other threads? Might be worth adding a comment. -- 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: 4 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: Tue, 21 Jan 2020 22:31:17 +0000 Gerrit-HasComments: Yes