Copilot commented on code in PR #12388:
URL: https://github.com/apache/gluten/pull/12388#discussion_r3661712267


##########
.github/workflows/util/delta-spark-ut/run-delta-tests.sh:
##########
@@ -0,0 +1,275 @@
+#!/usr/bin/env bash
+
+# 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.
+
+#
+# Runs the Delta `spark` module tests for one shard under the Gluten bundle:
+# arms a hang watchdog (thread-dumps + kills a wedged fork), invokes sbt
+# spark/test with the tuned JVM/heap flags, prints cgroup memory forensics, and
+# then gates the results against the baseline (compare-test-results.py). 
Extracted
+# from delta_spark_ut.yml so the workflow step stays readable.
+#
+# Driven by environment (set by the workflow step / job):
+#   SHARD_ID          - this shard's id (matrix.shard)
+#   SPARK_VERSION     - Delta -DsparkVersion value
+#   UPDATE_BASELINE   - 'true' -> gate seed mode; else enforce
+#   FAIL_ON_FIXED     - passed through to the gate
+#   DELTA_SCALA_VERSION, NUM_SHARDS, TEST_PARALLELISM_COUNT, DELTA_TESTING
+#                     - test env (see the workflow step's `env:` block)
+#   GITHUB_WORKSPACE  - repo root (holds the Delta clone + util scripts)
+#
+# JAVA_TOOL_OPTIONS is set by sourcing java-test-args.sh (below), not the 
caller.
+
+set -euo pipefail
+export JAVA_HOME=/usr/lib/jvm/java-17-openjdk
+export PATH=$JAVA_HOME/bin:$PATH
+# Gluten/JDK17 test JVM flags (--add-opens + Netty property), shared with local
+# dev runs. Sets JAVA_TOOL_OPTIONS so it reaches the sbt launcher + forked 
JVMs.
+# shellcheck source=./java-test-args.sh
+source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/java-test-args.sh"
+cd "$GITHUB_WORKSPACE/delta"
+chmod +x build/sbt
+# Only run the unified `spark` sbt project, NOT `sparkGroup/test` --
+# `sparkGroup` aggregates many other projects (sparkV2, contribs,
+# sharing, connect*, ...) that are out of scope for this pipeline.
+#
+# JVM heap layout -- two memory consumers on the ~16G runner:
+# * sbt launcher JVM: -J-Xmx4G for the test compile, then forced to
+#   return idle memory during the (long) test phase via G1 periodic GC
+#   (G1PeriodicGCInterval=10s; G1PeriodicGCSystemLoadThreshold=0 so the
+#   busy fork doesn't suppress it; -XX:-G1PeriodicGCInvokesConcurrent
+#   forces each periodic GC to a full STW collection) that uncommits to a
+#   tight free ratio (Min/MaxHeapFreeRatio 5/15, JEP 346) above a low
+#   -Xms512m floor.
+#   Without this the idle launcher holds ~5.3G for the whole run; with
+#   it, it drops back to ~1-2G. These flags touch no Gluten/Spark runtime
+#   config, so they cannot affect the measured pass/fail signal.
+# * Forked test JVM: -Xmx2G via the `set ... Test / javaOptions` command
+#   below. Delta caps its fork at -Xmx1024m in build.sbt; `++=` appends
+#   so our -Xmx2G comes last and wins. Gluten offloads data to Velox
+#   off-heap (capped at 2g via spark.memory.offHeap.size in the patched
+#   DeltaSQLCommandTest), so the fork's heap need is modest. A larger
+#   fork heap pushed the cgroup peak past the ~16G OOM threshold and the
+#   kernel OOM-killed the fork mid-shard (no hs_err), wedging sbt -- 2G
+#   keeps headroom. Keep heap-dump-on-OOM so a real >2G heap OOM is
+#   analyzable.
+# `-u target/test-reports` enables ScalaTest's JUnit XML reporter so
+# every suite writes per-test results. Delta itself only configures
+# the console reporter (-oDF), so without this we'd have no machine-
+# readable results to gate on. The path is relative to the forked
+# test JVM's working dir (Test / baseDirectory = spark/), i.e.
+# delta/spark/target/test-reports/TEST-*.xml.
+#
+# We deliberately do NOT let an sbt non-zero exit (which fires on the
+# MANY expected Delta-on-Gluten failures) fail this step directly.
+# Instead the known-failures gate below decides pass/fail: the build
+# is green when the only failures are ones already recorded in the
+# baseline, and red on a genuine regression.
+set +e
+# --- hang watchdog ---------------------------------------------------
+# Shard 2 (and occasionally others) hangs indefinitely after a suite's
+# last test with no further output. ScalaTest's failAfter only wraps
+# individual test BODIES, so a wedge in suite teardown/afterAll -- or in
+# a non-interruptible native Velox/JNI call that ignores
+# Thread.interrupt() -- has no timeout and stalls until the 350-min job
+# limit with zero diagnostics. This watchdog dumps the forked test JVM's
+# threads (to the job log, and to a file for the artifact) once the test
+# output has been silent for too long, so the deadlock is diagnosable.
+SBT_LOG="/tmp/sbt-spark-test-shard-${SHARD_ID}.log"
+# Marker the watchdog touches when it KILLS a wedged test fork. The killed 
fork's
+# running suite plus every suite queued behind it never run and never write a
+# report, and since we ignore sbt's exit code the gate would only judge the
+# suites that DID report -- so the main flow fails the shard when this exists.
+WATCHDOG_KILL_MARKER="/tmp/sbt-watchdog-killed-shard-${SHARD_ID}"
+: > "$SBT_LOG"
+rm -f /tmp/sbt-done "$WATCHDOG_KILL_MARKER"

Review Comment:
   The hang watchdog completion marker uses a non-shard-specific path 
(/tmp/sbt-done). Since this script is parameterized by SHARD_ID (and already 
uses shard-specific log/kill marker names), a global marker can cause 
interference if multiple shards are run concurrently in the same environment 
(e.g., local parallel runs). Make the marker shard-specific here so each 
watchdog tracks only its own shard.
   
   This issue also appears in the following locations of the same file:
   - line 135
   - line 222



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to