You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2017/08/25 03:05:24 UTC
[kudu-CR] [java] Reuse JVM across tests
Grant Henke has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7825
Change subject: [java] Reuse JVM across tests
......................................................................
[java] Reuse JVM across tests
Currently each java test is run in a separate JVM.This is extra overhead and can cause the tests torun longer. This patch ensures “shared” resources arecleaned up properly across tests and reconfigures thetest runs to reuse the JVM.
Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
---
M java/gradle/tests.gradle
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
M java/pom.xml
7 files changed, 50 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/7825/1
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
[kudu-CR] [java] Reuse JVM across tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: [java] Reuse JVM across tests
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7825/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java:
Line 70: public void testLowTimeoutScans() throws Exception {
I broke these tests out because it looks like sometimes a warm client could actually succeed in 1ms. Its also nice to have a method for each thing tested.
http://gerrit.cloudera.org:8080/#/c/7825/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
Line 350: def clear(): Unit = {
Having cached connection that outlived the client was causing issues when we started a whole new cluster with a new certificates in later tests. Is their any real-world issue having JVM wide cached connections like this could cause?
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] Reuse JVM across tests
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [java] Reuse JVM across tests
......................................................................
Patch Set 1:
> Having the Java tests run in dist-test would be really nice. We can't parallelize the unit tests locally because they have collisions on the MiniCluster usage.
yea, that could be solved the same way that the C++ client solves it, though (having the minicluster daemons bind to 127.a.b.c rather than 127.0.0.1). Doesn't work on OSX but c'est la vie
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] Reuse JVM across tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [java] Reuse JVM across tests
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7825/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
Line 350: def clear(): Unit = {
> Having cached connection that outlived the client was causing issues when w
Interesting. Are those issues related only to the TLS certs or it's something else? As I understand, in the real-world scenarios, update on TLS certificates is supposed to be a very rare occasion: it might be expiration of the Kudu's IPKI certs (in 10 years after generation) or some case of re-generating those in case of security concerns.
Anyway, I think clearing the cache might be good at least because not doing so might hide some issues with initial cache bootstrap for some corner cases.
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [java] Reuse JVM across tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has abandoned this change. ( http://gerrit.cloudera.org:8080/7825 )
Change subject: [java] Reuse JVM across tests
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-Change-Number: 7825
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [java] Reuse JVM across tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: [java] Reuse JVM across tests
......................................................................
Patch Set 1:
Having the Java tests run in dist-test would be really nice. We can't parallelize the unit tests locally because they have collisions on the MiniCluster usage.
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] Reuse JVM across tests
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [java] Reuse JVM across tests
......................................................................
Patch Set 1:
Not sure how I feel about this change. In my experience these kinds of things usually cause more hard-to-diagnose bugs than they are worth -- eg random tests end up leaving threads running in failure scenarios, and those threads end up logging stuff captured by future tests, interfering with them, etc.
If we want to improve test runtime I think we're better off focusing on distributed execution or parallel execution across many VMs, but still having a one-vm-per-test.
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [java] Reuse JVM across tests
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [java] Reuse JVM across tests
......................................................................
Patch Set 1:
(1 comment)
Just passing through with a question.
http://gerrit.cloudera.org:8080/#/c/7825/1//COMMIT_MSG
Commit Message:
Line 7: [java] Reuse JVM across tests
Could you measure the change in test running time and note it here?
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java] Reuse JVM across tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: [java] Reuse JVM across tests
......................................................................
Patch Set 1:
Yeah, I agree with "guaranteed isolation" being more reliable. This patch came as a result of the gradle patch where weird issues were seen because I didn't use a different JVM per test.
We can leave 1 JVM per test class, but these teardown fixes are still valid.
--
To view, visit http://gerrit.cloudera.org:8080/7825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e4c6d53054d5da4f2ecf9f4130a51df13d4fa6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No