You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/10/22 13:01:09 UTC
[GitHub] [spark] LuciferYang opened a new pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
LuciferYang opened a new pull request #34368:
URL: https://github.com/apache/spark/pull/34368
### What changes were proposed in this pull request?
There are 9 UTs failed in `repl` run with Java 17, the all failed tests have similar error stack as follows:
```
java.lang.IllegalAccessException: Can not set final $iw field $Lambda$2879/0x000000080188b928.arg$1 to $iw
at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:76)
at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:80)
at java.base/jdk.internal.reflect.UnsafeQualifiedObjectFieldAccessorImpl.set(UnsafeQualifiedObjectFieldAccessorImpl.java:79)
at java.base/java.lang.reflect.Field.set(Field.java:799)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:398)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:162)
at org.apache.spark.SparkContext.clean(SparkContext.scala:2490)
at org.apache.spark.rdd.RDD.$anonfun$map$1(RDD.scala:414)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:151)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:112)
at org.apache.spark.rdd.RDD.withScope(RDD.scala:406)
at org.apache.spark.rdd.RDD.map(RDD.scala:413)
... 95 elided
```
We know from error stack that we can't reset a `read-only` field `arg$1` to `$iw` through the reflection api when use Java 17, the `read-only` field is defined as `private final field (trustedFinal=true)` .
So this pr try to remove the `final` modifier before `outerField.set(func, clonedOuterThis)` in `ClosureCleaner#clean` method and reset the modifier value after `outerField.set(func, clonedOuterThis)` when use Java 17.
At the same time, in order to use the API in `java.lang.reflect` mod, `--add-opens=java.base/java.lang.reflect=ALL-UNNAMED` is added to `pom.xml`, `SparkBuild.scala` and `JavaModuleOptions.java`
### Why are the changes needed?
Pass UT with JDK 17
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
- Pass the Jenkins or GitHub Action
- Manual test use Java 17
```
mvn clean install -pl repl
```
**Before**
```
Run completed in 30 seconds, 826 milliseconds.
Total number of tests run: 42
Suites: completed 6, aborted 0
Tests: succeeded 33, failed 9, canceled 0, ignored 0, pending 0
*** 9 TESTS FAILED ***
```
**After**
```
Run completed in 45 seconds, 86 milliseconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 44, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950555378
I need to ask about that this works with Java8 build and Java17 run too, @LuciferYang . Could you be clear for both cases: `Java8 build + Java17 run` and `Java17 build + Java17 run`.
> Manual test use Java 17
> mvn clean install -pl repl -am -DskipTests
> mvn clean install -pl repl
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950548048
Does this code work for all JVMs, @LuciferYang ? When I tried this on Java 17, it seems to fail.
```
$ java -version
openjdk version "17" 2021-09-14 LTS
OpenJDK Runtime Environment Zulu17.28+13-CA (build 17+35-LTS)
OpenJDK 64-Bit Server VM Zulu17.28+13-CA (build 17+35-LTS, mixed mode, sharing)
$ mvn clean install -pl repl
...
[INFO] --- scalatest-maven-plugin:2.0.2:test (test) @ spark-repl_2.12 ---
Discovery starting.
Discovery completed in 157 milliseconds.
Run starting. Expected test count is: 44
ReplSuite:
Spark context available as 'sc' (master = local, app id = local-1635140304149).
Spark session available as 'spark'.
- SPARK-15236: use Hive catalog
Spark context available as 'sc' (master = local, app id = local-1635140306451).
Spark session available as 'spark'.
- SPARK-15236: use in-memory catalog
Spark context available as 'sc' (master = local, app id = local-1635140307427).
Spark session available as 'spark'.
- broadcast vars *** FAILED ***
isContain was true Interpreter output contained 'Exception':
Welcome to
____ __
/ __/__ ___ _____/ /__
_\ \/ _ \/ _ `/ __/ '_/
/___/ .__/\_,_/_/ /_/\_\ version 3.3.0-SNAPSHOT
/_/
Using Scala version 2.12.15 (OpenJDK 64-Bit Server VM, Java 17)
Type in expressions to have them evaluated.
Type :help for more information.
scala>
scala> array: Array[Int] = Array(0, 0, 0, 0, 0)
scala> broadcastArray: org.apache.spark.broadcast.Broadcast[Array[Int]] = Broadcast(0)
scala> java.lang.IllegalAccessException: Can not set final $iw field $Lambda$2881/0x0000000801887858.arg$1 to $iw
at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:76)
at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:80)
at java.base/jdk.internal.reflect.UnsafeQualifiedObjectFieldAccessorImpl.set(UnsafeQualifiedObjectFieldAccessorImpl.java:79)
at java.base/java.lang.reflect.Field.set(Field.java:799)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:398)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:162)
at org.apache.spark.SparkContext.clean(SparkContext.scala:2490)
at org.apache.spark.rdd.RDD.$anonfun$map$1(RDD.scala:414)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:151)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:112)
at org.apache.spark.rdd.RDD.withScope(RDD.scala:406)
at org.apache.spark.rdd.RDD.map(RDD.scala:413)
... 95 elided
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r734698461
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -394,8 +395,17 @@ private[spark] object ClosureCleaner extends Logging {
parent = null, outerThis, capturingClass, accessedFields)
val outerField = func.getClass.getDeclaredField("arg$1")
+ // SPARK-37072: When Java 17 is used and `outerField` is read-only,
+ // the content of `outerField` cannot be set by reflect api directly.
+ // But We can remove the `final` modifier of `outerField` before set value
Review comment:
`We` -> `we`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950499077
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49043/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950578362
@dongjoon-hyun
> I need to ask about that this works with Java8 build and Java17 run too, @LuciferYang . Could you be clear for both cases: Java8 build + Java17 run and Java17 build + Java17 run.
Manual test use Java 17
mvn clean install -pl repl -am -DskipTests
mvn clean install -pl repl
- Manual test use Java17 build + Java17 run (both openjdk version "17" 2021-09-14 LTS)
Use Java 17 to run the following two commands
```
mvn clean install -pl repl -am -DskipTests
mvn test -pl repl
```
**Before**
```
Run completed in 30 seconds, 826 milliseconds.
Total number of tests run: 42
Suites: completed 6, aborted 0
Tests: succeeded 33, failed 9, canceled 0, ignored 0, pending 0
*** 9 TESTS FAILED ***
```
**After**
```
Run completed in 45 seconds, 86 milliseconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 44, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
```
- Manual test use Java8 build + Java17 run
Use Java 8(openjdk version "1.8.0_292") to run the build command:
```
mvn clean install -pl repl -am -DskipTests
```
And Java 17(openjdk version "17" 2021-09-14 LTS) to run the test command:
```
mvn test -pl repl
```
**Before**
```
Run completed in 41 seconds, 855 milliseconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 35, failed 9, canceled 0, ignored 0, pending 0
*** 9 TESTS FAILED ***
```
**After**
```
Run completed in 44 seconds, 596 milliseconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 44, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950560671
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49045/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950482167
**[Test build #144572 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144572/testReport)** for PR 34368 at commit [`3d1daff`](https://github.com/apache/spark/commit/3d1daff980070aa388b0c564c833c573b19e719e).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949685903
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49008/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r734699454
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -407,6 +417,24 @@ private[spark] object ClosureCleaner extends Logging {
}
}
+ /**
+ * This method is used to get the final modifier field when use Java 17.
+ */
+ private def getFinalModifiersFieldForJava17(field: Field): Option[Field] = {
+ if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_17) &&
+ Modifier.isFinal(field.getModifiers)) {
Review comment:
indentation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang edited a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950578362
@dongjoon-hyun
> I need to ask about that this works with Java8 build and Java17 run too, @LuciferYang . Could you be clear for both cases: Java8 build + Java17 run and Java17 build + Java17 run.
> Manual test use Java 17
> mvn clean install -pl repl -am -DskipTests
> mvn clean install -pl repl
I tested these two scenarios, and they both passed the test. I also added them to the PR description
- Manual test use Java17 build + Java17 run (both openjdk version "17" 2021-09-14 LTS)
Use Java 17 to run the following two commands
```
mvn clean install -pl repl -am -DskipTests
mvn test -pl repl
```
**Before**
```
Run completed in 30 seconds, 826 milliseconds.
Total number of tests run: 42
Suites: completed 6, aborted 0
Tests: succeeded 33, failed 9, canceled 0, ignored 0, pending 0
*** 9 TESTS FAILED ***
```
**After**
```
Run completed in 45 seconds, 86 milliseconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 44, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
```
- Manual test use Java8 build + Java17 run
Use Java 8(openjdk version "1.8.0_292") to run the build command:
```
mvn clean install -pl repl -am -DskipTests
```
And Java 17(openjdk version "17" 2021-09-14 LTS) to run the test command:
```
mvn test -pl repl
```
**Before**
```
Run completed in 41 seconds, 855 milliseconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 35, failed 9, canceled 0, ignored 0, pending 0
*** 9 TESTS FAILED ***
```
**After**
```
Run completed in 44 seconds, 596 milliseconds.
Total number of tests run: 44
Suites: completed 7, aborted 0
Tests: succeeded 44, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950554993
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950551898
**[Test build #144572 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144572/testReport)** for PR 34368 at commit [`3d1daff`](https://github.com/apache/spark/commit/3d1daff980070aa388b0c564c833c573b19e719e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949751379
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144537/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949743529
**[Test build #144537 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144537/testReport)** for PR 34368 at commit [`0f54677`](https://github.com/apache/spark/commit/0f54677bc44012769084d56d76e6729ad7923457).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950553099
**[Test build #144574 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144574/testReport)** for PR 34368 at commit [`dcc06a4`](https://github.com/apache/spark/commit/dcc06a4a70b7751546ec174f33e16909ee89bea2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950553649
No problem. Thank you for your swift answer. Could you revise the PR description according to the procedure?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r734699000
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -407,6 +417,24 @@ private[spark] object ClosureCleaner extends Logging {
}
}
+ /**
+ * This method is used to get the final modifier field when use Java 17.
Review comment:
Maybe, `when use Java 17` -> `on Java 17`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950555453
> I wonder what happens if we don't clear this field in the closure in this case - seems kind of risky to do this. That said, who knows what behavior differences arise if we don't
@srowen I try to remove the following code and test the `repl` module
https://github.com/apache/spark/blob/adf9b64c0be8e6e5bc6042eaaecd53518fbc5e25/core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala#L396-L398
there are 2 test failed due to `Caused by: java.io.NotSerializableException: NotSerializableClass`
```
- SPARK-31399: should clone+clean line object w/ non-serializable state in ClosureCleaner *** FAILED ***
isContain was false Interpreter output did not contain 'r: Array[scala.collection.immutable.IndexedSeq[String]] = Array(Vector(), Vector(1someValue), Vector(1someValue, 1someValue, 2someValue))':
scala> defined class NotSerializableClass
scala> | | | | | ns: NotSerializableClass = NotSerializableClass@c63e6a1
topLevelValue: String = someValue
closure: Int => scala.collection.immutable.IndexedSeq[String] = $Lambda$4794/0x0000000801fcedd0@7a715bb
scala> org.apache.spark.SparkException: Task not serializable
at org.apache.spark.util.ClosureCleaner$.ensureSerializable(ClosureCleaner.scala:444)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:416)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:163)
at org.apache.spark.SparkContext.clean(SparkContext.scala:2490)
at org.apache.spark.rdd.RDD.$anonfun$map$1(RDD.scala:414)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:151)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:112)
at org.apache.spark.rdd.RDD.withScope(RDD.scala:406)
at org.apache.spark.rdd.RDD.map(RDD.scala:413)
... 35 elided
Caused by: java.io.NotSerializableException: NotSerializableClass
Serialization stack:
- object not serializable (class: NotSerializableClass, value: NotSerializableClass@c63e6a1)
- field (class: $iw, name: ns, type: class NotSerializableClass)
- object (class $iw, $iw@79ac2398)
- element of array (index: 0)
- array (class [Ljava.lang.Object;, size 1)
- field (class: java.lang.invoke.SerializedLambda, name: capturedArgs, type: class [Ljava.lang.Object;)
- object (class java.lang.invoke.SerializedLambda, SerializedLambda[capturingClass=class $iw, functionalInterfaceMethod=scala/Function1.apply:(Ljava/lang/Object;)Ljava/lang/Object;, implementation=invokeStatic $anonfun$closure$1$adapted:(L$iw;Ljava/lang/Object;)Lscala/collection/immutable/IndexedSeq;, instantiatedMethodType=(Ljava/lang/Object;)Lscala/collection/immutable/IndexedSeq;, numCaptured=1])
- writeReplace data (class: java.lang.invoke.SerializedLambda)
- object (class $Lambda$4794/0x0000000801fcedd0, $Lambda$4794/0x0000000801fcedd0@7a715bb)
at org.apache.spark.serializer.SerializationDebugger$.improveException(SerializationDebugger.scala:41)
at org.apache.spark.serializer.JavaSerializationStream.writeObject(JavaSerializer.scala:47)
at org.apache.spark.serializer.JavaSerializerInstance.serialize(JavaSerializer.scala:101)
at org.apache.spark.util.ClosureCleaner$.ensureSerializable(ClosureCleaner.scala:441)
... 43 more
scala> | _result_1635140723849: Int = 1
scala> (SingletonRepl2Suite.scala:96)
- SPARK-31399: ClosureCleaner should discover indirectly nested closure in inner class *** FAILED ***
isContain was false Interpreter output did not contain 'r: Array[scala.collection.immutable.IndexedSeq[String]] = Array(Vector(), Vector(1someValue), Vector(1someValue, 1someValue, 2someValue))':
scala> defined class NotSerializableClass
scala> | | | | | | | ns: NotSerializableClass = NotSerializableClass@3c10dced
topLevelValue: String = someValue
closure: Int => scala.collection.immutable.IndexedSeq[String] = $Lambda$4817/0x0000000801fe1330@1c0d770d
scala> org.apache.spark.SparkException: Task not serializable
at org.apache.spark.util.ClosureCleaner$.ensureSerializable(ClosureCleaner.scala:444)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:416)
at org.apache.spark.util.ClosureCleaner$.clean(ClosureCleaner.scala:163)
at org.apache.spark.SparkContext.clean(SparkContext.scala:2490)
at org.apache.spark.rdd.RDD.$anonfun$map$1(RDD.scala:414)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:151)
at org.apache.spark.rdd.RDDOperationScope$.withScope(RDDOperationScope.scala:112)
at org.apache.spark.rdd.RDD.withScope(RDD.scala:406)
at org.apache.spark.rdd.RDD.map(RDD.scala:413)
... 35 elided
Caused by: java.io.NotSerializableException: NotSerializableClass
Serialization stack:
- object not serializable (class: NotSerializableClass, value: NotSerializableClass@3c10dced)
- field (class: $iw, name: ns, type: class NotSerializableClass)
- object (class $iw, $iw@5a9299a9)
- element of array (index: 0)
- array (class [Ljava.lang.Object;, size 1)
- field (class: java.lang.invoke.SerializedLambda, name: capturedArgs, type: class [Ljava.lang.Object;)
- object (class java.lang.invoke.SerializedLambda, SerializedLambda[capturingClass=class $iw, functionalInterfaceMethod=scala/Function1.apply:(Ljava/lang/Object;)Ljava/lang/Object;, implementation=invokeStatic $anonfun$closure$1$adapted:(L$iw;Ljava/lang/Object;)Lscala/collection/immutable/IndexedSeq;, instantiatedMethodType=(Ljava/lang/Object;)Lscala/collection/immutable/IndexedSeq;, numCaptured=1])
- writeReplace data (class: java.lang.invoke.SerializedLambda)
- object (class $Lambda$4817/0x0000000801fe1330, $Lambda$4817/0x0000000801fe1330@1c0d770d)
at org.apache.spark.serializer.SerializationDebugger$.improveException(SerializationDebugger.scala:41)
at org.apache.spark.serializer.JavaSerializationStream.writeObject(JavaSerializer.scala:47)
at org.apache.spark.serializer.JavaSerializerInstance.serialize(JavaSerializer.scala:101)
at org.apache.spark.util.ClosureCleaner$.ensureSerializable(ClosureCleaner.scala:441)
... 43 more
scala> | _result_1635140724243: Int = 1
scala> (SingletonRepl2Suite.scala:96)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950556540
@dongjoon-hyun OK ~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang edited a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950552904
@dongjoon-hyun sorry, did you execute `mvn clean install -pl repl -am -DskipTests` ? If not, you can execute it first. I changed description
I used zulu17
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950515608
Thank you for updating, @LuciferYang .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r734914860
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -407,6 +417,24 @@ private[spark] object ClosureCleaner extends Logging {
}
}
+ /**
+ * This method is used to get the final modifier field when use Java 17.
+ */
+ private def getFinalModifiersFieldForJava17(field: Field): Option[Field] = {
+ if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_17) &&
+ Modifier.isFinal(field.getModifiers)) {
+ val methodGetDeclaredFields0 = classOf[Class[_]]
+ .getDeclaredMethod("getDeclaredFields0", classOf[Boolean])
+ methodGetDeclaredFields0.setAccessible(true)
+ val fields = methodGetDeclaredFields0.invoke(classOf[Field], false.asInstanceOf[Object])
+ .asInstanceOf[Array[Field]]
+ val modifiersFieldOption = fields.find(field => "modifiers".equals(field.getName))
+ assert(modifiersFieldOption.isDefined)
Review comment:
require, not assert
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -394,8 +395,17 @@ private[spark] object ClosureCleaner extends Logging {
parent = null, outerThis, capturingClass, accessedFields)
val outerField = func.getClass.getDeclaredField("arg$1")
+ // SPARK-37072: When Java 17 is used and `outerField` is read-only,
+ // the content of `outerField` cannot be set by reflect api directly.
+ // But We can remove the `final` modifier of `outerField` before set value
Review comment:
I wonder what happens if we don't clear this field in the closure in this case - seems kind of risky to do this. That said, who knows what behavior differences arise if we don't
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950530139
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49045/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950490362
**[Test build #144574 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144574/testReport)** for PR 34368 at commit [`dcc06a4`](https://github.com/apache/spark/commit/dcc06a4a70b7751546ec174f33e16909ee89bea2).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r735268807
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -394,8 +395,17 @@ private[spark] object ClosureCleaner extends Logging {
parent = null, outerThis, capturingClass, accessedFields)
val outerField = func.getClass.getDeclaredField("arg$1")
+ // SPARK-37072: When Java 17 is used and `outerField` is read-only,
+ // the content of `outerField` cannot be set by reflect api directly.
+ // But we can remove the `final` modifier of `outerField` before set value
+ // and reset the modifier after set value.
+ val modifiersField = getFinalModifiersFieldForJava17(outerField)
+ modifiersField
+ .foreach(m => m.setInt(outerField, outerField.getModifiers & ~Modifier.FINAL))
outerField.setAccessible(true)
outerField.set(func, clonedOuterThis)
+ modifiersField
+ .foreach(m => m.setInt(outerField, outerField.getModifiers | Modifier.FINAL))
Review comment:
If we touch `core/src/main`, we should remove `[TEST]` from the PR title, @LuciferYang ~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949701284
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49008/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r734931211
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -407,6 +417,24 @@ private[spark] object ClosureCleaner extends Logging {
}
}
+ /**
+ * This method is used to get the final modifier field when use Java 17.
+ */
+ private def getFinalModifiersFieldForJava17(field: Field): Option[Field] = {
+ if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_17) &&
+ Modifier.isFinal(field.getModifiers)) {
Review comment:
ok， will fix it later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949649086
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49008/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949612259
I need to re-test previous modules because this pr change the core module
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950560698
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49045/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #34368:
URL: https://github.com/apache/spark/pull/34368
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950490362
**[Test build #144574 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144574/testReport)** for PR 34368 at commit [`dcc06a4`](https://github.com/apache/spark/commit/dcc06a4a70b7751546ec174f33e16909ee89bea2).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949612735
**[Test build #144537 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144537/testReport)** for PR 34368 at commit [`0f54677`](https://github.com/apache/spark/commit/0f54677bc44012769084d56d76e6729ad7923457).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949751379
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144537/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950482167
**[Test build #144572 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144572/testReport)** for PR 34368 at commit [`3d1daff`](https://github.com/apache/spark/commit/3d1daff980070aa388b0c564c833c573b19e719e).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang edited a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950552904
@dongjoon-hyun sorry, did you execute `mvn clean install -pl repl -am -DskipTests` ? If not, you can execute it first. I changed description
I used zulu17:
```
openjdk version "17" 2021-09-14 LTS
OpenJDK Runtime Environment Zulu17.28+13-CA (build 17+35-LTS)
OpenJDK 64-Bit Server VM Zulu17.28+13-CA (build 17+35-LTS, mixed mode, sharing)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r735276108
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -394,8 +395,17 @@ private[spark] object ClosureCleaner extends Logging {
parent = null, outerThis, capturingClass, accessedFields)
val outerField = func.getClass.getDeclaredField("arg$1")
+ // SPARK-37072: When Java 17 is used and `outerField` is read-only,
+ // the content of `outerField` cannot be set by reflect api directly.
+ // But we can remove the `final` modifier of `outerField` before set value
+ // and reset the modifier after set value.
+ val modifiersField = getFinalModifiersFieldForJava17(outerField)
+ modifiersField
+ .foreach(m => m.setInt(outerField, outerField.getModifiers & ~Modifier.FINAL))
outerField.setAccessible(true)
outerField.set(func, clonedOuterThis)
+ modifiersField
+ .foreach(m => m.setInt(outerField, outerField.getModifiers | Modifier.FINAL))
Review comment:
Thank you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-951120966
Merged to master for Apache Spark 3.3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950518495
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49043/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950527920
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49043/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34368: [SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950527920
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49043/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950552904
sorry, did you execute `mvn clean install -pl repl -am -DskipTests` ? If not, you can execute it first. I changed description
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang edited a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950552904
@dongjoon-hyun sorry, did you execute `mvn clean install -pl repl -am -DskipTests` ? If not, you can execute it first. I changed description
I test with zulu17
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950554323
Yes, I have added this step to the PR description
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950554994
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34368: [SPARK-37072][CORE] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-950560698
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49045/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34368:
URL: https://github.com/apache/spark/pull/34368#discussion_r734931107
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -407,6 +417,24 @@ private[spark] object ClosureCleaner extends Logging {
}
}
+ /**
+ * This method is used to get the final modifier field when use Java 17.
+ */
+ private def getFinalModifiersFieldForJava17(field: Field): Option[Field] = {
+ if (SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_17) &&
+ Modifier.isFinal(field.getModifiers)) {
+ val methodGetDeclaredFields0 = classOf[Class[_]]
+ .getDeclaredMethod("getDeclaredFields0", classOf[Boolean])
+ methodGetDeclaredFields0.setAccessible(true)
+ val fields = methodGetDeclaredFields0.invoke(classOf[Field], false.asInstanceOf[Object])
+ .asInstanceOf[Array[Field]]
+ val modifiersFieldOption = fields.find(field => "modifiers".equals(field.getName))
+ assert(modifiersFieldOption.isDefined)
Review comment:
ok， will fix it later
##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -394,8 +395,17 @@ private[spark] object ClosureCleaner extends Logging {
parent = null, outerThis, capturingClass, accessedFields)
val outerField = func.getClass.getDeclaredField("arg$1")
+ // SPARK-37072: When Java 17 is used and `outerField` is read-only,
+ // the content of `outerField` cannot be set by reflect api directly.
+ // But We can remove the `final` modifier of `outerField` before set value
Review comment:
I don't know what the result will be, but I can do an experiment later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949701284
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49008/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34368: [WIP][SPARK-37072][CORE][TEST] Pass all UTs in `repl` with Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34368:
URL: https://github.com/apache/spark/pull/34368#issuecomment-949612735
**[Test build #144537 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144537/testReport)** for PR 34368 at commit [`0f54677`](https://github.com/apache/spark/commit/0f54677bc44012769084d56d76e6729ad7923457).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org