Dinesh Bhat has posted comments on this change. Change subject: KUDU-1741: Make MiniCluster and ExternalMiniCluster follow one semantic for Restart ......................................................................
Patch Set 1: > Intuitively restart means "stop then start" for me, and as shown by > your patch there's definitely a need for a method that does both. > IMO if you want to keep restart to mean "start a stopped process" > then that's fine but we should also have something that stops then > stars. TFTR JD, yeah I agree and current MiniCluster::Restart was more intuitive in that sense; I changed the MiniCluster(against the intuitive way) instead of ExternalMiniCluster mainly because ExternalMiniCluster was more widely used by pretty much all unit tests and integration tests. ExternalMiniCluster/ExternalMaster/ExternalTabletServer expect the cluster/nodes to have been shutdown before restart. We could : a) change the ExternalMiniCluster::Restart to internally do a shutdown/start to make it intuitive (follow MiniCluster semantics). b) Add a new API ShutDownAndStart, which would achieve what you are suggesting. I think a) sounds like more reasonable approach. Let me know what you think. -- To view, visit http://gerrit.cloudera.org:8080/5598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iad33b7c46bfca3f277ccbca7d0420272f06a6633 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-HasComments: No
