(datafusion-comet) 01/01: ci: rebalance the Spark SQL test shards (#6393)

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6393-aca67fd581fb1838a2745f5676b49f4deee3bb8d
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git

commit bd1d4d113cd4524b44927d59f10adc5791997a47
Author: Andy Grove <ag...@apache.org>
AuthorDate: Tue Sep 29 14:45:02 2026 +0000

    ci: rebalance the Spark SQL test shards (#6393)
    
    The Spark SQL suite sets the length of every merge-queue run, and two of
    its seven rows ran far longer than the rest: sql_hive-1 (median 67 min)
    and sql_core-1 (60 min), against 21 and 28 for their shortest siblings.
    
    Move the suites responsible into two new rows, excluding them from their
    old rows with sbt's `-<glob>` testOnly syntax and keeping each row's tag
    filters so every test still runs exactly once:
    
    - sql_hive-4: HivePartitionFilteringSuites, which runs its suite against
      every Hive client version and was 21 to 35 minutes of sql_hive-1.
    - sql_core-4: the untagged and Extended tests under
      org.apache.spark.sql.execution.datasources and
      org.apache.spark.sql.connector, 13 to 19 minutes of sql_core-1 plus
      11 of sql_core-2. Their Slow tests stay in sql_core-3.
    
    In the two queue runs sampled, the slowest row drops from 72 and 51
    minutes to about 48 and 40, at the cost of two more runners per run.
    
    Closes #6388.
---
 .github/workflows/spark_sql_test_reusable.yml |  4 +-
 dev/ci/spark-sql-modules.py                   | 58 ++++++++++++++++++++++++---
 dev/local-ci.sh                               |  6 +--
 docs/source/contributor-guide/ci.md           | 10 ++---
 4 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/.github/workflows/spark_sql_test_reusable.yml b/.github/workflows/spark_sql_test_reusable.yml
index 90a55bbdf1..3515d64899 100644
--- a/.github/workflows/spark_sql_test_reusable.yml
+++ b/.github/workflows/spark_sql_test_reusable.yml
@@ -42,8 +42,8 @@ on:
         default: false
         type: boolean
       modules:
-        # Which matrix rows to run. `core` is catalyst plus the three sql_core
-        # shards, `hive` is the three sql_hive shards, `all` is both. The
+        # Which matrix rows to run. `core` is catalyst plus the four sql_core
+        # shards, `hive` is the four sql_hive shards, `all` is both. The
         # umbrella uses this to keep the Spark 4.1 hive shards out of the PR
         # tier without a second call (and a second 40-minute build) for the
         # queue: one call, and the rows the tier does not want are left out
diff --git a/dev/ci/spark-sql-modules.py b/dev/ci/spark-sql-modules.py
index ff644abd37..471fdac498 100644
--- a/dev/ci/spark-sql-modules.py
+++ b/dev/ci/spark-sql-modules.py
@@ -39,13 +39,43 @@ from pathlib import Path
 # "Run Spark tests" step as per-row forked-test-JVM caps for SparkBuild.scala;
 # the sql_core rows set them because those shards were the ones hitting the
 # 7 GB runner budget.
+#
+# The tag filters split each module three ways, the same as Spark's own CI.
+# That left two rows far longer than the rest, and the longest row is what a
+# merge-queue run waits for, so the suites below are moved into rows of their
+# own (issue #6388). Each is excluded from the row it came from with sbt's
+# `-<glob>` testOnly syntax, and its row keeps that row's tag filters, so every
+# test still runs exactly once. The globs are package prefixes, which also
+# catch suites added to those packages later.
+#
+# HivePartitionFilteringSuites runs HivePartitionFilteringSuite against every
+# Hive client version, 21 to 35 minutes of sql_hive-1 in September 2026. It is
+# one class, so no name filter splits it further.
+HIVE_MOVED = ["org.apache.spark.sql.hive.client.HivePartitionFilteringSuites"]
+# The data source and connector suites were 13 to 19 minutes of sql_core-1 and,
+# with their Extended-tagged schema-pruning suites, 11 minutes of sql_core-2.
+CORE_MOVED = [
+    "org.apache.spark.sql.execution.datasources.*",
+    "org.apache.spark.sql.connector.*",
+]
+
+SQL_EXTENDED = "org.apache.spark.tags.ExtendedSQLTest"
+SQL_SLOW = "org.apache.spark.tags.SlowSQLTest"
+HIVE_EXTENDED = "org.apache.spark.tags.ExtendedHiveTest"
+HIVE_SLOW = "org.apache.spark.tags.SlowHiveTest"
+
+
+def excluding(globs):
+    return " ".join(f"-{glob}" for glob in globs)
+
+
 MODULES = [
     {"name": "catalyst", "group": "core", "args1": "catalyst/test", "args2": ""},
     {
         "name": "sql_core-1",
         "group": "core",
         "args1": "",
-        "args2": "sql/testOnly * -- -l org.apache.spark.tags.ExtendedSQLTest -l org.apache.spark.tags.SlowSQLTest",
+        "args2": f"sql/testOnly * {excluding(CORE_MOVED)} -- -l {SQL_EXTENDED} -l {SQL_SLOW}",
         "heap": "3g",
         "metaspace": "1g",
     },
@@ -53,7 +83,7 @@ MODULES = [
         "name": "sql_core-2",
         "group": "core",
         "args1": "",
-        "args2": "sql/testOnly * -- -n org.apache.spark.tags.ExtendedSQLTest",
+        "args2": f"sql/testOnly * {excluding(CORE_MOVED)} -- -n {SQL_EXTENDED}",
         "heap": "3g",
         "metaspace": "1g",
     },
@@ -61,7 +91,17 @@ MODULES = [
         "name": "sql_core-3",
         "group": "core",
         "args1": "",
-        "args2": "sql/testOnly * -- -n org.apache.spark.tags.SlowSQLTest",
+        "args2": f"sql/testOnly * -- -n {SQL_SLOW}",
+        "heap": "3g",
+        "metaspace": "1g",
+    },
+    {
+        # The moved suites' untagged and Extended tests; their Slow tests stay
+        # in sql_core-3 with every other Slow test.
+        "name": "sql_core-4",
+        "group": "core",
+        "args1": "",
+        "args2": f"sql/testOnly {' '.join(CORE_MOVED)} -- -l {SQL_SLOW}",
         "heap": "3g",
         "metaspace": "1g",
     },
@@ -69,19 +109,25 @@ MODULES = [
         "name": "sql_hive-1",
         "group": "hive",
         "args1": "",
-        "args2": "hive/testOnly * -- -l org.apache.spark.tags.ExtendedHiveTest -l org.apache.spark.tags.SlowHiveTest",
+        "args2": f"hive/testOnly * {excluding(HIVE_MOVED)} -- -l {HIVE_EXTENDED} -l {HIVE_SLOW}",
     },
     {
         "name": "sql_hive-2",
         "group": "hive",
         "args1": "",
-        "args2": "hive/testOnly * -- -n org.apache.spark.tags.ExtendedHiveTest",
+        "args2": f"hive/testOnly * -- -n {HIVE_EXTENDED}",
     },
     {
         "name": "sql_hive-3",
         "group": "hive",
         "args1": "",
-        "args2": "hive/testOnly * -- -n org.apache.spark.tags.SlowHiveTest",
+        "args2": f"hive/testOnly * -- -n {HIVE_SLOW}",
+    },
+    {
+        "name": "sql_hive-4",
+        "group": "hive",
+        "args1": "",
+        "args2": f"hive/testOnly {' '.join(HIVE_MOVED)} -- -l {HIVE_EXTENDED} -l {HIVE_SLOW}",
     },
 ]
 
diff --git a/dev/local-ci.sh b/dev/local-ci.sh
index a6a9d57d83..853108aeef 100755
--- a/dev/local-ci.sh
+++ b/dev/local-ci.sh
@@ -254,7 +254,7 @@ spark_row() {
 }
 
 # Every selected row at once, each in its own copy of the tree. That is exactly
-# what CI does: seven matrix rows, seven runners, seven extracted apache-spark/
+# what CI does: nine matrix rows, nine runners, nine extracted apache-spark/
 # trees. Because each row gets a tree to itself, the per-row settings stay
 # identical to CI's -- no shared sbt server, target/ or metastore tmpdir.
 run_spark_rows() {
@@ -271,7 +271,7 @@ run_spark_rows() {
   running=""
   while IFS=$'\037' read -r name args1 args2 heap metaspace; do
     [ -n "$name" ] || continue
-    # Seven concurrent sbt processes would interleave unreadably, so each row
+    # Nine concurrent sbt processes would interleave unreadably, so each row
     # gets its own log.
     spark_row "$name" "$args1" "$args2" "$heap" "$metaspace" "$dest-$name" \
       > "$logs/$name.log" 2>&1 &
@@ -313,7 +313,7 @@ run_spark() {
     purge_partial_poms
     drop_cached_resolution "$dest"
     # Only what the selected rows need. CI compiles all three because one
-    # artifact feeds seven shards; there is no artifact here.
+    # artifact feeds nine shards; there is no artifact here.
     compile=""
     for p in $PROJECTS; do compile="$compile $p/Test/compile"; done
     say "pre-compiling test classes:$compile"
diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md
index eb2c1f485b..9788ad3ed6 100644
--- a/docs/source/contributor-guide/ci.md
+++ b/docs/source/contributor-guide/ci.md
@@ -339,7 +339,7 @@ gh run view "$run" --repo apache/datafusion-comet --json jobs \
         | group_by(.conclusion)[] | "\(length)\t\(.[0].conclusion)"'
 ```
 
-A healthy run over a day of normal merges reports about 47 successes — seven Spark SQL shards for
+A healthy run over a day of normal merges reports about 53 successes — nine Spark SQL shards for
 each of 3.5, 4.0 and 4.2, plus the Iceberg shards — with the handful of skips being the suites that
 belong to the queue tier rather than the nightly one — Spark 3.4, Spark 4.1
 and Iceberg 1.11. If everything is skipped, open the run's `Detect changes` job: it logs the
@@ -405,12 +405,12 @@ the sbt projects the selected rows need.
 changing Comet or the run tests the previously installed JAR and goes green regardless.
 
 When more than one Spark row is selected they all run **at once**, each in its own copy of the
-prepared tree, which is what CI does: seven matrix rows, seven runners, seven extracted trees.
+prepared tree, which is what CI does: nine matrix rows, nine runners, nine extracted trees.
 Because each row owns a tree there is no shared sbt server, `target/` or metastore tmpdir, so the
 per-row settings stay identical to CI's. On APFS and btrfs the copies are copy-on-write, so a 4 GB
 tree costs kilobytes until the rows write their own reports.
 
-Seven concurrent sbt processes would interleave unreadably, so each row logs to
+Nine concurrent sbt processes would interleave unreadably, so each row logs to
 `$COMET_LOCAL_CI_HOME/logs-spark-<version>/<row>.log`, named on the line that reports the row
 starting. Each row then reports again when it finishes, with its elapsed time, and a failing row
 prints the last 20 lines of its log. Follow a row live with `tail -f`. The trees persist so the
@@ -421,8 +421,8 @@ Four caveats:
 
 - The sandbox lives under `/tmp`, so a reboot or a tmp reaper means downloading and compiling again.
   Point `COMET_LOCAL_CI_HOME` somewhere durable to keep it.
-- Running every row at once wants the memory and disk for it: seven sbt processes each forking a
-  test JVM, and seven trees diverging from their copy-on-write base. Select fewer rows, or one, on a
+- Running every row at once wants the memory and disk for it: nine sbt processes each forking a
+  test JVM, and nine trees diverging from their copy-on-write base. Select fewer rows, or one, on a
   smaller machine.
 - Preparing deletes `org/apache/parquet` from your local Maven repository as the workflows do, and
   additionally sweeps the **whole** repository for POMs with no sibling JAR. That is a shared cache,


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org