Simplify benchmark deployments and improve result reporting

benchmark-bot/src/executor.ts

@@ -12,6 +12,8 @@ interface FoundationOutputs {
 }
 
 const DEPLOYMENT_NAME = "datafusion-benchmark-bot";
+export const BENCHMARK_ITERATIONS = 5;
+export const BENCHMARK_WARMUP = true;
 
 export interface ExecutorConfig {
   repositoryUrl: string;
@@ -323,6 +325,10 @@ export class BenchmarkExecutor {
         `s3://${outputs.datasetBucketName}`,
         "--k8s-cluster",
         outputs.clusterName,
+        "--iterations",
+        String(BENCHMARK_ITERATIONS),
+        "--warmup",
+        String(BENCHMARK_WARMUP),
         "--result-name",
         resultName,
         "--kubeconfig",
@@ -369,10 +375,6 @@ export class BenchmarkExecutor {
       CARGO_TARGET_DIR: "/var/cache/datafusion-pr-build/target",
       DATAFUSION_BUILD_WRAPPER: "/usr/local/sbin/datafusion-pr-build",
       DEPLOYMENT_NAME,
-      K8S_RUNTIME_FILE: path.join(
-        path.dirname(this.config.foundationOutputsFile),
-        "benchmark-runtime.json",
-      ),
       KUBECONFIG: this.config.kubeconfig,
       PULUMI_OUTPUTS_FILE: this.config.foundationOutputsFile,
       WORKER_ARTIFACT_BUCKET: outputs.artifactBucketName,

benchmark-bot/src/render.ts

@@ -1,8 +1,10 @@
 import type { Job } from "./database.js";
-import type {
-  BenchmarkTiming,
-  ExecutionProgress,
-  ExecutionTimings,
+import {
+  BENCHMARK_ITERATIONS,
+  BENCHMARK_WARMUP,
+  type BenchmarkTiming,
+  type ExecutionProgress,
+  type ExecutionTimings,
 } from "./executor.js";
 
 const GITHUB_COMMENT_LIMIT = 65_536;
@@ -43,31 +45,62 @@ export function renderResult(
     Date.parse(job.updatedAt) - Date.parse(job.createdAt),
   );
 
-  const metadata = `${requestLink(job)}
+  const header = `${requestLink(job)}
+
+## Benchmark results
+
+**Compared:** ${revisionLink(job, "Base", job.baseSha)} → ${revisionLink(job, "PR head", job.headSha)} · [View exact source diff](${compareUrl(job)})
+
+`;
+  const details = `
 
 <details>
-<summary>Run metadata</summary>
+<summary>Verification and run details</summary>
+
+Job \`${job.id}\` captured both immutable revisions when the request was queued. The bot fetched and checked out each full commit SHA in detached HEAD, then built and deployed the \`datafusion-distributed-benchmarks --bin worker\` target from that checkout.
+
+| Identity | Base | PR head |
+| --- | --- | --- |
+| Source commit | ${fullRevisionLink(job, job.baseSha)} | ${fullRevisionLink(job, job.headSha)} |
 
 | Phase | Base | PR head |
 | --- | ---: | ---: |
 | Build and deployment | ${formatDuration(timings.baseDeployMs)} | ${formatDuration(timings.headDeployMs)} |
 | All benchmarks | ${formatDuration(baseBenchmarkMs)} | ${formatDuration(headBenchmarkMs)} |
 ${benchmarkRows}
 
-Queue: ${formatDuration(queueMs)} · Dataset validation: ${formatDuration(timings.validationMs)} · Total: ${formatDuration(timings.totalMs)}
+**Workload:** ${formatDatasets(job.datasets)} · all queries · ${BENCHMARK_WARMUP ? "1 warmup + " : ""}${BENCHMARK_ITERATIONS} measured iterations per query
 
-Capacity: ${capacity(job)}
+**Capacity:** ${capacity(job)} for both revisions
 
-</details>
+**Other timings:** Queue ${formatDuration(queueMs)} · Dataset validation ${formatDuration(timings.validationMs)} · Total ${formatDuration(timings.totalMs)}
 
-`;
-  if (metadata.length >= GITHUB_COMMENT_LIMIT) {
-    return truncate(metadata, GITHUB_COMMENT_LIMIT);
+</details>`;
+  const fixedLength = header.length + details.length;
+  if (fixedLength >= GITHUB_COMMENT_LIMIT) {
+    return truncate(`${header}${details}`, GITHUB_COMMENT_LIMIT);
   }
-  return `${metadata}${renderComparison(
+  const renderedComparison = renderComparison(
     comparison,
-    GITHUB_COMMENT_LIMIT - metadata.length,
-  )}`;
+    GITHUB_COMMENT_LIMIT - fixedLength,
+  );
+  return `${header}${renderedComparison}${details}`;
+}
+
+function revisionLink(job: Job, label: string, sha: string): string {
+  return `[${label} \`${sha.slice(0, 12)}\`](${commitUrl(job, sha)})`;
+}
+
+function fullRevisionLink(job: Job, sha: string): string {
+  return `[\`${sha}\`](${commitUrl(job, sha)})`;
+}
+
+function commitUrl(job: Job, sha: string): string {
+  return `https://github.com/${job.repository}/commit/${sha}`;
+}
+
+function compareUrl(job: Job): string {
+  return `https://github.com/${job.repository}/compare/${job.baseSha}...${job.headSha}`;
 }
 
 function renderComparison(comparison: string, maxLength: number): string {

benchmark-bot/src/worker.ts

@@ -1,7 +1,7 @@
 import type { Job, JobDatabase } from "./database.js";
 import type {
+  ExecutionResult,
   ExecutionProgress,
-  ExecutionTimings,
   ProgressReporter,
 } from "./executor.js";
 import type { GitHubApi } from "./github.js";
@@ -13,13 +13,7 @@ import {
 } from "./render.js";
 
 export interface JobExecutor {
-  execute(
-    job: Job,
-    onProgress?: ProgressReporter,
-  ): Promise<{
-    comparison: string;
-    timings: ExecutionTimings;
-  }>;
+  execute(job: Job, onProgress?: ProgressReporter): Promise<ExecutionResult>;
 }
 
 export class JobWorker {

benchmark-bot/test/executor.test.ts

@@ -300,6 +300,8 @@ test("runs benchmarks against the shared deployment and adjacent testdata", asyn
   const argument = (name: string): string | undefined =>
     arguments_[arguments_.indexOf(name) + 1];
   assert.equal(argument("--k8s-service"), "datafusion-benchmark-bot");
+  assert.equal(argument("--iterations"), "5");
+  assert.equal(argument("--warmup"), "true");
   assert.equal(
     argument("--testdata-root"),
     path.join(config.sourceRoot, "testdata"),

benchmark-bot/test/worker.test.ts

@@ -80,13 +80,19 @@ test("reports a completed comparison and consumes the job", async () => {
     assert.equal(comments[2]!.match(/TOTAL:/g)?.length, 1);
     assert.match(comments[2]!, /q1: prev= 100 ms/);
     assert.match(comments[2]!, /pull\/99#issuecomment-7/);
-    assert.match(comments[2]!, /Run metadata/);
+    assert.match(comments[2]!, /Benchmark results/);
+    assert.match(comments[2]!, /Base `aaaaaaaaaaaa`.*PR head `bbbbbbbbbbbb`/s);
+    assert.match(comments[2]!, /compare\/a{40}\.\.\.b{40}/);
+    assert.match(comments[2]!, /Verification and run details/);
+    assert.match(comments[2]!, /detached HEAD/);
+    assert.match(
+      comments[2]!,
+      /datafusion-distributed-benchmarks --bin worker/,
+    );
+    assert.match(comments[2]!, /1 warmup \+ 5 measured iterations per query/);
     assert.match(comments[2]!, /Build and deployment \| 2m 2s \| 2m 5s/);
     assert.match(comments[2]!, /Benchmark `tpch\/sf100` \| 30s \| 31s/);
-    assert.match(comments[2]!, /Total: 7m 0s/);
-    for (const comment of comments) {
-      assert.doesNotMatch(comment, /a{12}|b{12}/);
-    }
+    assert.match(comments[2]!, /Total 7m 0s/);
     assert.deepEqual(commentIds, [77, 77, 77]);
   } finally {
     database.close();

benchmarks-remote/.gitignore

@@ -1,4 +1,3 @@
 node_modules/
 engines/ballista/target/
 k8s/.kubeconfig
-k8s/.runtime.json

benchmarks-remote/k8s/deploy-engine.sh

@@ -19,8 +19,7 @@ manifest_values=()
 case ${engine} in
   datafusion)
     benchmark_instance_type=${BENCHMARK_INSTANCE_TYPE:-$(jq -er '.benchmarkInstanceType' "${outputs_file}")}
-    bash "${root}/benchmarks-remote/k8s/publish-datafusion.sh"
-    worker_artifact=${WORKER_ARTIFACT:-$(output_value '.workerArtifact' "${runtime_file}")}
+    worker_artifact=${WORKER_ARTIFACT:-$(bash "${root}/benchmarks-remote/k8s/publish-datafusion.sh")}
     : "${worker_artifact:?DataFusion worker publishing did not produce an artifact}"
     manifest_values+=(--set-string worker.artifact="${worker_artifact}")
     manifest_values+=(--set-string worker.datasetBucket="${dataset_bucket}")
@@ -40,8 +39,7 @@ case ${engine} in
   spark)
     benchmark_instance_type=$(jq -er '.benchmarkInstanceType' "${outputs_file}")
     coordinator_instance_type=$(jq -er '.coordinatorInstanceType' "${outputs_file}")
-    bash "${root}/benchmarks-remote/k8s/publish-image.sh" spark
-    spark_image=${SPARK_IMAGE:-$(output_value '.images.spark' "${runtime_file}")}
+    spark_image=${SPARK_IMAGE:-$(bash "${root}/benchmarks-remote/k8s/publish-image.sh" spark)}
     : "${spark_image:?Spark publishing did not produce an image}"
     manifest_values+=(--set-string image="${spark_image}")
     manifest_values+=(--set-string workerReplicas="${node_count}")
@@ -51,10 +49,10 @@ case ${engine} in
   ballista)
     benchmark_instance_type=$(jq -er '.benchmarkInstanceType' "${outputs_file}")
     coordinator_instance_type=$(jq -er '.coordinatorInstanceType' "${outputs_file}")
-    bash "${root}/benchmarks-remote/k8s/publish-ballista.sh"
-    scheduler_artifact=$(output_value '.ballistaArtifacts["ballista-scheduler"]' "${runtime_file}")
-    executor_artifact=$(output_value '.ballistaArtifacts["ballista-executor"]' "${runtime_file}")
-    http_artifact=$(output_value '.ballistaArtifacts["ballista-http"]' "${runtime_file}")
+    ballista_artifacts=$(bash "${root}/benchmarks-remote/k8s/publish-ballista.sh")
+    scheduler_artifact=$(jq -er '.["ballista-scheduler"]' <<<"${ballista_artifacts}")
+    executor_artifact=$(jq -er '.["ballista-executor"]' <<<"${ballista_artifacts}")
+    http_artifact=$(jq -er '.["ballista-http"]' <<<"${ballista_artifacts}")
     : "${scheduler_artifact:?Ballista publishing did not produce the scheduler artifact}"
     : "${executor_artifact:?Ballista publishing did not produce the executor artifact}"
     : "${http_artifact:?Ballista publishing did not produce the HTTP artifact}"

benchmarks-remote/k8s/lib.sh

@@ -6,7 +6,6 @@ init_environment() {
   root=$(cd "${k8s_dir}/../.." && pwd)
   region=${AWS_REGION:-us-east-1}
   outputs_file=${PULUMI_OUTPUTS_FILE:-${root}/benchmarks-remote/pulumi/.pulumi-outputs.json}
-  runtime_file=${K8S_RUNTIME_FILE:-${root}/benchmarks-remote/k8s/.runtime.json}
   export KUBECONFIG=${KUBECONFIG:-${k8s_dir}/.kubeconfig}
 
   if [[ ! -f ${outputs_file} ]]; then
@@ -45,32 +44,6 @@ benchmark_worker_selector() {
   esac
 }
 
-output_value() {
-  local expression=$1
-  local file=$2
-  if [[ -f ${file} ]]; then
-    jq -r "${expression} // empty" "${file}"
-  fi
-}
-
-update_runtime_file() {
-  local expression=${1:?usage: update_runtime_file JQ_EXPRESSION [JQ_ARGUMENTS...]}
-  shift
-  local current='{}'
-  if [[ -f ${runtime_file} ]]; then
-    current=$(<"${runtime_file}")
-  fi
-
-  (
-    local runtime_tmp
-    runtime_tmp=$(mktemp "${runtime_file}.XXXXXX")
-    trap 'rm -f "${runtime_tmp}"' EXIT INT TERM HUP
-    jq "$@" "${expression}" <<<"${current}" >"${runtime_tmp}"
-    mv "${runtime_tmp}" "${runtime_file}"
-    trap - EXIT INT TERM HUP
-  )
-}
-
 aws_cli() {
   AWS_PAGER='' aws --region "${region}" "$@"
 }

benchmarks-remote/k8s/publish-ballista.sh

@@ -13,17 +13,17 @@ CARGO_HOME=${CARGO_HOME:-${TMPDIR:-/tmp}/datafusion-distributed-ballista-cargo-h
   cargo zigbuild \
   --manifest-path "${root}/benchmarks-remote/engines/ballista/Cargo.toml" \
   --release \
-  --target x86_64-unknown-linux-gnu
+  --target x86_64-unknown-linux-gnu >&2
 target="${root}/benchmarks-remote/engines/ballista/target/x86_64-unknown-linux-gnu/release"
 artifacts='{}'
 for binary in ballista-scheduler ballista-executor ballista-http; do
   sha=$(shasum -a 256 "${target}/${binary}" | awk '{print $1}')
   key=".benchmark-artifacts/ballista/${sha}/${binary}"
   if ! aws_cli s3api head-object --bucket "${dataset_bucket}" --key "${key}" >/dev/null 2>&1; then
-    aws_cli s3 cp "${target}/${binary}" "s3://${dataset_bucket}/${key}"
+    aws_cli s3 cp "${target}/${binary}" "s3://${dataset_bucket}/${key}" >&2
   fi
   artifacts=$(jq --arg binary "${binary}" --arg uri "s3://${dataset_bucket}/${key}" \
     '.[$binary] = $uri' <<<"${artifacts}")
 done
-update_runtime_file '.ballistaArtifacts = $artifacts' --argjson artifacts "${artifacts}"
-echo "Published Ballista binaries"
+echo "Published Ballista binaries" >&2
+jq -c . <<<"${artifacts}"

benchmarks-remote/k8s/publish-datafusion.sh

@@ -11,7 +11,7 @@ artifact_prefix=${WORKER_ARTIFACT_PREFIX:-.benchmark-artifacts/datafusion}
 source_root=${DATAFUSION_SOURCE_ROOT:-${root}/../datafusion-distributed}
 
 if [[ -n ${DATAFUSION_BUILD_WRAPPER:-} ]]; then
-  sudo "${DATAFUSION_BUILD_WRAPPER}"
+  sudo "${DATAFUSION_BUILD_WRAPPER}" >&2
 else
   ZIG_GLOBAL_CACHE_DIR=${ZIG_GLOBAL_CACHE_DIR:-${TMPDIR:-/tmp}/datafusion-distributed-zig-global} \
   ZIG_LOCAL_CACHE_DIR=${ZIG_LOCAL_CACHE_DIR:-${TMPDIR:-/tmp}/datafusion-distributed-zig-local} \
@@ -20,7 +20,7 @@ else
     --package datafusion-distributed-benchmarks \
     --release \
     --bin worker \
-    --target x86_64-unknown-linux-gnu
+    --target x86_64-unknown-linux-gnu >&2
 fi
 
 target_dir=${CARGO_TARGET_DIR:-${source_root}/target}
@@ -31,11 +31,8 @@ artifact="s3://${artifact_bucket}/${artifact_key}"
 if ! aws_cli s3api head-object \
   --bucket "${artifact_bucket}" \
   --key "${artifact_key}" >/dev/null 2>&1; then
-  aws_cli s3 cp "${worker_binary}" "${artifact}"
+  aws_cli s3 cp "${worker_binary}" "${artifact}" >&2
 fi
 
-update_runtime_file \
-  '.workerArtifact = $workerArtifact | .workerBinarySha = $workerBinarySha' \
-  --arg workerArtifact "${artifact}" \
-  --arg workerBinarySha "${binary_sha}"
-echo "Published ${artifact}"
+echo "Published ${artifact}" >&2
+echo "${artifact}"

benchmarks-remote/k8s/publish-image.sh

@@ -29,7 +29,7 @@ if ! aws_cli ecr describe-images \
   archive=$(mktemp)
   COPYFILE_DISABLE=1 tar -czf "${archive}" -C "${context}" Dockerfile spark_http.py
   artifact="s3://${results_bucket}/runs/bootstrap/images/${engine}-${tag}.tar.gz"
-  aws_cli s3 cp "${archive}" "${artifact}"
+  aws_cli s3 cp "${archive}" "${artifact}" >&2
   build_id=$(aws_cli codebuild start-build \
     --project-name "${builder}" \
     --environment-variables-override \
@@ -62,6 +62,5 @@ if ! aws_cli ecr describe-images \
   done
 fi
 
-update_runtime_file '.images = (.images // {}) | .images[$engine] = $image' \
-  --arg engine "${engine}" --arg image "${image}"
-echo "Published ${image}"
+echo "Published ${image}" >&2
+echo "${image}"

benchmarks-remote/pulumi/destroy.sh

@@ -22,9 +22,6 @@ else
   kubeconfig=${KUBECONFIG:-${script_dir}/../k8s/.kubeconfig.${stack}}
 fi
 rm -f "${outputs_file}" "${kubeconfig}"
-if [[ ${stack} == benchmark ]]; then
-  rm -f "${script_dir}/../k8s/.runtime.json"
-fi
 "${pulumi_bin}" stack select "${stack}"
 "${pulumi_bin}" state unprotect --stack "${stack}" --all --yes
 "${pulumi_bin}" destroy --stack "${stack}" --yes

benchmarks-remote/test/run-benchmark.test.ts

@@ -28,6 +28,28 @@ test("benchmark runs do not create cluster state", () => {
   assert.doesNotMatch(library, /benchmark_lock|heartbeat|configmap/);
 });
 
+test("engine publishers return artifacts directly without runtime files", () => {
+  const files = [
+    "lib.sh",
+    "deploy-engine.sh",
+    "publish-datafusion.sh",
+    "publish-ballista.sh",
+    "publish-image.sh",
+  ].map((file) =>
+    fs.readFileSync(path.resolve(__dirname, "../k8s", file), "utf8"),
+  );
+  for (const source of files) {
+    assert.doesNotMatch(
+      source,
+      /runtime_file|K8S_RUNTIME_FILE|update_runtime_file|output_value/,
+    );
+  }
+  const deploy = files[1]!;
+  assert.match(deploy, /worker_artifact=.*publish-datafusion\.sh/);
+  assert.match(deploy, /spark_image=.*publish-image\.sh/);
+  assert.match(deploy, /ballista_artifacts=.*publish-ballista\.sh/);
+});
+
 test("all benchmark clients use the same local port", () => {
   for (const client of [
     "datafusion-bench.ts",