New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get tests working on java 9 #10228

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@rmuir
Contributor

rmuir commented Mar 23, 2015

Fix build for java9.

  • add compiler workarounds for java 9 (compiler bug, JI-9019884 filed for this)
  • remove permgen specification during tests (this results in an error)

Fixes #10145

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Mar 23, 2015

Contributor

LGTM I guess we need to drop the permgen space from the init scripts as well as some point since java 9 doesn't like it

Contributor

s1monw commented Mar 23, 2015

LGTM I guess we need to drop the permgen space from the init scripts as well as some point since java 9 doesn't like it

@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Mar 23, 2015

Contributor

When running tests on java9 i hit IAE from java.util.concurrent.ThreadPoolExecutor.setCorePoolSize().

Maybe this should be fixed as a separate issue, but I have not seen it before.

Suite: org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests
1> [2015-03-23 17:52:16,761][INFO ][test ] Test testShutdownDownNowDoesntBlock(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,764][INFO ][test ] Test testShutdownDownNowDoesntBlock(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,765][INFO ][test ] Test testScalingExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,767][INFO ][test ] Test testScalingExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,767][INFO ][test ] Test testFixedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,769][INFO ][test ] Test testFixedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,769][INFO ][test ] Test testCachedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,772][INFO ][test ] Test testCachedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,772][INFO ][test ] Test testCustomThreadPool(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,777][ERROR][test ] FAILURE : testCustomThreadPool(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests)
1> REPRODUCE WITH : mvn clean test -Dtests.seed=216A1B2C2D2B1E3C -Dtests.class=org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests -Dtests.method="testCustomThreadPool" -Des.logger.level=INFO -Dtests.heap.size=512m -Dtests.locale=ru_RU -Dtests.timezone=America/Rankin_Inlet -Dtests.processors=8
1> Throwable:
1> java.lang.IllegalArgumentException
1> __randomizedtesting.SeedInfo.seed([216A1B2C2D2B1E3C:82BF82836AB18119]:0)
1> java.util.concurrent.ThreadPoolExecutor.setCorePoolSize(ThreadPoolExecutor.java:1541)
1> org.elasticsearch.threadpool.ThreadPool.rebuild(ThreadPool.java:345)
1> org.elasticsearch.threadpool.ThreadPool.updateSettings(ThreadPool.java:441)

Contributor

rmuir commented Mar 23, 2015

When running tests on java9 i hit IAE from java.util.concurrent.ThreadPoolExecutor.setCorePoolSize().

Maybe this should be fixed as a separate issue, but I have not seen it before.

Suite: org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests
1> [2015-03-23 17:52:16,761][INFO ][test ] Test testShutdownDownNowDoesntBlock(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,764][INFO ][test ] Test testShutdownDownNowDoesntBlock(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,765][INFO ][test ] Test testScalingExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,767][INFO ][test ] Test testScalingExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,767][INFO ][test ] Test testFixedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,769][INFO ][test ] Test testFixedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,769][INFO ][test ] Test testCachedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,772][INFO ][test ] Test testCachedExecutorType(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) finished
1> [2015-03-23 17:52:16,772][INFO ][test ] Test testCustomThreadPool(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests) started
1> [2015-03-23 17:52:16,777][ERROR][test ] FAILURE : testCustomThreadPool(org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests)
1> REPRODUCE WITH : mvn clean test -Dtests.seed=216A1B2C2D2B1E3C -Dtests.class=org.elasticsearch.threadpool.UpdateThreadPoolSettingsTests -Dtests.method="testCustomThreadPool" -Des.logger.level=INFO -Dtests.heap.size=512m -Dtests.locale=ru_RU -Dtests.timezone=America/Rankin_Inlet -Dtests.processors=8
1> Throwable:
1> java.lang.IllegalArgumentException
1> __randomizedtesting.SeedInfo.seed([216A1B2C2D2B1E3C:82BF82836AB18119]:0)
1> java.util.concurrent.ThreadPoolExecutor.setCorePoolSize(ThreadPoolExecutor.java:1541)
1> org.elasticsearch.threadpool.ThreadPool.rebuild(ThreadPool.java:345)
1> org.elasticsearch.threadpool.ThreadPool.updateSettings(ThreadPool.java:441)

@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Mar 23, 2015

Contributor

In java8, javadocs for this method look like:

Throws:
IllegalArgumentException - if corePoolSize < 0

But in java9 its changed, we are hitting the second condition:

 * @throws IllegalArgumentException if {@code corePoolSize < 0}
 *         or {@code corePoolSize} is greater than the {@linkplain
 *         #getMaximumPoolSize() maximum pool size}
Contributor

rmuir commented Mar 23, 2015

In java8, javadocs for this method look like:

Throws:
IllegalArgumentException - if corePoolSize < 0

But in java9 its changed, we are hitting the second condition:

 * @throws IllegalArgumentException if {@code corePoolSize < 0}
 *         or {@code corePoolSize} is greater than the {@linkplain
 *         #getMaximumPoolSize() maximum pool size}
@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Mar 23, 2015

Contributor

I have no idea how to fix the code to dynamically increase threadpool sizes.

With the new check, it seems impossible: setCorePoolSize complains if its > maximumPoolSize, but setMaximumPoolSize complains if its < corePoolSize. This check was added here: https://bugs.openjdk.java.net/browse/JDK-7153400

I feel like i'm losing my mind, maybe someone else can take a second look here?

Contributor

rmuir commented Mar 23, 2015

I have no idea how to fix the code to dynamically increase threadpool sizes.

With the new check, it seems impossible: setCorePoolSize complains if its > maximumPoolSize, but setMaximumPoolSize complains if its < corePoolSize. This check was added here: https://bugs.openjdk.java.net/browse/JDK-7153400

I feel like i'm losing my mind, maybe someone else can take a second look here?

@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Mar 23, 2015

Contributor

I guess, depending if we are growing or shrinking the pool, we have to do the code either:

setMaximumPoolSize(x);
setCorePoolSize(y);

or

setCorePoolSize(y);
setMaximumPoolSize(x);

This seems pretty crazy to me, i can't imagine its intended. Its just like ConcurrentMergeScheduler in lucene used to be.

Contributor

rmuir commented Mar 23, 2015

I guess, depending if we are growing or shrinking the pool, we have to do the code either:

setMaximumPoolSize(x);
setCorePoolSize(y);

or

setCorePoolSize(y);
setMaximumPoolSize(x);

This seems pretty crazy to me, i can't imagine its intended. Its just like ConcurrentMergeScheduler in lucene used to be.

@rmuir rmuir added the review label Mar 23, 2015

@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Mar 23, 2015

Contributor

I fixed threadpool sizing with rmuir@88c3934

Please have a look, I think its ready. All tests pass on java9 now:

[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 09:02 min
[INFO] Finished at: 2015-03-23T19:53:03-05:00
[INFO] Final Memory: 38M/1404M
[INFO] ------------------------------------------------------------------------

Contributor

rmuir commented Mar 23, 2015

I fixed threadpool sizing with rmuir@88c3934

Please have a look, I think its ready. All tests pass on java9 now:

[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 09:02 min
[INFO] Finished at: 2015-03-23T19:53:03-05:00
[INFO] Final Memory: 38M/1404M
[INFO] ------------------------------------------------------------------------

@rjernst

This comment has been minimized.

Show comment
Hide comment
@rjernst

rjernst Mar 24, 2015

Member

LGTM

Member

rjernst commented Mar 24, 2015

LGTM

@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Mar 24, 2015

Contributor

LGTM I guess we need to drop the permgen space from the init scripts as well as some point since java 9 doesn't like it

I don't think we are setting this anymore. We were only setting it for tests. I grepped for MaxPermSize and don't see it anywhere else, except dependency-reduced-pom.xml, which is under .gitignore?!

Contributor

rmuir commented Mar 24, 2015

LGTM I guess we need to drop the permgen space from the init scripts as well as some point since java 9 doesn't like it

I don't think we are setting this anymore. We were only setting it for tests. I grepped for MaxPermSize and don't see it anywhere else, except dependency-reduced-pom.xml, which is under .gitignore?!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment