Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/14345 )
Change subject: [mini_cluster] small fix on Mini{Chronyd,Hms}::Start()
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14345/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14345/3//COMMIT_MSG@18
PS3, Line 18: mini_chronyd wasn't recreating
: the configuration file for chronyd (the test NTP server) as
necessary
: in MiniChronyd::Start().
> Why did we need to recreate the config file? Which properties changed?
Since we use ephemeral port reservation technique for chronyd, next run of
mini_chronyd from external minicluster will most likely get other port reserved.
I'll add these details.
http://gerrit.cloudera.org:8080/#/c/14345/3/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/14345/3/src/kudu/hms/mini_hms.cc@161
PS3, Line 161: // Run the schematool to initialize the database if not yet
initialized.
> Does this tangibly slow down Start(), since it means starting yet another J
Yep, it slows it down 1/3 (at 2GHz 16 code machine with a lot of memory and HDD
it's about extra 5 seconds). It's also a concern to me.
We could get away fixing only MiniChronyd::Start(), and leaving
MiniHms::Start() as is.
What do you think?
http://gerrit.cloudera.org:8080/#/c/14345/3/src/kudu/mini-cluster/external_mini_cluster-test.cc
File src/kudu/mini-cluster/external_mini_cluster-test.cc:
http://gerrit.cloudera.org:8080/#/c/14345/3/src/kudu/mini-cluster/external_mini_cluster-test.cc@290
PS3, Line 290: // Shutdown the cluster explicitly.
: cluster->Shutdown();
> Could probably elide this; the test's destructor will do it.
Maybe, we still need to test an explicit call of MiniClusterShutdown()? I'm
not sure we want to remove it.
--
To view, visit http://gerrit.cloudera.org:8080/14345
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I014eac84386c4d6e4f025afc1082e6a229a97454
Gerrit-Change-Number: 14345
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 02 Oct 2019 23:54:29 +0000
Gerrit-HasComments: Yes
