You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org> on 2016/10/03 17:00:04 UTC

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Jean-Daniel Cryans has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4598

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................

KUDU-1669. Java ITClient test can orphan processes

ITClient#configureAndStartProcess now catches the InterruptedException and cleans up the newly
created process.

Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
---
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
1 file changed, 16 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/4598/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4598
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4598

to look at the new patch set (#3).

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................

KUDU-1669. Java ITClient test can orphan processes

This removes the interruption of threads, which is the reason why we were orphaning
processes (in the ChaosThread).

Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
---
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
1 file changed, 2 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/4598/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4598
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................


KUDU-1669. Java ITClient test can orphan processes

This removes the interruption of threads, which is the reason why we were orphaning
processes (in the ChaosThread).

Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Reviewed-on: http://gerrit.cloudera.org:8080/4598
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
1 file changed, 2 insertions(+), 3 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4598/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:

I really don't like how ITClient uses thread interruption to clean up, and this workaround just makes the affected code even messier. Instead, can you modify ITClient to use cooperative thread signaling for clean up? It should be straight-forward; it's how we clean up every single test thread in the C++ tests.


Line 237:     PROCESS_INPUT_PRINTERS.add(thread);
Let's defer this until after the try/catch below. Then we won't need a corresponding remove() to clean up in the event of an error.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4598

to look at the new patch set (#2).

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................

KUDU-1669. Java ITClient test can orphan processes

This removes the interruption of threads, which is the reason why we were orphaning
processes (in the ChaosThread).

Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
---
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/4598/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4598
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................


Patch Set 1:

BTW, the test failure appears to be exactly the one I filed KUDU-1669 about. Could you look into it? Maybe this fix alone isn't enough in some way.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1669. Java ITClient test can orphan processes

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: KUDU-1669. Java ITClient test can orphan processes
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4598/2/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java:

Line 169:           e.printStackTrace();
Probably don't care for this anymore.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0085323ba9ea544488c6ce896d8627f24ea69f4b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes