Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14227 )

Change subject: [mini_cluster] introduce 'builtin' clock source
......................................................................


Patch Set 3:

(2 comments)

Could you also include a test with the built-in client turned on? Perhaps in 
external_mini_cluster-test?

http://gerrit.cloudera.org:8080/#/c/14227/2/src/kudu/integration-tests/master_migration-itest.cc
File src/kudu/integration-tests/master_migration-itest.cc:

http://gerrit.cloudera.org:8080/#/c/14227/2/src/kudu/integration-tests/master_migration-itest.cc@202
PS2, Line 202:   // unique_ptr wrapper by calling 'std::unique_ptr::reset()'.
> The reason for this change is that upon call of ExternalMinicluster::Shutdo
Do you think Shutdown() ought to stop helper processes too?


http://gerrit.cloudera.org:8080/#/c/14227/3/src/kudu/mini-cluster/external_mini_cluster.h
File src/kudu/mini-cluster/external_mini_cluster.h:

http://gerrit.cloudera.org:8080/#/c/14227/3/src/kudu/mini-cluster/external_mini_cluster.h@190
PS3, Line 190:   // Number of NTP servers to start as part of the cluster. If 0 
or less,
             :   // no NTP servers are started. The NTP servers are used as 
time references
             :   // for the NTP client built into masters and tablet servers.
This is a bit confusing:
- "If 0 or less, no servers are started" --> isn't that obvious?
- It doesn't mention also activating the built-in NTP client.



--
To view, visit http://gerrit.cloudera.org:8080/14227
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c334ae6fa1fb12b033de7f8e8584b8dd3aa2d32
Gerrit-Change-Number: 14227
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Greg Solovyev <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 24 Sep 2019 01:34:01 +0000
Gerrit-HasComments: Yes

Reply via email to