Adar Dembo has posted comments on this change. Change subject: KUDU-1669. Java client tests leak orphan processes (part 2) ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4636/1/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java: PS1, Line 359: catch (InterruptedException ex) { : // We still need to finish cleaning up. : } This isn't quite correct; if we are interrupted (not that we should be, but Java forces us to handle this case anyway) after waiting for the first master, we should still destroy/remove the remaining processes. Meaning, we should try/catch around each destroyAndWaitForProcess(...) call, and proceed to remove() that process either way. Line 363: thread.interrupt(); It would be nice to avoid using thread interruption here too. I wonder: if we kill and wait for all of the processes associated with the input printers, shouldn't the input printers exit naturally, without the need for explicit interruption? Won't BufferedReader.readLine() throw an EOFException or some such? Also, we should join() on these threads exiting. -- To view, visit http://gerrit.cloudera.org:8080/4636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61d33ca2339048a51acfbb35f5b71e827d3a47f7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
