[SPARK-36704][CORE] Expand exception handling to more Java 9 cases where reflection is limited at runtime, when reflecting to manage DirectByteBuffer settings #33947

srowen · 2021-09-09T17:29:01Z

What changes were proposed in this pull request?

Improve exception handling in the Platform initialization, where it attempts to assess whether reflection is possible to modify DirectByteBuffer. This can apparently fail in more cases on Java 9+ than are currently handled, whereas Spark can continue without reflection if needed.

More detailed comments on the change inline.

Why are the changes needed?

This exception seems to be possible and fails startup:

Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make private java.nio.DirectByteBuffer(long,int) accessible: module java.base does not "opens java.nio" to unnamed module @71e9ddb4
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357)
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
        at java.base/java.lang.reflect.Constructor.checkCanSetAccessible(Constructor.java:188)
        at java.base/java.lang.reflect.Constructor.setAccessible(Constructor.java:181)
        at org.apache.spark.unsafe.Platform.<clinit>(Platform.java:56)

Does this PR introduce any user-facing change?

Should strictly allow Spark to continue in more cases.

How was this patch tested?

Existing tests.

…mited at runtime, when reflecting to manage DirectByteBuffer settings

srowen · 2021-09-09T17:29:34Z

common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java

-      // hack below. It doesn't break, just means the user might run into the default JVM limit
-      // on off-heap memory and increase it or set the flag above. This tests whether it's
-      // available:
+      Class<?> cls = Class.forName("java.nio.DirectByteBuffer");


This was simply moved from above to further unify handling of exceptions during init

srowen · 2021-09-09T17:30:09Z

common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java

+        cleanerField.setAccessible(true);
+      } catch (RuntimeException re) {
+        // This is a Java 9+ exception, so needs to be handled without importing it
+        if ("InaccessibleObjectException".equals(re.getClass().getSimpleName())) {


This is really the new exception handling; now have to catch this and also handle the case where DBB_CONSTRUCTOR and DBB_CLEANER_FIELD aren't available.

srowen · 2021-09-09T17:30:55Z

If this is OK, this should go into 3.2.x for 3.2.0 if possible

SparkQA · 2021-09-09T18:17:09Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47627/

SparkQA · 2021-09-09T18:26:53Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47627/

SparkQA · 2021-09-09T20:07:34Z

Test build #143123 has finished for PR 33947 at commit 09d7287.

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

dongjoon-hyun · 2021-09-09T23:22:04Z

Thank you, @srowen . +1 for backporting from my side. cc @gengliangwang

If this is OK, this should go into 3.2.x for 3.2.0 if possible

common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java

mridulm · 2021-09-10T03:21:54Z

Rest of the changes look fine, +1 to backporting to 3.2

gengliangwang · 2021-09-10T06:26:51Z

+1 to backporting to 3.2, too

SparkQA · 2021-09-10T12:15:46Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47649/

SparkQA · 2021-09-10T12:24:32Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47649/

SparkQA · 2021-09-10T14:09:36Z

Test build #143145 has finished for PR 33947 at commit c30df80.

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

mridulm

LGTM, thanks for fixing this @srowen !

…ere reflection is limited at runtime, when reflecting to manage DirectByteBuffer settings ### What changes were proposed in this pull request? Improve exception handling in the Platform initialization, where it attempts to assess whether reflection is possible to modify DirectByteBuffer. This can apparently fail in more cases on Java 9+ than are currently handled, whereas Spark can continue without reflection if needed. More detailed comments on the change inline. ### Why are the changes needed? This exception seems to be possible and fails startup: ``` Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make private java.nio.DirectByteBuffer(long,int) accessible: module java.base does not "opens java.nio" to unnamed module 71e9ddb4 at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357) at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297) at java.base/java.lang.reflect.Constructor.checkCanSetAccessible(Constructor.java:188) at java.base/java.lang.reflect.Constructor.setAccessible(Constructor.java:181) at org.apache.spark.unsafe.Platform.<clinit>(Platform.java:56) ``` ### Does this PR introduce _any_ user-facing change? Should strictly allow Spark to continue in more cases. ### How was this patch tested? Existing tests. Closes #33947 from srowen/SPARK-36704. Authored-by: Sean Owen <[email protected]> Signed-off-by: Sean Owen <[email protected]> (cherry picked from commit e5283f5) Signed-off-by: Sean Owen <[email protected]>

srowen · 2021-09-11T18:39:28Z

Merged to master/3.2/3.1/3.0

…ere reflection is limited at runtime, when reflecting to manage DirectByteBuffer settings ### What changes were proposed in this pull request? Improve exception handling in the Platform initialization, where it attempts to assess whether reflection is possible to modify DirectByteBuffer. This can apparently fail in more cases on Java 9+ than are currently handled, whereas Spark can continue without reflection if needed. More detailed comments on the change inline. ### Why are the changes needed? This exception seems to be possible and fails startup: ``` Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make private java.nio.DirectByteBuffer(long,int) accessible: module java.base does not "opens java.nio" to unnamed module 71e9ddb4 at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357) at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297) at java.base/java.lang.reflect.Constructor.checkCanSetAccessible(Constructor.java:188) at java.base/java.lang.reflect.Constructor.setAccessible(Constructor.java:181) at org.apache.spark.unsafe.Platform.<clinit>(Platform.java:56) ``` ### Does this PR introduce _any_ user-facing change? Should strictly allow Spark to continue in more cases. ### How was this patch tested? Existing tests. Closes apache#33947 from srowen/SPARK-36704. Authored-by: Sean Owen <[email protected]> Signed-off-by: Sean Owen <[email protected]> (cherry picked from commit e5283f5) Signed-off-by: Sean Owen <[email protected]>

Expand exception handling to more Java 9 cases where reflection is li…

09d7287

…mited at runtime, when reflecting to manage DirectByteBuffer settings

srowen self-assigned this Sep 9, 2021

github-actions bot added the SQL label Sep 9, 2021

srowen commented Sep 9, 2021

View reviewed changes

mridulm reviewed Sep 10, 2021

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java Show resolved Hide resolved

Fix error in exception handling

c30df80

mridulm approved these changes Sep 10, 2021

View reviewed changes

srowen closed this in e5283f5 Sep 11, 2021

srowen deleted the SPARK-36704 branch November 24, 2021 15:11

xerial mentioned this pull request Jun 28, 2022

Support JDK17 msgpack/msgpack-java#660

Merged

serge-rider mentioned this pull request May 11, 2023

Error occured while deserializing arrow data: sun.misc.Unsafe or java.nio.DirectByteBuffer.<init>(long, int) not available dbeaver/dbeaver#18962

Closed

[SPARK-36704][CORE] Expand exception handling to more Java 9 cases where reflection is limited at runtime, when reflecting to manage DirectByteBuffer settings #33947

[SPARK-36704][CORE] Expand exception handling to more Java 9 cases where reflection is limited at runtime, when reflecting to manage DirectByteBuffer settings #33947

Uh oh!

Conversation

srowen commented Sep 9, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

srowen Sep 9, 2021

Choose a reason for hiding this comment

Uh oh!

srowen Sep 9, 2021

Choose a reason for hiding this comment

Uh oh!

srowen commented Sep 9, 2021

Uh oh!

SparkQA commented Sep 9, 2021

Uh oh!

SparkQA commented Sep 9, 2021

Uh oh!

SparkQA commented Sep 9, 2021

Uh oh!

dongjoon-hyun commented Sep 9, 2021

Uh oh!

Uh oh!

mridulm commented Sep 10, 2021

Uh oh!

gengliangwang commented Sep 10, 2021

Uh oh!

SparkQA commented Sep 10, 2021

Uh oh!

SparkQA commented Sep 10, 2021

Uh oh!

SparkQA commented Sep 10, 2021

Uh oh!

mridulm left a comment

Choose a reason for hiding this comment

Uh oh!

srowen commented Sep 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants