Skip to content

Commit

Permalink
apacheGH-38684: [Integration] Try to strengthen C Data Interface test…
Browse files Browse the repository at this point in the history
…ing (apache#38846)

### Rationale for this change

C Data Interface integration tests sometimes crash with a complicated traceback involving Java, Go and C# signal handlers.

### What changes are included in this PR?

* Update JDK version in integration build
* Disable signal-based memory management in the JVM so as to improve cooperation with other runtimes
* Set memory limits to the various managed runtimes (Java, .Net, Go)
* Enable some checks in the Go runtime
* Enable debug allocator in Arrow Java

### Are these changes tested?

Yes. I am not able to reproduce any sporadic crash using these changes.

### Are there any user-facing changes?

No.

* Closes: apache#38684

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
  • Loading branch information
pitrou authored Nov 22, 2023
1 parent 1e769b0 commit 3726773
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 6 deletions.
1 change: 0 additions & 1 deletion .github/workflows/java.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ env:
DOCKER_VOLUME_PREFIX: ".docker/"

jobs:

ubuntu:
name: AMD64 Ubuntu 22.04 Java JDK ${{ matrix.jdk }} Maven ${{ matrix.maven }}
runs-on: ubuntu-latest
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/java_jni.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ env:
DOCKER_VOLUME_PREFIX: ".docker/"

jobs:

docker:
name: AMD64 manylinux2014 Java JNI
runs-on: ubuntu-latest
Expand Down
5 changes: 5 additions & 0 deletions ci/scripts/integration_arrow.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ fi
# Get more detailed context on crashes
export PYTHONFAULTHANDLER=1

# Due to how Go reads environment variables, we have to set them from the calling
# process, or they would get ignored.
# (see https://forum.golangbridge.org/t/are-godebug-and-other-env-vars-ignored-when-loading-a-go-dll-from-foreign-code/33694)
export GOMEMLIMIT=200MiB
export GODEBUG=gctrace=1,clobberfree=1

# Rust can be enabled by exporting ARCHERY_INTEGRATION_WITH_RUST=1
time archery integration \
Expand Down
1 change: 1 addition & 0 deletions dev/archery/archery/integration/tester_cpp.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ def make_c_data_importer(self):

@functools.lru_cache
def _load_ffi(ffi, lib_path=_ARROW_DLL):
os.environ['ARROW_DEBUG_MEMORY_POOL'] = 'trap'
ffi.cdef(_cpp_c_data_entrypoints)
dll = ffi.dlopen(lib_path)
dll.ArrowCpp_CDataIntegration_ExportSchemaFromJson
Expand Down
1 change: 1 addition & 0 deletions dev/archery/archery/integration/tester_csharp.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def _load_clr():
global _clr_loaded
if not _clr_loaded:
_clr_loaded = True
os.environ['DOTNET_GCHeapHardLimit'] = '0xC800000' # 200 MiB
import pythonnet
pythonnet.load("coreclr")
import clr
Expand Down
3 changes: 3 additions & 0 deletions dev/archery/archery/integration/tester_go.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ def make_c_data_importer(self):

@functools.lru_cache
def _load_ffi(ffi, lib_path=_INTEGRATION_DLL):
# NOTE that setting Go environment variables here (such as GODEBUG)
# would be ignored by the Go runtime. The environment variables need
# to be set from the process calling Archery.
ffi.cdef(_go_c_data_entrypoints)
dll = ffi.dlopen(lib_path)
return dll
Expand Down
13 changes: 10 additions & 3 deletions dev/archery/archery/integration/tester_java.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ def load_version_from_pom():
return version_tag.text


# XXX Should we add "-Darrow.memory.debug.allocator=true"? It adds a couple
# minutes to total CPU usage of the integration test suite.
# NOTE: we don't add "-Darrow.memory.debug.allocator=true" here as it adds a
# couple minutes to total CPU usage of the integration test suite
# (see setup_jpype() below).
_JAVA_OPTS = [
"-Dio.netty.tryReflectionSetAccessible=true",
"-Darrow.struct.conflict.policy=CONFLICT_APPEND",
"--add-opens=java.base/java.nio=ALL-UNNAMED",
]

_arrow_version = load_version_from_pom()
Expand Down Expand Up @@ -80,7 +82,12 @@ def setup_jpype():
jar_path = f"{_ARROW_TOOLS_JAR}:{_ARROW_C_DATA_JAR}"
# XXX Didn't manage to tone down the logging level here (DEBUG -> INFO)
jpype.startJVM(jpype.getDefaultJVMPath(),
"-Djava.class.path=" + jar_path, *_JAVA_OPTS)
"-Djava.class.path=" + jar_path,
# This flag is too heavy for IPC and Flight tests
"-Darrow.memory.debug.allocator=true",
# Reduce internal use of signals by the JVM
"-Xrs",
*_JAVA_OPTS)


class _CDataBase:
Expand Down
3 changes: 2 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1706,7 +1706,8 @@ services:
args:
repo: ${REPO}
arch: ${ARCH}
jdk: ${JDK}
# Use a newer JDK as it seems to improve stability
jdk: 17
# conda-forge doesn't have 3.5.4 so pinning explicitly, but this should
# be set to ${MAVEN}
maven: 3.5
Expand Down

0 comments on commit 3726773

Please sign in to comment.