You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "yadavay-amzn (via GitHub)" <gi...@apache.org> on 2026/05/06 18:31:49 UTC
[PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
yadavay-amzn opened a new pull request, #55720:
URL: https://github.com/apache/spark/pull/55720
### What changes were proposed in this pull request?
Fixes SPARK-56448 (https://issues.apache.org/jira/browse/SPARK-56448).
The Spark Connect REPL uses Ammonite. Ammonite's default `Storage.Folder`
persists compiled predef classes under `~/.ammonite/<version>/cache`. On a
subsequent REPL start from the same working directory, the cached
`CodePredef` class is reloaded but its reference to the per-session
`ArgsPredef` helper is stale, producing a `NullPointerException` during
predef initialization.
This PR switches the Connect REPL's compile cache to `Storage.InMemory`
so every session starts fresh and no stale cache is carried across
restarts. The `ammonite.Main` construction is extracted into a
package-private helper (`ConnectRepl.newAmmoniteMain`) to keep the
regression test localised to unit-level.
### Why are the changes needed?
The stale-cache failure is a user-visible crash on every second
invocation of `bin/spark-shell --remote sc://...` from the same working
directory. Repro steps are on the JIRA.
### Does this PR introduce _any_ user-facing change?
There is one minor observable tradeoff: because the compile cache is
now in-memory rather than persisted, each REPL start recompiles the
predef instead of reading the cached classfiles. This adds ~a few
hundred milliseconds to subsequent REPL startups but eliminates the
NPE. We believe this is the correct tradeoff — a small startup cost
is preferable to a hard failure.
### How was this patch tested?
Added `ConnectReplSuite` with a regression test that asserts the
`ammonite.Main` returned by `ConnectRepl.newAmmoniteMain` exposes no
on-disk cache directory (`storageBackend.dirOpt.isEmpty`) and is an
instance of `Storage.InMemory`.
The `dirOpt` check is an observable behavioural assertion: if the
`Storage.InMemory()` wiring is reverted, `ammonite.Main` falls back to
the default `Storage.Folder(Defaults.ammoniteHome)` and the assertion
fails with a clear message rather than silently compiling. I verified
this locally by temporarily reverting only the `Storage.InMemory()`
line and re-running the test; it fails with
`Some(/home/<user>/.ammonite) was not empty`. Restoring the fix makes
the test pass.
Targeted test run:
```
./build/sbt "connect-client-jvm/Test/testOnly org.apache.spark.sql.application.ConnectReplSuite"
...
[info] - SPARK-56448: Connect REPL uses an in-memory Ammonite storage backend (107 milliseconds)
[info] Tests: succeeded 1, failed 0
```
Also ran `./dev/scalastyle` locally — passes.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.7
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493875030
@attilapiros Thanks for digging into this and finding the root cause + fix! I updated PR #55999 to add `--sun-misc-unsafe-memory-access=allow` in `JavaModuleOptions.java` but see that you've created a PR with the fix so closing mine.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3267356403
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/AmmoniteReplE2ESuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class AmmoniteReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
Review Comment:
This test case seems to introduce a flakiness. I didn't dig much, but have you observed the following failures in your CIs, @yadavay-amzn and @attilapiros ?
```
[info] AmmoniteReplE2ESuite:
[info] - SPARK-56448: restarting spark-shell --remote does not throw NPE *** FAILED *** (1 second, 975 milliseconds)
[info] 1 did not equal 0 First spark-shell failed (exit=1): WARNING: Using incubator modules: jdk.incubator.vector
[info] Exception in thread "main" java.lang.ExceptionInInitializerError
[info] at org.apache.arrow.memory.netty.DefaultAllocationManagerFactory.<clinit>(DefaultAllocationManagerFactory.java:26)
...
[info] Caused by: java.lang.UnsupportedOperationException
[info] at org.sparkproject.connect.client.io.netty.buffer.EmptyByteBuf.memoryAddress(EmptyByteBuf.java:961)
[info] at org.sparkproject.connect.client.io.netty.buffer.UnsafeDirectLittleEndian.<init>(UnsafeDirectLittleEndian.java:45)
[info] at org.sparkproject.connect.client.io.netty.buffer.PooledByteBufAllocatorL.<init>(PooledByteBufAllocatorL.java:47)
[info] ... 32 more (AmmoniteReplE2ESuite.scala:66)
```
cc @peter-toth
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4492440674
I would not switch off the test! It is very good that we found this error.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4410397452
Thanks for the review @attilapiros. Updated the test to exercise the restart scenario directly: it calls `ConnectRepl.newAmmoniteMain(...).run(...)` twice with the same predef and bind setup, reproducing the "second REPL start from the same working directory" path that previously triggered the NPE. With the fix, both starts complete cleanly; without it the second `run` throws `NullPointerException` during predef initialization.
Does this approach work?
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3267356403
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/AmmoniteReplE2ESuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class AmmoniteReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
Review Comment:
This test case seems to introduce a flakiness. I didn't dig much, but have you observed the following failures in your CIs, @yadavay-amzn and @attilapiros ?
```
- SPARK-56448: restarting spark-shell --remote does not throw NPE *** FAILED ***
1 did not equal 0 Second spark-shell failed (exit=1): WARNING: Using incubator modules: jdk.incubator.vector
Exception in thread "main" java.lang.NullPointerException: Cannot invoke "ammonite.predef.ArgsPredef$Helper.spark()" because the return value of "ammonite.predef.CodePredef.ArgsPredef()" is null
```
cc @peter-toth
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3223901802
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/ConnectReplE2ESuite.scala:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class ConnectReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
+ // First invocation
+ val (exit1, _, stderr1) = runSparkShell()
+ assert(exit1 == 0, s"First spark-shell failed (exit=$exit1): $stderr1")
+
+ // Second invocation -- without the fix, this would NPE from stale Ammonite cache
+ val (exit2, _, stderr2) = runSparkShell()
+ assert(exit2 == 0, s"Second spark-shell failed (exit=$exit2): $stderr2")
+ assert(!stderr2.contains("NullPointerException"), s"Second spark-shell threw NPE: $stderr2")
Review Comment:
This line cannot be reached and thus not needed.
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/ConnectReplSuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.io.{ByteArrayInputStream, ByteArrayOutputStream}
+
+import ammonite.runtime.Storage
+import ammonite.util.Bind
+
+import org.apache.spark.sql.connect.test.ConnectFunSuite
+
+class ConnectReplSuite extends ConnectFunSuite {
Review Comment:
I think this test is not needed anymore.
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/ConnectReplE2ESuite.scala:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class ConnectReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
Review Comment:
We already have a `ReplE2ESuite` but I agree better to have this in a separate suite.
But as we have a connect REPL which `./bin/spark-connect-shell` what about renaming this test to `AmmoniteReplE2ESuite`?
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3268315219
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/AmmoniteReplE2ESuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class AmmoniteReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
Review Comment:
@dongjoon-hyun Thanks for taking a look. We haven't seen this specific Arrow/Netty failure in our CI runs.
The CI failures we saw were always the `OracleIntegrationSuite` Docker test.
The test launches spark-shell --remote as a subprocess, so it's sensitive to the JDK environment the CI runner uses. Would it help to add a retry or mark this test as flaky?
I'll also investigate the Arrow/Netty root cause by running the test locally to see if the CI flaky behavior is reproducible locally.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4425665749
@attilapiros Good suggestion — added `ConnectReplE2ESuite` that follows the `SparkShellSuite` pattern: starts a Connect server via `RemoteSparkSession`, then runs `bin/spark-shell --remote sc://localhost:$port` twice as a subprocess (closing stdin immediately so it exits on EOF). Asserts both invocations exit cleanly and the second one does not contain `NullPointerException` in stderr.
Kept the existing `ConnectReplSuite` unit test as well (fast, no server needed).
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4433588526
Done -- reverted the method extraction. The fix is now minimal: just the `Storage.InMemory` argument + import + inline comment explaining SPARK-56448.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3268549909
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/AmmoniteReplE2ESuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class AmmoniteReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
Review Comment:
Ran the `AmmoniteReplE2ESuite` test 10 times locally on current master (JDK 17) and its passing consistently. So the flakiness seems specific to the CI run.
The error (`ExceptionInInitializerError` in Arrow's `DefaultAllocationManagerFactory` / Netty's `PooledByteBufAllocatorL`) looks like a JDK/Arrow memory allocator incompatibility which may be JDK version sensitive. The test itself just launches spark-shell --remote as a subprocess and it doesn't do anything Arrow/Netty-specific.
@dongjoon-hyun / @peter-toth Could you share which JDK vendor/version and CI environment produced the failure? Or a link to the CI job if you have it? I can deep dive to figure out the issue and try repro with same setup.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3269196157
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/AmmoniteReplE2ESuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class AmmoniteReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
Review Comment:
Thank you for confirming, @yadavay-amzn . For my cases, the failures are consistently observed with Java 25 + `UBI 10` combination.
```
[root@1d494a9a9969 /]# java --version
openjdk 25.0.3 2026-04-21 LTS
...
[root@1d494a9a9969 /]# cat /etc/os-release
NAME="Red Hat Enterprise Linux"
VERSION="10.1 (Coughlan)"
```
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4467968373
Merged into master and branch-4.x, branch-4.2, branch-4.1, branch-4.0, branch-3.5. Thanks @yadavay-amzn!
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493201072
This way as this is an error in the test switching off the test is good workaround for now.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493253570
Thanks @dongjoon-hyun for flagging me on this PR.
Do we need the fallback switch in branch-4.2 before i cut RC1 tomorrow?
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4418088486
@attilapiros I verified this both in-process (calling `run()` twice in the same JVM) and cross-process (forking separate JVMs with `Storage.Folder` sharing a temp cache directory). The NPE does not reproduce in either case — it requires a specific Ammonite classloader state that only occurs in real interactive usage (your original reproduction in the JIRA).
Updated the test to directly assert that `newAmmoniteMain` wires `Storage.InMemory` (which is the exact semantic the fix adds), and smoke-tests the restart scenario by calling `run()` twice. Also fixed the non-ASCII character.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493059937
@attilapiros Oh that's interesting, I reproduced the failure on Corretto 25.0.3+9-LTS (Amazon Linux). May I ask which JDK 25 vendor/build are you using?
Also wondering, the test runs `spark-shell --remote` as a subprocess via ProcessBuilder. Could there be an environment difference between running it interactively vs as a subprocess (e.g., different classpath assembly, missing JVM flags etc)?
>I would not switch off the test! It is very good that we found this error.
I was disabling the test since it sounded like it was blocking the release, can close PR #55999 and focus on identifying the real root cause instead.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493448506
@dongjoon-hyun, @huaxingao, @yadavay-amzn
Sorry for the confusion!
I was wrong before: this is NOT a simple test issue but a bug in the `spark-shell` as well on JDK25:
```
./bin/spark-shell --remote sc://localhost:15002
WARNING: Using incubator modules: jdk.incubator.vector
WARNING: package sun.security.action not in java.base
26/05/19 17:38:31 INFO BaseAllocator: Debug mode disabled. Enable with the VM option -Darrow.memory.debug.allocator=true.
26/05/19 17:38:31 INFO DefaultAllocationManagerOption: allocation manager type not specified, using netty as the default type
26/05/19 17:38:31 INFO CheckAllocator: Using DefaultAllocationManager at memory/netty/DefaultAllocationManagerFactory.class
Exception in thread "main" java.lang.ExceptionInInitializerError
at org.sparkproject.org.apache.arrow.memory.netty.DefaultAllocationManagerFactory.<clinit>(DefaultAllocationManagerFactory.java:26)
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:467)
at java.base/java.lang.Class.forName(Class.java:458)
at org.sparkproject.org.apache.arrow.memory.DefaultAllocationManagerOption.getFactory(DefaultAllocationManagerOption.java:105)
at org.sparkproject.org.apache.arrow.memory.DefaultAllocationManagerOption.getDefaultAllocationManagerFactory(DefaultAllocationManagerOption.java:92)
at org.sparkproject.org.apache.arrow.memory.BaseAllocator$Config.getAllocationManagerFactory(BaseAllocator.java:826)
at org.sparkproject.org.apache.arrow.memory.ImmutableConfig.access$001(ImmutableConfig.java:20)
at org.sparkproject.org.apache.arrow.memory.ImmutableConfig$InitShim.getAllocationManagerFactory(ImmutableConfig.java:80)
at org.sparkproject.org.apache.arrow.memory.ImmutableConfig.<init>(ImmutableConfig.java:43)
at org.sparkproject.org.apache.arrow.memory.ImmutableConfig$Builder.build(ImmutableConfig.java:492)
at org.sparkproject.org.apache.arrow.memory.BaseAllocator.<clinit>(BaseAllocator.java:72)
at org.apache.spark.sql.connect.SparkSession.<init>(SparkSession.scala:89)
at org.apache.spark.sql.connect.SparkSession$Builder.tryCreateSessionFromClient(SparkSession.scala:1059)
at org.apache.spark.sql.connect.SparkSession$Builder.$anonfun$getOrCreate$1(SparkSession.scala:1119)
at org.apache.spark.sql.connect.SparkSession$.withLocalConnectServer(SparkSession.scala:949)
at org.apache.spark.sql.connect.SparkSession$Builder.getOrCreate(SparkSession.scala:1118)
at org.apache.spark.sql.application.ConnectRepl$.$anonfun$doMain$1(ConnectRepl.scala:91)
at org.apache.spark.sql.connect.SparkSession$.withLocalConnectServer(SparkSession.scala:949)
at org.apache.spark.sql.application.ConnectRepl$.doMain(ConnectRepl.scala:68)
at org.apache.spark.sql.application.ConnectRepl$.main(ConnectRepl.scala:58)
at org.apache.spark.sql.application.ConnectRepl.main(ConnectRepl.scala)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at org.apache.spark.deploy.JavaMainApplication.start(SparkApplication.scala:52)
at org.apache.spark.deploy.SparkSubmit.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:1033)
at org.apache.spark.deploy.SparkSubmit.doRunMain$1(SparkSubmit.scala:203)
at org.apache.spark.deploy.SparkSubmit.submit(SparkSubmit.scala:226)
at org.apache.spark.deploy.SparkSubmit.doSubmit(SparkSubmit.scala:95)
at org.apache.spark.deploy.SparkSubmit$$anon$2.doSubmit(SparkSubmit.scala:1171)
at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:1180)
at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala)
Caused by: java.lang.UnsupportedOperationException
at org.sparkproject.io.netty.buffer.EmptyByteBuf.memoryAddress(EmptyByteBuf.java:961)
at org.sparkproject.io.netty.buffer.DuplicatedByteBuf.memoryAddress(DuplicatedByteBuf.java:115)
at org.sparkproject.io.netty.buffer.UnsafeDirectLittleEndian.<init>(UnsafeDirectLittleEndian.java:45)
at org.sparkproject.io.netty.buffer.PooledByteBufAllocatorL.<init>(PooledByteBufAllocatorL.java:47)
at org.sparkproject.org.apache.arrow.memory.netty.NettyAllocationManager.<clinit>(NettyAllocationManager.java:54)
... 32 more
```
Still I think it is unrelated to the ammonite fix!
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493531129
We are not alone: https://github.com/apache/iceberg/issues/15930
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4412623986
@yadavay-amzn please doublecheck whether it fails when `Storage.Folder` is used.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3228783683
##########
sql/connect/client/jvm/src/main/scala/org/apache/spark/sql/application/ConnectRepl.scala:
##########
@@ -100,11 +101,43 @@ Spark session available as 'spark'.
|
|spark.registerClassFinder(new AmmoniteClassFinder(repl.sess))
|""".stripMargin
+ val main = newAmmoniteMain(
+ welcomeBannerText = splash.format(spark_version, spark.version),
+ predefCode = predefCode,
+ inputStream = inputStream,
+ outputStream = outputStream,
+ errorStream = errorStream)
+
+ if (semaphore.nonEmpty) {
+ // Used for testing.
+ main.run(sparkBind, new Bind[Semaphore]("semaphore", semaphore.get))
+ } else {
+ main.run(sparkBind)
+ }
+ }
+
+ /**
+ * Construct the [[ammonite.Main]] used by the Spark Connect REPL.
+ *
+ * The default [[ammonite.runtime.Storage.Folder]] backend persists Ammonite's compile cache
+ * under `~/.ammonite`. When the Connect REPL is started a second time from the same working
+ * directory, the cached `CodePredef` class is reloaded but its reference to the per-session
+ * `ArgsPredef` helper is stale, which causes a `NullPointerException` during predef
+ * initialization (SPARK-56448). Using [[ammonite.runtime.Storage.InMemory]] keeps the compile
+ * cache scoped to a single REPL session and avoids the stale-cache failure on restart.
+ */
+ private[application] def newAmmoniteMain(
Review Comment:
Usually we prefer minimal changes and here this new function has no real value as it is only called from one place . So can you please revert this back?
The function's scaladoc can be comment inside...
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4445615117
I’ll leave this open for another 2–3 days to ensure everyone has a chance to provide feedback before I merge.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3267356403
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/AmmoniteReplE2ESuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class AmmoniteReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
Review Comment:
This test case seems to introduce a flakiness. I didn't dig much, but have you observed the following failures in your CIs, @yadavay-amzn and @attilapiros ?
```
[info] AmmoniteReplE2ESuite:
[info] - SPARK-56448: restarting spark-shell --remote does not throw NPE *** FAILED *** (1 second, 975 milliseconds)
[info] 1 did not equal 0 First spark-shell failed (exit=1): WARNING: Using incubator modules: jdk.incubator.vector
[info] Exception in thread "main" java.lang.ExceptionInInitializerError
[info] at org.apache.arrow.memory.netty.DefaultAllocationManagerFactory.<clinit>(DefaultAllocationManagerFactory.java:26)
...
[info] Caused by: java.lang.UnsupportedOperationException
[info] at org.sparkproject.connect.client.io.netty.buffer.EmptyByteBuf.memoryAddress(EmptyByteBuf.java:961)
[info] at org.sparkproject.connect.client.io.netty.buffer.UnsafeDirectLittleEndian.<init>(UnsafeDirectLittleEndian.java:45)
[info] at org.sparkproject.connect.client.io.netty.buffer.PooledByteBufAllocatorL.<init>(PooledByteBufAllocatorL.java:47)
[info] ... 32 more (AmmoniteReplE2ESuite.scala:66)
```
cc @peter-toth
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4492400892
@attilapiros Thanks for confirming the analysis! I also ran this locally on JDK
25 (Corretto 25.0.3) and can confirm:
- `AmmoniteReplE2ESuite` fails consistently on JDK 25 with the same
`EmptyByteBuf.memoryAddress()` error
- Passes 10/10 on JDK 17 and passes on JDK 21
So yes, `spark-shell --remote` itself cannot start on JDK 25 due to the Arrow/Netty
issue, independent of our ammonite fix.
I've updated SPARK-56955 with the full reproduction details and stack trace. PR
#55999 has the test skip workaround.
cc @dongjoon-hyun
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4424224636
@yadavay-amzn You can do something similar to what `SparkShellSuite` does: run two spark `spark-shell`s after each other.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4492186115
@dongjoon-hyun I agree @yadavay-amzn this must be independent from the ammonite fix and it means on JDK 25 `bin/spark-shell --remote` is not working as this could be easily the first test where we call `spark-shell` with the `remote` flag.
But let me doublecheck this by trying this out locally on JDK 25
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4435513896
@attilapiros Thanks for all the feedback and review!
PR description looks good
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3213200472
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/ConnectReplSuite.scala:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.io.{ByteArrayInputStream, ByteArrayOutputStream}
+
+import ammonite.util.Bind
+
+import org.apache.spark.sql.connect.test.ConnectFunSuite
+
+class ConnectReplSuite extends ConnectFunSuite {
+
+ test("SPARK-56448: restarting the Connect REPL does not throw NPE") {
+ // Reproduces the scenario from SPARK-56448: prior to the fix, starting the Connect
+ // REPL a second time from the same working directory caused Ammonite to reload a
+ // cached `CodePredef` whose reference to the per-session `ArgsPredef` helper was
+ // stale, producing a `NullPointerException` during predef initialization. The fix
+ // wires `Storage.InMemory` in `ConnectRepl.newAmmoniteMain` so nothing is persisted
+ // across sessions. This test exercises `newAmmoniteMain` twice with the same predef
+ // and bind setup and asserts that neither start throws.
+ def startRepl(): Unit = {
+ val in = new ByteArrayInputStream(Array.emptyByteArray)
+ val out = new ByteArrayOutputStream()
+ val err = new ByteArrayOutputStream()
+ val main = ConnectRepl.newAmmoniteMain(
+ welcomeBannerText = "",
+ predefCode = "val x = 1",
+ inputStream = in,
+ outputStream = out,
+ errorStream = err)
+ // A `Bind` is required to exercise the `ArgsPredef` → `CodePredef` wiring that is
Review Comment:
Avoid non-ascii char here:
```suggestion
// A `Bind` is required to exercise the `ArgsPredef` -> `CodePredef` wiring that is
```
Otherwise Scalastyle fails with:
```
Scalastyle checks failed at following occurrences:
[error] /__w/spark/spark/sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/ConnectReplSuite.scala:44:26: nonascii.message
```
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #55720:
URL: https://github.com/apache/spark/pull/55720#discussion_r3267356403
##########
sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/application/AmmoniteReplE2ESuite.scala:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.sql.application
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.mutable.ArrayBuffer
+import scala.sys.process.BasicIO
+
+import org.apache.spark.sql.connect.test.{ConnectFunSuite, RemoteSparkSession}
+import org.apache.spark.tags.AmmoniteTest
+
+@AmmoniteTest
+class AmmoniteReplE2ESuite extends ConnectFunSuite with RemoteSparkSession {
+
+ private def runSparkShell(): (Int, String, String) = {
+ val sparkHome = sys.props.getOrElse(
+ "spark.test.home",
+ sys.env.getOrElse("SPARK_HOME", fail("spark.test.home or SPARK_HOME not set")))
+ val command = Seq(s"$sparkHome/bin/spark-shell", "--remote", s"sc://localhost:$serverPort")
+
+ val process = new ProcessBuilder(command: _*).start()
+ // Close stdin immediately so shell exits on EOF
+ process.getOutputStream.close()
+
+ val stdout = new ArrayBuffer[String]()
+ val stderr = new ArrayBuffer[String]()
+ val stdoutThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stdout += _)(process.getInputStream)
+ }
+ val stderrThread = new Thread() {
+ setDaemon(true)
+ override def run(): Unit = BasicIO.processFully(stderr += _)(process.getErrorStream)
+ }
+ stdoutThread.start()
+ stderrThread.start()
+
+ val exited = process.waitFor(60, TimeUnit.SECONDS)
+ if (!exited) {
+ process.destroyForcibly()
+ fail("spark-shell did not exit within 60 seconds")
+ }
+ stdoutThread.join(10000)
+ stderrThread.join(10000)
+ (process.exitValue(), stdout.mkString("\n"), stderr.mkString("\n"))
+ }
+
+ test("SPARK-56448: restarting spark-shell --remote does not throw NPE") {
Review Comment:
This test case seems to introduce a flakiness. I didn't dig much, but have you observed the following failures in your CIs, @yadavay-amzn and @attilapiros ?
```
[info] AmmoniteReplE2ESuite:
[info] - SPARK-56448: restarting spark-shell --remote does not throw NPE *** FAILED *** (1 second, 975 milliseconds)
[info] 1 did not equal 0 First spark-shell failed (exit=1): WARNING: Using incubator modules: jdk.incubator.vector
[info] Exception in thread "main" java.lang.ExceptionInInitializerError
[info] at org.apache.arrow.memory.netty.DefaultAllocationManagerFactory.<clinit>(DefaultAllocationManagerFactory.java:26)
```
cc @peter-toth
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493277232
Oh, this is not a blocker for anything. It would be great but we can investigate more on this and decide on RC2, @huaxingao ~
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros closed pull request #55720: [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache
URL: https://github.com/apache/spark/pull/55720
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4427705318
Done -- renamed to `AmmoniteReplE2ESuite`, removed the unreachable NPE assertion, and dropped `ConnectReplSuite` since the E2E test covers it.
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4435130908
@yadavay-amzn I have updated the PR description please doublecheck it
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4492532822
@yadavay-amzn for me `spark-shell --remote` works (with the Ammonite fix):
```
$ ./bin/spark-shell sc://localhost:15002
WARNING: Using incubator modules: jdk.incubator.vector
WARNING: package sun.security.action not in java.base
Using Spark's default log4j profile: org/apache/spark/log4j2-defaults.properties
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
Welcome to
____ __
/ __/__ ___ _____/ /__
_\ \/ _ \/ _ `/ __/ '_/
/___/ .__/\_,_/_/ /_/\_\ version 4.2.0-SNAPSHOT
/_/
Using Scala version 2.13.18 (OpenJDK 64-Bit Server VM, Java 25.0.3)
Type in expressions to have them evaluated.
Type :help for more information.
26/05/19 15:15:24 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::allocateMemory has been called by io.netty.util.internal.PlatformDependent0$2 (file:/Users/apiros/git/attilapiros/spark3_II/assembly/target/scala-2.13/jars/netty-common-4.2.13.Final.jar)
WARNING: Please consider reporting this to the maintainers of class io.netty.util.internal.PlatformDependent0$2
WARNING: sun.misc.Unsafe::allocateMemory will be removed in a future release
26/05/19 15:15:24 WARN Utils: Service 'SparkUI' could not bind on port 4040. Attempting port 4041.
26/05/19 15:15:24 WARN Utils: Service 'SparkUI' could not bind on port 4041. Attempting port 4042.
Spark context Web UI available at http://10.96.96.252:4042
Spark context available as 'sc' (master = local[*], app id = local-1779228924892).
Spark session available as 'spark'.
scala>
```
But the test not. This changes everything!
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493664189
The `--sun-misc-unsafe-memory-access=allow` flag solves the problem:
```
[INFO] --- scalatest:2.2.0:test (test) @ spark-connect-client-jvm_2.13 ---
[INFO] ScalaTest report directory: /Users/apiros/git/attilapiros/spark3_II/sql/connect/client/jvm/target/surefire-reports
WARNING: Using incubator modules: jdk.incubator.vector
WARNING: package sun.security.action not in java.base
Discovery starting.
Discovery completed in 110 milliseconds.
Run starting. Expected test count is: 1
AmmoniteReplE2ESuite:
WARNING: Using incubator modules: jdk.incubator.vector
WARNING: package sun.security.action not in java.base
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::allocateMemory has been called by io.netty.util.internal.PlatformDependent0$2 (file:/Users/apiros/.m2/repository/io/netty/netty-common/4.2.13.Final/netty-common-4.2.13.Final.jar)
WARNING: Please consider reporting this to the maintainers of class io.netty.util.internal.PlatformDependent0$2
WARNING: sun.misc.Unsafe::allocateMemory will be removed in a future release
Ready for client connections.
- SPARK-56448: restarting spark-shell --remote does not throw NPE
Run completed in 19 seconds, 109 milliseconds.
Total number of tests run: 1
Suites: completed 1, aborted 0
Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
No more client connections.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 33.246 s
[INFO] Finished at: 2026-05-19T18:21:40-07:00
[INFO] ------------------------------------------------------------------------
➜ spark3_II git:(bd8872a0cc7) ✗ java -version
openjdk version "25.0.3" 2026-04-21 LTS
OpenJDK Runtime Environment Temurin-25.0.3+9 (build 25.0.3+9-LTS)
```
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4491799480
@dongjoon-hyun Thanks for confirming the JDK 25 + UBI 10 environment.
The production fix (Storage.InMemory) is JDK-independent as it's pure ammonite/Scala code with no Arrow/Netty dependency. The test failure is because spark-shell --remote initializes Arrow, which hits the Netty memoryAddress() incompatibility on JDK 25. Based on my understanding, this would affect any test that launches spark-shell on JDK 25, not just ours.
I've submitted [PR #55999](https://github.com/apache/spark/pull/55999) to skip the test on JDK 25+ with an assume() guard. Please let me know if that is sufficient for now or if you'd like to disable the test some other way.
Also filed [SPARK-56955](https://issues.apache.org/jira/browse/SPARK-56955) to track the underlying Arrow/Netty JDK 25 issue separately.
@attilapiros Could you please keep me honest here on the production change (Storage.InMemory in ConnectRepl.scala) has no JDK-version sensitivity? It should be safe on all JDKs since it's just ammonite cache configuration, right?
--
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
Re: [PR] [SPARK-56448][CONNECT] Fix NPE on Spark Connect client restart due to ammonite compile cache [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #55720:
URL: https://github.com/apache/spark/pull/55720#issuecomment-4493813044
Opened https://github.com/apache/spark/pull/56006.
--
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