From cc9c47eaf223811f652ca1a6ef950eb175e4a58d Mon Sep 17 00:00:00 2001 From: comphead Date: Thu, 17 Sep 2026 17:29:23 -0700 Subject: [PATCH 1/2] ci: add dev/local-ci.sh to run the Spark SQL and Iceberg suites locally The Spark SQL and Iceberg suites do not run on an unlabeled pull request. Spark 4.1 and Iceberg 1.11 first report in the merge queue, where a failure evicts the pull request and blocks everyone else's merges. Spark 3.5, 4.0 and Iceberg 1.8/1.9/1.10 first report in the nightly run, after the change has landed. Those two workflows are 522 of the 891 runner-minutes a queue pipeline costs. The commands are documented but nothing executes them, so every local run is retyped. The parts that are easy to get wrong are the parts that decide whether a local pass means anything: the -l/-n tag splits, the per-row heap caps, the Iceberg shard init script, and the Maven cache purges the workflows depend on. dev/local-ci.sh spark every Spark SQL matrix row dev/local-ci.sh spark sql_core-1 one row, or all/core/hive dev/local-ci.sh iceberg every Iceberg target dev/local-ci.sh iceberg shard-2 one shard, or extensions/runtime Selecting more than one Spark row runs them all at once, each in its own copy of the prepared tree. That is what CI does -- one runner and one extracted apache-spark/ per matrix row -- so the per-row settings stay byte-identical to the workflow's rather than diverging to get parallelism. Budget a full tree per row: the copies are copy-on-write at first but diverge almost completely as each row recompiles and writes its own reports. dev/ci/local-ci-config.py is the single parser for everything the script reads: versions and the JDK from ci.yml, the matrix rows from spark-sql-modules.py, the shard count from check-iceberg-shards.py, the Iceberg Scala default and the DEDICATED_JVM_SBT_TESTS gate from the reusable workflows, and the default version from POLICY in compute-changes.py. It validates every value against a version shape, so a requoted or reindented input fails loudly instead of producing a URL with quotes in it. check_local_ci_config imports it during preflight and adds bash -n on the script, because actionlint runs with --shellcheck=off and only looks at workflow files. Three failure modes found while using it, each guarded now: Comet's install leaves POMs whose JARs it never fetched, which Coursier reports as a missing JAR it can see a POM for; purging those invalidates any resolution sbt has cached, which a plugin like sbt-antlr4 reads instead of the filesystem; and editing a diff used to wedge the tree until it was deleted, so the applied diff is recorded and reverted before a new one goes on. Three findings from review, each reproduced first. `eval "$(...)"` reports eval's own status, so a parser that failed and printed nothing looked like success and the run exited 0 having tested nothing; the assignment is captured and checked before evaluating. Overlapping selectors such as `core sql_core-1` named a row twice, and because a row's tree and log are keyed on its name the duplicate deleted the tree the first copy was running in; selection now deduplicates by name. A tree copy failing after earlier rows had launched exited on set -e without awaiting them, leaving sbt processes behind; every tree is copied before any row starts. --- dev/ci/check-ci-config.py | 58 +++++ dev/ci/local-ci-config.py | 261 +++++++++++++++++++++++ dev/local-ci.sh | 431 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 750 insertions(+) create mode 100644 dev/ci/local-ci-config.py create mode 100755 dev/local-ci.sh diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index abb6b1f2d4b..80fdeea439e 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -67,11 +67,13 @@ import importlib.util import re +import subprocess import sys from pathlib import Path WORKFLOWS = Path(".github/workflows") ASF_YAML = Path(".asf.yaml") +LOCAL_CI = Path("dev/local-ci.sh") # The ci.yml job that aggregates every other job's result. AGGREGATOR_JOB = "required_checks" @@ -483,6 +485,61 @@ def load_filters(): return module +def load_ci_module(name, filename): + """Import one of the hyphenated dev/ci scripts as a module.""" + spec = importlib.util.spec_from_file_location(name, f"dev/ci/{filename}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def check_local_ci_config(): + """dev/local-ci.sh reads ci.yml and dev/ci; make that drift fail here. + + The values live in dev/ci/local-ci-config.py, the single parser, which + validates each one against a version shape. Importing it here is what turns + a requoted `spark-full`, a reindented `with:` block or a renamed job into a + preflight failure rather than a surprise the next time somebody needs the + script. `bash -n` covers the shell, since actionlint runs with + --shellcheck=off and looks only at workflow files. + """ + if not LOCAL_CI.exists(): + return True + failures = [] + + syntax = subprocess.run( + ["bash", "-n", str(LOCAL_CI)], capture_output=True, text=True, check=False + ) + if syntax.returncode != 0: + failures.append(f"{LOCAL_CI} is not valid bash: {syntax.stderr.strip()}") + + try: + config = load_ci_module("local_ci_config", "local-ci-config.py").config() + # Each default must name a job ci.yml actually defines, or the script + # defaults to a version it then cannot look up. + for suite in ("spark", "iceberg"): + if not config[suite]: + failures.append(f"local-ci-config.py found no {suite}_* jobs in ci.yml") + elif config[f"{suite}_default"] not in config[suite]: + failures.append( + f"local-ci-config.py defaults {suite} to " + f"{config[f'{suite}_default']}, which ci.yml has no job for" + ) + if config["dedicated_gate_version"] not in config["spark"]: + failures.append( + "the DEDICATED_JVM_SBT_TESTS gate names Spark " + f"{config['dedicated_gate_version']}, which ci.yml has no job for" + ) + if not config["rows"]: + failures.append("local-ci-config.py found no Spark SQL matrix rows") + except Exception as err: # noqa: BLE001 - any parse failure is the finding + failures.append(f"dev/ci/local-ci-config.py could not read the CI config: {err}") + + for failure in failures: + print(f"local-ci: {failure}") + return not failures + + def check_spark_sql_modules(): """`--modules core` and `--modules hive` must partition `--modules all`. @@ -1143,6 +1200,7 @@ def check_cache_save_scope(): ok = check_nightly_scope() and ok ok = check_nightly_base_fallback() and ok ok = check_cache_save_scope() and ok + ok = check_local_ci_config() and ok if not ok: sys.exit(1) print("CI config checks passed") diff --git a/dev/ci/local-ci-config.py b/dev/ci/local-ci-config.py new file mode 100644 index 00000000000..e3ca344f78f --- /dev/null +++ b/dev/ci/local-ci-config.py @@ -0,0 +1,261 @@ +# 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. + +# Everything dev/local-ci.sh needs to know about the CI configuration. +# +# The shell used to parse ci.yml and dev/ci/ with its own awk and sed, and +# check-ci-config.py parsed them again in Python so preflight could compare the +# two. That is worse than one parser twice over: it is two implementations to +# keep in step, and when they share a blind spot -- both stripped only single +# quotes, so `spark-full: "4.1.3"` came out with the quotes attached -- they +# agree with each other and the comparison says nothing. +# +# So there is one parser, here. The shell evals `--shell`, and preflight +# imports `config()` directly and checks the values rather than a second +# rendering of them. +# +# Usage: +# local-ci-config.py --shell spark 4.1 eval-able assignments for one job +# local-ci-config.py --print every value, for eyeballing + +import argparse +import json +import re +import shlex +import subprocess +import sys +from pathlib import Path + +CI_YML = Path(".github/workflows/ci.yml") +SPARK_YML = Path(".github/workflows/spark_sql_test_reusable.yml") +ICEBERG_YML = Path(".github/workflows/iceberg_spark_test_reusable.yml") +SHARDS_PY = Path("dev/ci/check-iceberg-shards.py") +MODULES_PY = Path("dev/ci/spark-sql-modules.py") +POLICY_PY = Path("dev/ci/compute-changes.py") + +# Values are versions or a JDK major. Anything else means a quoting or +# indentation change upstream has fooled the parse, and the shell would go on +# to build a URL or a Gradle task name out of it. +VERSION = re.compile(r"^\d+\.\d+$") +FULL_VERSION = re.compile(r"^\d+\.\d+\.\d+$") +MAJOR = re.compile(r"^\d+$") + + +class ConfigError(Exception): + pass + + +def _check(value, pattern, what): + if not pattern.match(value or ""): + raise ConfigError(f"{what} is {value!r}, which is not shaped like a version") + return value + + +def _job_inputs(): + """The `with:` inputs of every ci.yml job, keyed by job name.""" + jobs, job, in_with = {}, None, False + for line in CI_YML.read_text(encoding="utf-8").splitlines(): + header = re.match(r"^ ([A-Za-z0-9_-]+):\s*$", line) + if header: + job, in_with = header.group(1), False + elif job and re.match(r"^ with:\s*$", line): + in_with = True + elif in_with: + entry = re.match(r"^ ([a-z][a-z0-9-]*):\s*(\S.*?)\s*$", line) + if entry: + # YAML accepts either quote style. + jobs.setdefault(job, {})[entry.group(1)] = entry.group(2).strip("'\"") + elif re.match(r"^ [a-z]", line): + in_with = False + return jobs + + +def _load(name, path): + import importlib.util + + spec = importlib.util.spec_from_file_location(name, path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _queue_version(prefix, policy): + """The version the merge queue gates on: the newest Comet fully supports. + + Only purely version-shaped keys, since `spark_4_1_hive` is queue-tier too. + """ + found = [ + match.group(1).replace("_", ".") + for key, events in policy.items() + for match in [re.fullmatch(rf"{prefix}_(\d+(?:_\d+)*)", key)] + if match and "queue" in events + ] + if not found: + raise ConfigError(f"no queue-tier {prefix} version in {POLICY_PY}") + return max(found, key=lambda v: [int(part) for part in v.split(".")]) + + +def _dedicated_gate(): + """The one Spark version whose suites the workflow process-isolates. + + Returns (version, suites). Every other version legitimately gets nothing, + so the guard is on the line being present and parseable. + """ + text = SPARK_YML.read_text(encoding="utf-8") + match = re.search(r"DEDICATED_JVM_SBT_TESTS:.*?spark-short == '([^']*)' && '([^']*)'", text) + if not match: + raise ConfigError(f"no parseable DEDICATED_JVM_SBT_TESTS in {SPARK_YML}") + return _check(match.group(1), VERSION, "dedicated-jvm gate"), match.group(2) + + +def _iceberg_scala(): + """ci.yml leaves `scala` unset, so the reusable workflow default applies.""" + text = ICEBERG_YML.read_text(encoding="utf-8") + match = re.search(r"^ scala:.*?^ default:\s*'?([0-9.]+)'?", text, re.S | re.M) + if not match: + raise ConfigError(f"no scala default in {ICEBERG_YML}") + return _check(match.group(1), VERSION, "iceberg scala") + + +def _iceberg_shards(): + shards = _load("iceberg_shards", SHARDS_PY).SHARD_COUNT + if not isinstance(shards, int) or shards < 1: + raise ConfigError(f"SHARD_COUNT in {SHARDS_PY} is {shards!r}") + return shards + + +def spark_rows(selectors=()): + """The Spark SQL matrix rows a selector names, in workflow order.""" + modules = _load("spark_sql_modules", MODULES_PY) + rows = modules.select("all") + names = [row["name"] for row in rows] + picked = [] + for want in selectors or ["all"]: + if want in ("all", "core", "hive"): + picked += [r for r in rows if want == "all" or r["group"] == want] + elif want in names: + picked += [r for r in rows if r["name"] == want] + else: + raise ConfigError( + f"unknown module {want!r}; try: " + ", ".join(names + ["all", "core", "hive"]) + ) + # Overlapping selectors, `core sql_core-1` say, would otherwise name a row + # twice. Each row's tree and log are keyed on its name, so the duplicate + # would delete the tree out from under the copy already running in it. + seen = set() + return [r for r in picked if not (r["name"] in seen or seen.add(r["name"]))] + + +def config(): + """Every value the script reads, validated. Raises ConfigError on drift.""" + jobs = _job_inputs() + policy = _load("compute_changes", POLICY_PY).POLICY + gate_version, gate_suites = _dedicated_gate() + + spark, iceberg = {}, {} + for job, given in jobs.items(): + version = job.split("_", 1)[1].replace("_", ".") if "_" in job else job + if re.fullmatch(r"spark_\d+_\d+", job) and "spark-full" in given: + spark[_check(version, VERSION, job)] = { + "full": _check(given.get("spark-full"), FULL_VERSION, f"{job} spark-full"), + "java": _check(given.get("java"), MAJOR, f"{job} java"), + } + elif re.fullmatch(r"iceberg_\d+_\d+", job) and "iceberg-full" in given: + iceberg[_check(version, VERSION, job)] = { + "full": _check(given.get("iceberg-full"), FULL_VERSION, f"{job} iceberg-full"), + "spark": _check(given.get("spark-short"), VERSION, f"{job} spark-short"), + "java": _check(given.get("java"), MAJOR, f"{job} java"), + } + if not spark or not iceberg: + raise ConfigError(f"no spark_*/iceberg_* jobs found in {CI_YML}") + + return { + "spark": spark, + "iceberg": iceberg, + "spark_default": _queue_version("spark", policy), + "iceberg_default": _queue_version("iceberg", policy), + "iceberg_scala": _iceberg_scala(), + "iceberg_shards": _iceberg_shards(), + "dedicated_gate_version": gate_version, + "dedicated_gate_suites": gate_suites, + "rows": [row["name"] for row in spark_rows()], + } + + +def shell(suite, version, selectors): + """Eval-able assignments for one run of dev/local-ci.sh.""" + conf = config() + if version is None: + version = conf[f"{suite}_default"] + known = conf[suite] + if version not in known: + raise ConfigError( + f"unknown {suite} version {version!r}; try: " + ", ".join(sorted(known)) + ) + + out = {"VERSION": version, "DEFAULTED": "1" if version == conf[f"{suite}_default"] else ""} + if suite == "spark": + rows = spark_rows(selectors) + out["FULL"] = known[version]["full"] + out["JAVA"] = known[version]["java"] + # Unit separated per field, newline per row: a tab is IFS whitespace, so + # the shell's `read` would collapse the empty args1 of an sql_core row. + out["ROWS"] = "\n".join( + "\x1f".join([r["name"], r["args1"], r["args2"], r["heap"], r["metaspace"]]) + for r in rows + ) + out["PROJECTS"] = " ".join( + sorted({(r["args1"] or r["args2"]).split("/")[0] for r in rows}) + ) + out["DEDICATED"] = ( + conf["dedicated_gate_suites"] if version == conf["dedicated_gate_version"] else "" + ) + else: + out["FULL"] = known[version]["full"] + out["SPARK"] = known[version]["spark"] + out["JAVA"] = known[version]["java"] + out["SCALA"] = conf["iceberg_scala"] + out["SHARDS"] = str(conf["iceberg_shards"]) + return "\n".join(f"{k}={shlex.quote(v)}" for k, v in out.items()) + + +def main(argv): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--shell", action="store_true", help="emit eval-able assignments") + parser.add_argument("--print", dest="show", action="store_true", help="show every value") + parser.add_argument("suite", nargs="?", choices=("spark", "iceberg")) + parser.add_argument("version", nargs="?") + parser.add_argument("selectors", nargs="*") + args = parser.parse_args(argv) + try: + if args.shell: + if not args.suite: + parser.error("--shell needs a suite") + print(shell(args.suite, args.version or None, args.selectors)) + else: + print(json.dumps(config(), indent=2, sort_keys=True)) + except ConfigError as err: + print(f"local-ci config: {err}", file=sys.stderr) + return 1 + except subprocess.CalledProcessError as err: + print(f"local-ci config: {err}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/dev/local-ci.sh b/dev/local-ci.sh new file mode 100755 index 00000000000..8e3267302d9 --- /dev/null +++ b/dev/local-ci.sh @@ -0,0 +1,431 @@ +#!/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. +# +# Run the Spark SQL or Iceberg CI workflow locally. Mirrors +# .github/workflows/spark_sql_test_reusable.yml and +# .github/workflows/iceberg_spark_test_reusable.yml. +# +# Versions, matrix rows and the shard count are read from ci.yml and dev/ci/ at +# run time, so a version bump needs no change here. Written for bash 3.2. + +set -euo pipefail + +REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SHARDS_PY="$REPO/dev/ci/check-iceberg-shards.py" +# Rebuildable trees, so keep them out of $HOME. Override to survive a reboot. +SANDBOX="${COMET_LOCAL_CI_HOME:-/tmp/comet-local-ci}" +case "$(uname -s)" in + # bsdtar reads Spark's dot-prefixed .crc fixtures as AppleDouble metadata and + # exits nonzero after extracting them correctly. + Darwin) LIB=libcomet.dylib TAR_FLAGS=--no-mac-metadata ;; + *) LIB=libcomet.so TAR_FLAGS= ;; +esac + +# Yellow starting, green finished, red failed. +say() { printf '\n\033[1;33m[local-ci] %s\033[0m\n' "$*" >&2; } +ok() { printf '\n\033[1;32m[local-ci] %s\033[0m\n' "$*" >&2; } +fail() { printf '\n\033[1;31m[local-ci] %s\033[0m\n' "$*" >&2; } +die() { + fail "$*" + exit 1 +} +FAILED="" + +# Seconds as 1h 05m 12s. +hms() { + if [ "$1" -ge 3600 ]; then + printf '%dh %02dm %02ds' $(($1 / 3600)) $(($1 % 3600 / 60)) $(($1 % 60)) + elif [ "$1" -ge 60 ]; then + printf '%dm %02ds' $(($1 / 60)) $(($1 % 60)) + else + printf '%ds' "$1" + fi +} + +usage() { + cat >&2 <<'EOF' +Usage: dev/local-ci.sh [version] [target...] + + dev/local-ci.sh spark every Spark SQL matrix row + dev/local-ci.sh spark sql_core-1 one row, or all/core/hive + dev/local-ci.sh iceberg every Iceberg target + dev/local-ci.sh iceberg shard-2 one shard, or extensions/runtime + +The version defaults to the one the merge queue gates on. Name an older one to +reproduce a nightly failure: dev/local-ci.sh spark 3.5 sql_core-1 + + --print-config show what is read from ci.yml and dev/ci, then exit + + SKIP_PREPARE=1 run the tests only. Skips the Comet install too, so do + not use it after changing Comet + COMET_LOCAL_CI_HOME where the sources live (default /tmp/comet-local-ci) +EOF + exit 2 +} + +# Read every value from dev/ci/local-ci-config.py, the single parser. It +# validates shapes and fails loudly, so nothing below has to re-guard a parse. +# shellcheck disable=SC2153 # VERSION/FULL/JAVA/ROWS/... all come from here +load_config() { + # Assign first and check that, rather than `eval "$(...)"`: eval reports its + # own status, so a parser that failed and printed nothing would look like + # success and the run would exit 0 having tested nothing. + conf="$(cd "$REPO" && python3 dev/ci/local-ci-config.py --shell "$@")" || + die "could not read the CI configuration" + eval "$conf" + if [ -n "$DEFAULTED" ]; then + say "$1 $VERSION (the version the merge queue gates on)" + fi +} + +# --- sandbox --------------------------------------------------------------—-- + +setup_jdk() { + if [ -x /usr/libexec/java_home ]; then + JAVA_HOME="$(/usr/libexec/java_home -v "$1")" || die "JDK $1 not installed" + export JAVA_HOME + fi + [ -n "${JAVA_HOME:-}" ] || die "export JAVA_HOME pointing at a JDK $1" + say "JDK $1: $JAVA_HOME" +} + +# The ci profile is release without LTO. Stage it where -Prelease looks. +build_native() { + say "cargo build --profile ci" + (cd "$REPO/native" && cargo build --profile ci) + mkdir -p "$REPO/native/target/release" + cp "$REPO/native/target/ci/$LIB" "$REPO/native/target/release/$LIB" +} + +# Spark's build reaches for git only through build/spark-build-info, which has no +# `set -e`, so the tag archive works and skips the git objects a clone carries. +# Extract beside the target and move, so a failed download leaves nothing that +# the next run mistakes for a complete tree. +fetch_archive() { + if [ -d "$2" ]; then return 0; fi + say "downloading $1" + rm -rf "$2.part" + mkdir -p "$2.part" + # Checked, not left to `set -e`, which bash disables inside a function called + # from a conditional. + # shellcheck disable=SC2086 + if ! curl -fsSL "$1" | tar -xz $TAR_FLAGS -C "$2.part" --strip-components=1; then + rm -rf "$2.part" + die "could not download $1" + fi + mv "$2.part" "$2" +} + +# Iceberg does need a repository: its build takes the project version from the +# latest apache-iceberg-* tag via com.palantir.git-version. +clone_tag() { + if [ -d "$3/.git" ]; then return 0; fi + say "cloning $1 at $2" + git clone --depth 1 --branch "$2" "$1" "$3" +} + +# `git apply` works outside a repository, so this covers both trees. The applied +# diff is recorded so that editing dev/diffs/ and re-running reverts the old one +# first; otherwise the tree wedges and only deleting it helps. +apply_diff() { + marker="$1/.local-ci-applied.diff" + if [ -f "$marker" ] && cmp -s "$marker" "$2"; then return 0; fi + # A tree patched before the record existed: adopt it rather than fail. + if [ ! -f "$marker" ] && (cd "$1" && git apply --check --reverse "$2") 2>/dev/null; then + cp "$2" "$marker" + return 0 + fi + if [ -f "$marker" ]; then + say "reverting the previously applied diff" + (cd "$1" && git apply -R "$marker") || die "cannot revert $marker. Delete $1 and re-run." + rm -f "$marker" + fi + say "applying $(basename "$2")" + (cd "$1" && git apply "$2") || die "$(basename "$2") does not apply. Delete $1 and re-run." + cp "$2" "$marker" +} + +install_comet() { + say "mvnw install -Prelease -DskipTests $*" + (cd "$REPO" && ./mvnw -B install -Prelease -DskipTests "$@") +} + +# Ask Maven rather than assuming ~/.m2/repository: settings.xml and +# -Dmaven.repo.local can relocate it, and guessing wrong makes the purges below +# silent no-ops for the people who need them most. +MAVEN_REPO="" +maven_repo() { + if [ -z "$MAVEN_REPO" ]; then + MAVEN_REPO="$(cd "$REPO" && ./mvnw -q -N help:evaluate \ + -Dexpression=settings.localRepository -DforceStdout 2>/dev/null | tail -1)" + case "$MAVEN_REPO" in /*) ;; *) die "could not resolve the local Maven repository" ;; esac + fi + printf '%s\n' "$MAVEN_REPO" +} + +# Comet's install leaves POMs whose JARs it never fetched. Coursier then calls +# the artifact found-locally and refuses to fall back to Maven Central, so sbt +# dies on a JAR it can see a POM for. Both workflows drop the Parquet tree for +# that reason; the wider sweep is the same problem one level out. +purge_parquet() { + dir="$(maven_repo)/org/apache/parquet" + [ -d "$dir" ] || return 0 + say "removing $dir so sbt and gradle refetch it" + rm -rf "$dir" +} + +# setup-spark-builder greps for an explicit jar|bundle and +# so misses a POM declaring none, which Maven defaults to jar. org.antlr:antlr4 +# is one of those. A pom parent has no JAR by design. +purge_partial_poms() { + repo="$(maven_repo)" + [ -d "$repo" ] || return 0 + say "dropping POM-only entries across all of $repo" + find "$repo" -name '*.pom' | while read -r pom; do + [ -f "${pom%.pom}.jar" ] && continue + packaging="$(sed -n 's:.*\(.*\).*:\1:p' "$pom" | head -1)" + case "${packaging:-jar}" in jar | bundle) ;; *) continue ;; esac + rm -f "$pom" "$pom.sha1" "${pom%.pom}.pom.lastUpdated" \ + "$(dirname "$pom")/_remote.repositories" + done +} + +# Purging invalidates what sbt already resolved: plugins such as sbt-antlr4 build +# their classpath from the cached update report, not the filesystem, so a +# refetched artifact stays invisible until the report is rebuilt. +drop_cached_resolution() { + [ -d "$1" ] || return 0 + say "clearing cached sbt resolution so it re-reads the Maven repository" + find "$1" -type d -name update -path '*/target/*' -prune -exec rm -rf {} + +} + +# Copy a prepared tree so a shard can have one to itself, the way each CI +# matrix row gets its own runner and its own extracted apache-spark/. On APFS +# and btrfs this is copy-on-write, so a 4 GB tree costs kilobytes until the +# shards start writing their own reports. +clone_tree() { + rm -rf "$2" + cp -Rc "$1" "$2" 2>/dev/null || + cp -R --reflink=auto "$1" "$2" 2>/dev/null || + cp -R "$1" "$2" +} + +# --- runners ----------------------------------------------------------------- + +row_label() { printf 'spark-sql-%s / spark-%s-jdk%s\n' "$1" "$FULL" "$JAVA"; } + +# spark_row +# One row, in the tree it is given. A subshell so HEAP_SIZE cannot leak. +spark_row() { + ( + cd "$6" + printf -- '-J-Xms1g\n-J-Xmx4g\n-J-XX:MaxMetaspaceSize=1g\n' > .sbtopts + export LC_ALL=C.UTF-8 NOLINT_ON_COMPILE=true + # shellcheck disable=SC2030 + export ENABLE_COMET=true ENABLE_COMET_ONHEAP=true + export SBT_OPTS="-Xss4m -XX:+UseG1GC -XX:+UseStringDeduplication -XX:MaxMetaspaceSize=384m -XX:G1HeapRegionSize=2m -XX:InitiatingHeapOccupancyPercent=35 -XX:+ParallelRefProcEnabled -XX:+ExitOnOutOfMemoryError" + [ -n "$4" ] && export HEAP_SIZE="$4" + [ -n "$5" ] && export METASPACE_SIZE="$5" + # What the workflow exports. SERIAL_SBT_TESTS suppresses Spark's own test + # grouping and parallelExecution; the cap keeps one forked test JVM. Rows run + # beside each other in their own trees instead, the way CI does it. + export SERIAL_SBT_TESTS=1 + set -- -Dsbt.log.noformat=true -mem 1024 \ + "set Global / concurrentRestrictions := Seq(Tags.limit(Tags.ForkedTestGroup, 1))" \ + ${2:+"$2"} ${3:+"$3"} + build/sbt "$@" + ) +} + +# 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/ +# 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() { + logs="$SANDBOX/logs-spark-$FULL" + mkdir -p "$logs" + # Copy every tree before starting anything. Interleaving the copies with the + # launches means a copy that fails -- a full disk is the likely way -- exits + # on set -e with earlier rows still running and unawaited. + while IFS=$'\037' read -r name args1 args2 heap metaspace; do + [ -n "$name" ] || continue + clone_tree "$dest" "$dest-$name" + done <<< "$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 + # gets its own log. + spark_row "$name" "$args1" "$args2" "$heap" "$metaspace" "$dest-$name" \ + > "$logs/$name.log" 2>&1 & + say "started spark-sql-$name, log $logs/$name.log" + running="$running$name $SECONDS $! " + done <<< "$ROWS" + await_rows "$running" + [ -z "$FAILED" ] || die "failed:$FAILED" +} + +# await_rows " ..." - one line per row as it finishes. +await_rows() { + # shellcheck disable=SC2086 # word splitting is the point: name start pid ... + set -- $1 + while [ $# -ge 3 ]; do + if wait "$3"; then + ok "spark-sql-$1 passed in $(hms $((SECONDS - $2)))" + else + FAILED="$FAILED $1" + fail "spark-sql-$1 failed in $(hms $((SECONDS - $2))), last 20 lines of $logs/$1.log:" + tail -20 "$logs/$1.log" >&2 + fi + shift 3 + done +} + +run_spark() { + load_config spark "$@" + dest="$SANDBOX/apache-spark-$FULL" + setup_jdk "$JAVA" + + if [ -z "${SKIP_PREPARE:-}" ]; then + prep=$SECONDS + build_native + fetch_archive "https://github.com/apache/spark/archive/refs/tags/v$FULL.tar.gz" "$dest" + apply_diff "$dest" "$REPO/dev/diffs/$FULL.diff" + install_comet "-Pspark-$VERSION" + purge_parquet + 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. + compile="" + for p in $PROJECTS; do compile="$compile $p/Test/compile"; done + say "pre-compiling test classes:$compile" + # shellcheck disable=SC2086 + (cd "$dest" && NOLINT_ON_COMPILE=true build/sbt -Dsbt.log.noformat=true -mem 3072 $compile) + ok "prepare took $(hms $((SECONDS - prep)))" + fi + + [ -n "$DEDICATED" ] && export DEDICATED_JVM_SBT_TESTS="$DEDICATED" + + # A single row runs in the prepared tree; there is nothing to run beside it. + if [ "$(printf '%s\n' "$ROWS" | grep -c .)" -eq 1 ]; then + IFS=$'\037' read -r name args1 args2 heap metaspace <<< "$ROWS" + started=$SECONDS + say "started $(row_label "$name")" + spark_row "$name" "$args1" "$args2" "$heap" "$metaspace" "$dest" + ok "spark-sql-$name took $(hms $((SECONDS - started)))" + else + run_spark_rows + fi +} + +run_iceberg() { + load_config iceberg "$1" + shift + dest="$SANDBOX/apache-iceberg-$FULL" + + if [ $# -eq 0 ]; then + i=1 + while [ "$i" -le "$SHARDS" ]; do + set -- "$@" "shard-$i" + i=$((i + 1)) + done + set -- "$@" extensions runtime + fi + # Vet the whole list before anything is built, so a typo costs nothing. + for target in "$@"; do + case "$target" in + extensions | runtime) ;; + shard-[1-9] | shard-[1-9][0-9]) + [ "${target#shard-}" -le "$SHARDS" ] || die "no $target; there are $SHARDS shards" + ;; + *) die "unknown target '$target'; try shard-1..$SHARDS, extensions, runtime" ;; + esac + done + + setup_jdk "$JAVA" + if [ -z "${SKIP_PREPARE:-}" ]; then + prep=$SECONDS + build_native + clone_tag https://github.com/apache/iceberg.git "apache-iceberg-$FULL" "$dest" + apply_diff "$dest" "$REPO/dev/diffs/iceberg/$FULL.diff" + install_comet "-Pspark-$SPARK" "-Pscala-$SCALA" + purge_parquet + purge_partial_poms + ok "prepare took $(hms $((SECONDS - prep)))" + fi + + core=":iceberg-spark:iceberg-spark-${SPARK}_${SCALA}:test" + for target in "$@"; do + say "iceberg-$FULL / spark-$SPARK / $target" + started=$SECONDS + case "$target" in + shard-*) + gradlew "$core" --init-script "$REPO/dev/ci/iceberg-test-shards.gradle" \ + "-PcometShardTask=$core" "-PcometShardIndex=${target#shard-}" \ + "-PcometShardCount=$SHARDS" + ;; + extensions) gradlew ":iceberg-spark:iceberg-spark-extensions-${SPARK}_${SCALA}:test" ;; + runtime) + # The workflow runs the sharding fixture in this job before the test. + python3 "$SHARDS_PY" --gradle "$dest/gradlew" + gradlew ":iceberg-spark:iceberg-spark-runtime-${SPARK}_${SCALA}:integrationTest" + ;; + esac + ok "$target took $(hms $((SECONDS - started)))" + done +} + +# Reads $dest, $spark and $SCALA from run_iceberg. +gradlew() { + ( + cd "$dest" + # shellcheck disable=SC2031 + export SPARK_LOCAL_IP=localhost ENABLE_COMET=true ENABLE_COMET_ONHEAP=true + ./gradlew "-DsparkVersions=$SPARK" "-DscalaVersion=$SCALA" \ + -DflinkVersions= -DkafkaVersions= "$@" -Pquick=true -x javadoc + ) +} + +# --- dispatch ---------------------------------------------------------------- + +[ $# -ge 1 ] || usage +if [ "$1" = "--print-config" ]; then + (cd "$REPO" && python3 dev/ci/local-ci-config.py --print) + exit 0 +fi +what="$1" +shift +case "$what" in spark | iceberg) ;; *) usage ;; esac + +# A version is the only argument shaped like N.N; anything else is a target. +# load_config defaults it and reports back through VERSION and DEFAULTED. +case "${1:-}" in + [0-9]*.[0-9]*) ;; + *) set -- "" "$@" ;; +esac + +# However this exits, say how long it took. Capture and re-raise the status +# first: the trap's own last command would otherwise become the exit status and +# turn every failure into a green run. +# shellcheck disable=SC2154 # assigned in the trap body, which shellcheck cannot see +trap 'status=$?; ok "total runtime $(hms $SECONDS)"; exit $status' EXIT + +"run_$what" "$@" From ed904dc4d2b9207f602afc36ad594d657a672324 Mon Sep 17 00:00:00 2001 From: comphead Date: Thu, 17 Sep 2026 17:29:23 -0700 Subject: [PATCH 2/2] docs: document running the CI suites locally Rewrite "Reproducing a suite failure locally" in the CI guide around dev/local-ci.sh: the commands, SKIP_PREPARE and what it skips, how row-level concurrency works, and the caveats that matter. The sandbox lives under /tmp so a reboot costs a recompile, preparing sweeps the whole shared Maven repository rather than just Comet and Spark artifacts, running every row at once wants a full tree each, and CI is x86_64 so a pass elsewhere does not cover x86-specific native codegen. Point the Spark SQL and Iceberg test guides at it, keeping their manual steps as the reference since those are also the diff-regeneration workflow. AGENTS.md gains "Checking a change against CI": a green pull request only covers the PR tier, so a change touching the serde, the planner, a native operator, a Spark shim, an Iceberg path or dev/diffs/ needs either a local run or the matching run-* label before it is queued. It names the cost of each so the choice is deliberate, and flags the two things that matter when an agent runs it unattended. --- AGENTS.md | 24 ++++++++ docs/source/contributor-guide/ci.md | 56 +++++++++++++++++-- .../contributor-guide/iceberg-spark-tests.md | 11 ++++ .../contributor-guide/spark-sql-tests.md | 11 ++++ 4 files changed, 97 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2dabec82d0b..741378286e1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,6 +34,30 @@ Relevant entry points: When opening a pull request, use the [PR template](.github/pull_request_template.md) and fill in every section. +## Checking a change against CI + +A green pull request does not mean a change is safe to queue. The pull request tier runs the Comet +suites against the default Spark profile only. Spark's own SQL suite and the Iceberg suites first +report in the merge queue, where a failure evicts the pull request and blocks everyone else's +merges, or in the nightly run, after the change has already landed. + +So when a change touches the serde, the planner, a native operator, a Spark shim, an Iceberg code +path, or anything under `dev/diffs/`, get a verdict first. Either run the suite locally: + +```shell +dev/local-ci.sh spark sql_core-1 # one matrix row, or `spark` for all of them +dev/local-ci.sh iceberg shard-2 # one shard, or `iceberg` for every target +``` + +or apply the matching `run-*` label so CI runs it instead. Which changes warrant which suite, the +label names, and the script's caveats are in +[Continuous Integration](docs/source/contributor-guide/ci.md). + +Pick one rather than both. A full local job is hours of compute and tens of GB of disk, so prefer +the shard that covers the change, and prefer the label when the change is broad. Two things matter +when running it unattended: preparing sweeps the whole local Maven repository, which is a shared +cache, and `SKIP_PREPARE=1` skips the Comet install so it must not be used after changing Comet. + ## Skills Repository-specific agent skills live under `.ai/skills/`. Each subdirectory is a single skill diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index 20f2eedf808..3f1e09f36f5 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -246,11 +246,57 @@ nightly suites, if you need a result before the next scheduled run. ## Reproducing a suite failure locally -The Spark SQL suites outside the PR tier run Spark's own test suite against Comet, with the -version's diff from `dev/diffs/` applied. See [Spark SQL Tests](spark-sql-tests.md) for how to run -one locally, and [Iceberg Spark Tests](iceberg-spark-tests.md) for the Iceberg equivalents. For the -Comet test suites that run on macOS, `make test-jvm` on a Mac runs the same suites the workflow -does; the macOS job differs from Linux only in the platform. +`dev/local-ci.sh` builds the same sandbox a runner builds and runs the Spark SQL or Iceberg +workflow: + +```sh +dev/local-ci.sh spark # everything the Spark job runs +dev/local-ci.sh spark sql_core-1 # just the shard that failed +dev/local-ci.sh iceberg # everything the Iceberg job runs +dev/local-ci.sh iceberg shard-2 +dev/local-ci.sh spark 3.5 sql_core-1 # a nightly-tier version, named explicitly +dev/local-ci.sh --print-config # what it read from ci.yml and dev/ci +``` + +The version defaults to the one the merge queue gates on. That, the matrix rows and the shard count +are read from the workflow files and from `dev/ci/`, so a local shard runs what the CI shard of the +same name runs. It prepares first (native build, Comet install, patched Spark or Iceberg source +under `$COMET_LOCAL_CI_HOME`, default `/tmp/comet-local-ci`), then runs the tests, compiling only +the sbt projects the selected rows need. + +`SKIP_PREPARE=1` runs the tests only. It skips the **Comet install** too, so do not use it after +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. +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 +`$COMET_LOCAL_CI_HOME/logs-spark-/.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 +reports stay readable. A single row runs in the prepared tree with no copy. Iceberg targets still run +one after another. + +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 + 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, + so other projects will re-download. `--print-config` touches nothing. +- CI is x86_64 Linux built with `-Ctarget-cpu=x86-64-v3`, so a pass elsewhere covers Scala, serde + and planner behavior but not x86-specific native codegen. + +The underlying steps are documented in [Spark SQL Tests](spark-sql-tests.md) and +[Iceberg Spark Tests](iceberg-spark-tests.md), which are also where the diff-regeneration workflow +lives. For the Comet test suites that run on macOS, `make test-jvm` on a Mac runs the same suites +the workflow does. The macOS job differs from Linux only in the platform. ## Changing CI itself diff --git a/docs/source/contributor-guide/iceberg-spark-tests.md b/docs/source/contributor-guide/iceberg-spark-tests.md index 09374a041fa..75ad2f3a1ac 100644 --- a/docs/source/contributor-guide/iceberg-spark-tests.md +++ b/docs/source/contributor-guide/iceberg-spark-tests.md @@ -40,6 +40,17 @@ Here is an overview of the changes that the diffs make to Iceberg: [#5259]: https://github.com/apache/datafusion-comet/issues/5259 [apache/iceberg#15674]: https://github.com/apache/iceberg/pull/15674 +`dev/local-ci.sh` runs all of the steps below the way CI runs them: + +```shell +dev/local-ci.sh iceberg # every target the workflow runs +dev/local-ci.sh iceberg shard-2 # one shard of the core test job +dev/local-ci.sh iceberg 1.9 shard-2 # a non-default Iceberg version +``` + +See [Continuous Integration](ci.md#reproducing-a-suite-failure-locally). The manual steps below +are still the reference, and are what you want when updating a diff. + ## 1. Install Comet Run `make release` in Comet to install the Comet JAR into the local Maven repository, specifying the Spark version. diff --git a/docs/source/contributor-guide/spark-sql-tests.md b/docs/source/contributor-guide/spark-sql-tests.md index acfb8fcedc7..34e09836263 100644 --- a/docs/source/contributor-guide/spark-sql-tests.md +++ b/docs/source/contributor-guide/spark-sql-tests.md @@ -32,6 +32,17 @@ Here is an overview of the changes that we need to make to Spark: Here are the steps involved in running the Spark SQL tests with Comet, using Spark 3.4.3 for this example. +`dev/local-ci.sh` runs all of the steps below the way CI runs them: + +```shell +dev/local-ci.sh spark # every matrix row +dev/local-ci.sh spark catalyst # one row +dev/local-ci.sh spark 3.4 catalyst # a non-default Spark version +``` + +See [Continuous Integration](ci.md#reproducing-a-suite-failure-locally). The manual steps below +are still the reference, and are what you want when creating or updating a diff file. + ## 1. Install Comet Run `make release` in Comet to install the Comet JAR into the local Maven repository, specifying the Spark version.