[SPARK-29923][SQL][TESTS] Set io.netty.tryReflectionSetAccessible for Arrow on JDK9+

[dongjoon-hyun]

What changes were proposed in this pull request?

This PR aims to add io.netty.tryReflectionSetAccessible=true to the testing configuration for JDK11 because this is an officially documented requirement of Apache Arrow.

Apache Arrow community documented this requirement at 0.15.0 (ARROW-6206).

For java 9 or later, should set "-Dio.netty.tryReflectionSetAccessible=true".

This fixes java.lang.UnsupportedOperationException: sun.misc.Unsafe or java.nio.DirectByteBuffer.(long, int) not available. thrown by netty.

Why are the changes needed?

After ARROW-3191, Arrow Java library requires the property io.netty.tryReflectionSetAccessible to be set to true for JDK >= 9. After #26133, JDK11 Jenkins job seem to fail.

• https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-master-test-maven-hadoop-3.2-jdk-11/676/
• https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-master-test-maven-hadoop-3.2-jdk-11/677/
• https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-master-test-maven-hadoop-3.2-jdk-11/678/

Previous exception in task: 
sun.misc.Unsafe or java.nio.DirectByteBuffer.<init>(long, int) not available&#010;
io.netty.util.internal.PlatformDependent.directBuffer(PlatformDependent.java:473)&#010;
io.netty.buffer.NettyArrowBuf.getDirectBuffer(NettyArrowBuf.java:243)&#010;
io.netty.buffer.NettyArrowBuf.nioBuffer(NettyArrowBuf.java:233)&#010;
io.netty.buffer.ArrowBuf.nioBuffer(ArrowBuf.java:245)&#010;
org.apache.arrow.vector.ipc.message.ArrowRecordBatch.computeBodyLength(ArrowRecordBatch.java:222)&#010; 

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the Jenkins with JDK11.

[dongjoon-hyun]

cc @BryanCutler

[BryanCutler]

@dongjoon-hyun I'm not too familiar with Netty and this issue, but it did come about in https://issues.apache.org/jira/browse/ARROW-3191 for Arrow 0.14.0.

Is this something that will affect consumers of Spark jars?

I actually thought this was defined in the Arrow POM, but looks like it's not possibly due to this comment apache/arrow#4522 (comment). cc @emkornfield who might have some more insight.

[dongjoon-hyun]

Thank you for comments, @BryanCutler . I'm also very surprised because this happens suddenly.

• On Mac environment (with JDK 11.0.5), this doesn't happen with master branch.
• On Linux environment (with JDK 11.0.4), I reproduced this like Jenkins. However, io.netty.tryReflectionSetAccessible doesn't solve this situation.

If we don't have any proper follow-up fix for this, I need to revert your commit about Arrow 0.15.1 because our Jenkins is broken. I'm trying to find a way to avoid that.

[SparkQA]

Test build #113901 has finished for PR 26552 at commit a967d4f.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

I am not sure this change in the PR actually sets the system property for tests. I think it needs to be set in the surefire plugin config. Like in <argLine> add -Dio.netty.tryReflectionSetAccessible=true as a test? If that works, good, but still not a great situation, as it means any JDK 11 users would have to set this.

[dongjoon-hyun]

Yes. This is a headache for Arrow users. I'll switch to argLine. Thanks.

[srowen]

If that works, I'd ask, why not default to allowing this access in Arrow? CCing @BryanCutler as an expert.

[dongjoon-hyun]

Yes. Right.
Apache Arrow community seems to document this at 0.15.0 (apache/arrow#5078).

For java 9 or later, should set "-Dio.netty.tryReflectionSetAccessible=true".
This fixes java.lang.UnsupportedOperationException: sun.misc.Unsafe or java.nio.DirectByteBuffer.(long, int) not available. thrown by netty.

And, this means they don't want to enable this by default in code-side. I don't know the reason either.

[dongjoon-hyun]

Retest this please.

[dongjoon-hyun]

Retest this please.

[dongjoon-hyun]

Oh, @srowen . Do you mean enabling this in our code base (somewhere arrow-related code)?

[SparkQA]

Test build #113903 has finished for PR 26552 at commit 80641b9.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #113904 has finished for PR 26552 at commit 59c126a.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[BryanCutler]

Thanks for helping out with this @dongjoon-hyun and @srowen. I think this should be taken care of in the Arrow code-base since by default it causes a failure, I'm not sure why it wasn't but I'll followup with the Arrow community about it.

In the meantime, I understand if we need to revert the upgrade from #26133 but we should also downgrade the Jenkins pyarrow at the same time or else it will fail too.

It looks like the last test failed also. I don't know where the proper place to put this is, but I'll try to run some tests also.

[SparkQA]

Test build #113906 has finished for PR 26552 at commit 82d894d.

• This patch fails PySpark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[dongjoon-hyun]

No worry, @BryanCutler . There is a progress. It passed all Scala/Java UTs. Now, it seems that we need to add this conf to PySpark/SparkR UT.

[dongjoon-hyun]

cc @HyukjinKwon

[BryanCutler]

For pyspark, the patch below seems to work, but it would only fix unit tests and not all usage

diff --git a/python/run-tests.py b/python/run-tests.py
index 5bcf8b0..ecf5859 100755
--- a/python/run-tests.py
+++ b/python/run-tests.py
@@ -87,6 +87,7 @@ def run_individual_python_test(target_dir, test_name, pyspark_python):
 
     # Also override the JVM's temp directory by setting driver and executor options.
     spark_args = [
+        "--conf", "spark.driver.extraJavaOptions=-Dio.netty.tryReflectionSetAccessible=true",
         "--conf", "spark.driver.extraJavaOptions=-Djava.io.tmpdir={0}".format(tmp_dir),
         "--conf", "spark.executor.extraJavaOptions=-Djava.io.tmpdir={0}".format(tmp_dir),
         "pyspark-shell"

[dongjoon-hyun]

Thank you!

[dongjoon-hyun]

Ur, do we have two spark.driver.extraJavaOptions?

[dongjoon-hyun]

For PySpark, I created another PR to do faster iteration.

• [WIP][test-hadoop3.2][test-java11] JDK11 pyspark test #26553

For SparkR, I created another PR for the same reason. I'll merge them all together into this PR later.

• [WIP][test-hadoop3.2][test-java11] Spark arrow jdk11 r #26554

[SparkQA]

Test build #113911 has finished for PR 26552 at commit e5079d3.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[dongjoon-hyun]

I'll merge this to recover JDK11 Jenkins job.
Please ignore the AppVeyor failure. It fails on both JDK8/JDK11 due to OOM. We are tracking it in the following PR.

• [SPARK-29378][R] Upgrade SparkR to use Arrow 0.15 API #26555

[SparkQA]

Test build #113905 has finished for PR 26552 at commit 82d894d.

• This patch fails PySpark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #113912 has finished for PR 26552 at commit 2c5112e.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[dongjoon-hyun]

We can remove this later after Apache Arrow has the proper logic inside itself.

[srowen]

OK, at least we need to mark this as a release-notes item, until it's undone.

BTW this isn't really an Arrow behavior, but a Netty one:
http://git.yunmaozj.com/opensource/netty/commit/e72c197aa3a88e97c10793ee7870775ab8807f92
I think it's not great to default to not-working on JDK 9+ vs print a warning. But it happened 2 years ago. I guess Spark doesn't trigger the same kind of issue in its use of Netty.

We can try to set this system property at startup in Spark, though I don't know if we can do it early enough. I don't know if Arrow can or wants to try to set this, likewise? like if not set, set to true? This isn't ideal for JDK 9+ users as several things won't work unless their deployment also sets this value - and same could be true of any Arrow user.

See also https://gitter.im/netty/netty?at=5ce3e8d513e9854e334005d7 which suggests using a different allocator avoids this, but I don't know if it's directly helpful. I think the issue is that Arrow is reaching into its PlatformDependent class directly.

[dongjoon-hyun]

Yes. I created SPARK-29924 Document Arrow requirement in JDK9+ for that. Since telease note might be insufficient, I created the issue yesterday.

[emkornfield]

For turning on the flag in Arrow we can discuss it again. When this issue came up the first time, it seemed like it shouldn't be Arrow's job to force this, but I'm happy to reconsider and I suppose we can come up with a mechanism to allow end users to opt out if for some reason there is a need to turn it off (we might also be able to investigate moving away from the components that require this).

[dongjoon-hyun]

Thank you, @emkornfield !

[BryanCutler]

Thanks @emkornfield , I'll start a thread in the Arrow dev list and we can discuss there.

[HyukjinKwon]

Sorry for my late response. LGTM for now.