[ZEPPELIN-5875] Fix flaky tests in core-modules
#4556
Conversation
jongyoul
changed the title
TimeoutLifecycleManagerTestcore-modules
| @@ -117,6 +117,8 @@ | |||
| <exclude>org/w3c/dom/**/*</exclude> | |||
| <exclude>org/xml/sax/*</exclude> | |||
| <exclude>org/xml/sax/**/*</exclude> | |||
|
|
|||
| <exclude>org/eclipse/sisu/**</exclude> | |||
There was a problem hiding this comment.
It's because
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] Bad service configuration file, or exception thrown while constructing Processor object: javax.annotation.processing.Processor: jar:file:/Users/jl/Works/Projects/zeppelin/zeppelin-interpreter-shaded/target/zeppelin-interpreter-shaded-0.11.0-SNAPSHOT.jar!/META-INF/services/javax.annotation.processing.Processor:1: Illegal provider-class name: org/apache/zeppelin/shaded/org.apache.zeppelin.shaded.org.eclipse.sisu.space.SisuIndexAPT6
| @@ -44,6 +45,7 @@ public PooledRemoteClient(SupplierWithIO<T> supplier, int connectionPoolSize) { | |||
| GenericObjectPoolConfig poolConfig = new GenericObjectPoolConfig(); | |||
| poolConfig.setMaxTotal(connectionPoolSize); | |||
| poolConfig.setMaxIdle(connectionPoolSize); | |||
| poolConfig.setEvictionPolicyClassName(DefaultEvictionPolicy.class.getName()); | |||
There was a problem hiding this comment.
It's similar to the above case. We can add ignore rule for shade as well but I think it's better to set up the class name explicitly
Error: Exception in thread "main" java.lang.IllegalArgumentException: Unable to create EvictionPolicy instance of type org/apache/zeppelin/shaded/org.apache.zeppelin.shaded.org.apache.commons.pool2.impl.DefaultEvictionPolicy
| @@ -113,6 +113,7 @@ | |||
| <groupId>org.apache.maven.plugins</groupId> | |||
| <artifactId>maven-shade-plugin</artifactId> | |||
| <configuration> | |||
| <createDependencyReducedPom>false</createDependencyReducedPom> | |||
There was a problem hiding this comment.
It's a bit complicated but let me explain it.
When we install jar, maven-instll-plugin tries to install the file under /target/*.jar into .m2 but our shade script store shaded jars under /interpreter/{interpreter.name} so the file under /target directory is an original file which is not shaded. so maven-install-plugin copies the original file into .m2 directly as a result. BTW, in case of pom file, dependency-reduced-pom would be copied into .m2 directory even if the jar is not shaded. In our old version of shade plugin, the default setting didn't create dependency-reduced-pom when using outputFile configuration but in the latest one, the plugin creates it even if we use outputFile setting. So I disabled creating it.
|
CI is all green now. |
|
Thanks @jongyoul |
| @@ -59,7 +59,8 @@ jobs: | |||
| !~/.m2/repository/org/apache/zeppelin/ | |||
| ~/.spark-dist | |||
| ~/.cache | |||
| key: ${{ runner.os }}-zeppelin-${{ hashFiles('**/pom.xml') }} | |||
| ~/conda_pkgs_dir | |||
| key: ${{ runner.os }}-zeppelin-${{ hashFiles('**/pom.xml') }}-${{ hashFiles('testing/env_python_3.7_with_R.yml') }} | |||
There was a problem hiding this comment.
Why ${{ hashFiles('testing/env_python_3.7_with_R.yml') }}? We do not have a conda environment in the CI cache.
There was a problem hiding this comment.
Oh? really? I didn't check the time carefully but felt like it reduce the whole time to setup conda but didn't it?
There was a problem hiding this comment.
I'm afraid I don't have numbers, do you have numbers?
In order not to confuse others we should remove the suffix again.
| @@ -44,6 +45,9 @@ public PooledRemoteClient(SupplierWithIO<T> supplier, int connectionPoolSize) { | |||
| GenericObjectPoolConfig poolConfig = new GenericObjectPoolConfig(); | |||
| poolConfig.setMaxTotal(connectionPoolSize); | |||
| poolConfig.setMaxIdle(connectionPoolSize); | |||
| // ZEPPELIN-5875 maven-shade-plugin issue | |||
| // `org/apache/zeppelin/shaded/org.apache.zeppelin.shaded.org.apache.commons.pool2.impl.DefaultEvictionPolicy` | |||
There was a problem hiding this comment.
I think we still have a shade problem.
org/apache/zeppelin/shaded/org.apache.zeppelin.shaded.org.apache.commons.pool2.impl.DefaultEvictionPolicy does not look like a good class name.
There was a problem hiding this comment.
Yes, correct. It's a kind of shade configuration issue. We need to change it but I thought we should make it normal at first. and Then, we could improve it.
Agreed. It would be great if we could handle all thing this time. |
What is this PR for?
Fixing flaky test in
core-modules (hadoop[2|3]).What type of PR is it?
Bug Fix
Todos
testTimeout_2What is the Jira issue?
How should this be tested?
Screenshots (if appropriate)
Questions: