Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions integration_tests/src/main/python/py4j_workaround_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Copyright (c) 2026, NVIDIA CORPORATION.
#
# Licensed 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.

import gc
import logging
import weakref

from spark_init_internal import (
_apply_py4j_strong_ref_workaround_if_needed,
get_spark_i_know_what_i_am_doing,
)


class _Container:
pass


def test_py4j_chained_scala_map_get():
spark = get_spark_i_know_what_i_am_doing()
scala_map = spark.conf._jconf.getAll()
keys = scala_map.keys().iterator()

assert keys.hasNext()
key = keys.next()
Comment on lines +28 to +35

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing CPU/GPU session coverage

This Spark-backed regression invokes the chained JVM map access only through the shared default session, so it can pass without confirming that the same call remains functional across the harness's CPU and GPU session configurations. Use the repository's CPU/GPU comparison or fallback assertion path for this integration test.

Rule Used: Integration tests must verify GPU execution using ... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regression targets driver-side Py4J JavaMember lifetime, not Spark query execution. with_cpu_session and with_gpu_session share the same SparkSession and gateway and only toggle RAPIDS SQL configuration, so they do not exercise a different lifetime path. The GPU/fallback assertion helpers require a DataFrame and execution plan, while this test intentionally exercises the exact scala_map.get(key).get() failure shape directly after the workaround is installed before SparkSession creation. Importing spark_session to use those helpers also runs the vulnerable _from_scala_map path during module initialization. A CPU/GPU split is therefore not applicable here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — this regression is specifically about driver-side Py4J object lifetime, and the CPU/GPU helpers would not provide additional coverage because they reuse the same SparkSession and gateway while only changing RAPIDS SQL configuration. They also introduce the unrelated DataFrame/plan path and can trigger the vulnerable initialization earlier. The direct chained-call test is the appropriate coverage here, so the CPU/GPU session coverage request does not apply.

assert scala_map.get(key).get() == scala_map.apply(key)


def test_py4j_weak_container_is_replaced_with_strong_reference():
class WeakJavaMember:
def __init__(self, name, container, target_id, gateway_client):
self.container = weakref.ref(container)

assert _apply_py4j_strong_ref_workaround_if_needed(WeakJavaMember)

container = _Container()
container_ref = weakref.ref(container)
member = WeakJavaMember('get', container, 'o1', None)
del container
gc.collect()

assert container_ref() is member.container


def test_py4j_workaround_is_idempotent():
class WeakJavaMember:
def __init__(self, name, container, target_id, gateway_client):
self.container = weakref.ref(container)

assert _apply_py4j_strong_ref_workaround_if_needed(WeakJavaMember)
patched_init = WeakJavaMember.__init__

assert not _apply_py4j_strong_ref_workaround_if_needed(WeakJavaMember)
assert WeakJavaMember.__init__ is patched_init


def test_py4j_upstream_strong_container_is_unchanged():
class StrongJavaMember:
def __init__(self, name, container, target_id, gateway_client):
self.container = container

original_init = StrongJavaMember.__init__

assert not _apply_py4j_strong_ref_workaround_if_needed(StrongJavaMember)
assert StrongJavaMember.__init__ is original_init


def test_py4j_probe_failure_skips_workaround(caplog):
class FailingJavaMember:
def __init__(self, name, container, target_id, gateway_client):
raise RuntimeError('probe failed')

original_init = FailingJavaMember.__init__
with caplog.at_level(logging.ERROR):
assert not _apply_py4j_strong_ref_workaround_if_needed(FailingJavaMember)

assert FailingJavaMember.__init__ is original_init
assert 'continuing without the temporary integration-test workaround' in caplog.text
65 changes: 65 additions & 0 deletions integration_tests/src/main/python/spark_init_internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import functools
import logging
import os
import pytest
import re
import stat
import traceback
import weakref

logging.basicConfig(
format="%(asctime)s %(levelname)-8s %(message)s",
Expand Down Expand Up @@ -55,6 +57,60 @@ def conf_for_env(env_name):
# does not raise NameError if session startup fails before assignment.
_spark = None

_PY4J_STRONG_REF_WORKAROUND_MARKER = '_spark_rapids_strong_ref_workaround'


def _java_member_uses_weak_container(java_member_class):
class _ProbeContainer:
pass

class _ProbeGatewayProperty:
pool = None

class _ProbeGatewayClient:
gateway_property = _ProbeGatewayProperty()
converters = ()

container = _ProbeContainer()
member = java_member_class(
'_spark_rapids_container_lifetime_probe', container, 'o0', _ProbeGatewayClient())
return isinstance(member.container, weakref.ReferenceType)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probe constructs JavaMember with a fake gateway client and is invoked from pytest_sessionstart with no try/except. If Dataproc's patched __init__ touches more of the gateway than upstream, the xdist worker fails before any test runs. Probe failures should log and skip the patch rather than abort session startup.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated.



def _apply_py4j_strong_ref_workaround_if_needed(java_member_class=None):
"""Restore upstream JavaMember container lifetime semantics in this IT process."""
if java_member_class is None:
from py4j.java_gateway import JavaMember
java_member_class = JavaMember

original_init = java_member_class.__init__
if getattr(original_init, _PY4J_STRONG_REF_WORKAROUND_MARKER, False):
return False
try:
uses_weak_container = _java_member_uses_weak_container(java_member_class)
except Exception:
logging.exception(
"Could not probe Py4J JavaMember container lifetime; continuing without the "
"temporary integration-test workaround")
return False
if not uses_weak_container:
return False

@functools.wraps(original_init)
def strong_ref_init(self, name, container, target_id, gateway_client):
original_init(self, name, container, target_id, gateway_client)
if isinstance(self.container, weakref.ReferenceType):
self.container = container

setattr(strong_ref_init, _PY4J_STRONG_REF_WORKAROUND_MARKER, True)
java_member_class.__init__ = strong_ref_init
logging.warning(
"Detected weak Py4J JavaMember container references; applying the temporary "
"integration-test strong-reference workaround for the missing-target failures in "
"https://github.com/NVIDIA/cudf-spark/issues/15805")
return True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The weak-container analysis explained the Jenkins #249 missing-target failures. The original #248 ORC boolean-encoding signal on #15805 was called out as separate, so this workaround should not be treated as covering every failure on that issue.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.



def findspark_init():
import findspark
findspark.init()
Expand Down Expand Up @@ -120,6 +176,15 @@ def pytest_sessionstart(session):
import pyspark
from py4j.java_gateway import java_import

# Dataproc 2.2.86 can delete temporary JVM targets before chained Py4J calls complete.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a process-wide monkeypatch of the IT harness, not a Dataproc/Py4J platform fix. That boundary is fine, but a green Linux/Databricks IT run should not be read as fixing user pyspark jobs on 2.2.86. Also worth wrapping this call so a probe exception cannot take down the whole worker.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

# Keep this before SparkSession creation. The behavior probe makes it a no-op for upstream Py4J.
try:
_apply_py4j_strong_ref_workaround_if_needed()
except Exception:
logging.exception(
"Could not initialize the temporary Py4J integration-test workaround; "
"continuing without it")

# Force the RapidsPlugin to be enabled, so it blows up if the classpath is not set properly
# DO NOT SET ANY OTHER CONFIGS HERE!!!
# due to bugs in pyspark/pytest it looks like any configs set here
Expand Down
Loading