Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11956 )
Change subject: [sentry] add mini Sentry to the external mini cluster ...................................................................... Patch Set 10: (5 comments) Can you loop master_sentry-itest 1000 times just to make sure that the "use port 10000 on a UNIQUE_LOOPBACK address" approach is OK? http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/mini-cluster/external_mini_cluster.cc@257 PS2, Line 257: RETURN_NOT_OK_PREPEND(kdc_->CreateServiceKeytab(spn, &ktpath), > Sure, though with the new approach you suggested which can avoid restart fo I think so, because it'll allow macOS to benefit from a restart-less ExternalMiniCluster. http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/mini-cluster/external_mini_cluster.cc@233 PS10, Line 233: we cannot choose a random port which can cause port collision Nit: "we cannot choose a port at random as that can cause a port collision." http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/mini-cluster/external_mini_cluster.cc@236 PS10, Line 236: reconfigured Nit: reconfigure http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/sentry/mini_sentry.cc File src/kudu/sentry/mini_sentry.cc: http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/sentry/mini_sentry.cc@68 PS10, Line 68: void MiniSentry::EnableHms(string hms_uris) { We should CHECK(!sentry_process_) here and in the other new setters. http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/sentry/mini_sentry.cc@137 PS10, Line 137: Status wait = WaitForTcpBind(sentry_process_->pid(), &port_, ip_, If port_ isn't 0 before this call, we might want to check that its value hasn't changed afterwards. The port should have only changed if the original value was 0. -- To view, visit http://gerrit.cloudera.org:8080/11956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 Gerrit-Change-Number: 11956 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 14 Dec 2018 05:26:16 +0000 Gerrit-HasComments: Yes
