Adar Dembo has posted comments on this change.

Change subject: [java client] Add a way to restart processes
......................................................................


Patch Set 1:

(1 comment)

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<>();
> > Because you don't want to add "never removed from" behavior to  masterPro
> A TS that shows up on a different port but with the same UUID is currently 
> not handled, so the test would fail anyways.

In the C++ mini-cluster, there is functionality to add new servers to an 
existing cluster. When that functionality is added here, this historical map 
will become unsafe because there could be port collisions between stopped 
processes, each of which represents a _different_ server (not the same server 
with the same UUID).

So once again, I urge you to consider a more robust solution here. It'll be 
more short term work for sure, but I think it'll pay off in the longer term.


-- 
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

Reply via email to