Adar Dembo has posted comments on this change. Change subject: [java client] Add a way to restart processes ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/2828/1/java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java File java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java: Line 375: Thread.sleep(1000); > Give a chance for the clients to fail reaching that TS (or below that maste It's odd for this behavior to be baked into methods with generic "restart" names. It seems like a semantic only certain "restarters" would care about. The majority will probably just be interested in restarting the server in question without providing some time for ongoing client operations to fail. As a parallel, I'd be surprised to see the same behavior in the generic "restart" methods provided by ExternalMiniCluster. In fact, the approach taking by EMC is to explicitly split Shutdown() and Restart() so that users can call them as they see fit, possibly with their own delay in between. http://gerrit.cloudera.org:8080/#/c/2828/1/java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java File java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java: Line 56: private final Map<Integer, String[]> commandLines = new ConcurrentHashMap<>(); > Three things: A wrapper doesn't have to get in the way if it subclasses Process, overrides all its methods, and routes each overridden method to the wrapped Process. It's annoying boilerplate code, but there aren't that many methods, and once done it's easy to extend in other more interesting ways (such as by adding a command line). The code to start a process would look something like this: ProcessBuilder pb = new ProcessBuilder(cmdLine); Process p = pb.start(); KuduProcess kp = new KuduProcess(p, cmdLine); return kp; -- To view, visit http://gerrit.cloudera.org:8080/2828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I065917cd219e1dd8fa93fed9ca36d58a3372935d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
