diff --git a/benchmark-bot/src/executor.ts b/benchmark-bot/src/executor.ts index 6a8f439..6e1909e 100644 --- a/benchmark-bot/src/executor.ts +++ b/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, diff --git a/benchmark-bot/src/render.ts b/benchmark-bot/src/render.ts index 9a5b6b7..00bb695 100644 --- a/benchmark-bot/src/render.ts +++ b/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,10 +45,23 @@ 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 = `
-Run metadata +Verification and run details + +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 | | --- | ---: | ---: | @@ -54,20 +69,38 @@ export function renderResult( | 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 -
+**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); +`; + 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 { diff --git a/benchmark-bot/src/worker.ts b/benchmark-bot/src/worker.ts index c57187d..e1f661e 100644 --- a/benchmark-bot/src/worker.ts +++ b/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; } export class JobWorker { diff --git a/benchmark-bot/test/executor.test.ts b/benchmark-bot/test/executor.test.ts index 33950ed..5fcdcb2 100644 --- a/benchmark-bot/test/executor.test.ts +++ b/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"), diff --git a/benchmark-bot/test/worker.test.ts b/benchmark-bot/test/worker.test.ts index df3e741..5ab2876 100644 --- a/benchmark-bot/test/worker.test.ts +++ b/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(); diff --git a/benchmarks-remote/.gitignore b/benchmarks-remote/.gitignore index 20bdf8e..68bc67f 100644 --- a/benchmarks-remote/.gitignore +++ b/benchmarks-remote/.gitignore @@ -1,4 +1,3 @@ node_modules/ engines/ballista/target/ k8s/.kubeconfig -k8s/.runtime.json diff --git a/benchmarks-remote/k8s/deploy-engine.sh b/benchmarks-remote/k8s/deploy-engine.sh index 259ec0f..f4aa5ae 100755 --- a/benchmarks-remote/k8s/deploy-engine.sh +++ b/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}" diff --git a/benchmarks-remote/k8s/lib.sh b/benchmarks-remote/k8s/lib.sh index c61894c..744ef75 100644 --- a/benchmarks-remote/k8s/lib.sh +++ b/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}" "$@" } diff --git a/benchmarks-remote/k8s/publish-ballista.sh b/benchmarks-remote/k8s/publish-ballista.sh index 7b9695f..e05d76f 100755 --- a/benchmarks-remote/k8s/publish-ballista.sh +++ b/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}" diff --git a/benchmarks-remote/k8s/publish-datafusion.sh b/benchmarks-remote/k8s/publish-datafusion.sh index 6362dec..32370cb 100755 --- a/benchmarks-remote/k8s/publish-datafusion.sh +++ b/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}" diff --git a/benchmarks-remote/k8s/publish-image.sh b/benchmarks-remote/k8s/publish-image.sh index 3b97f3f..fd1cb2b 100755 --- a/benchmarks-remote/k8s/publish-image.sh +++ b/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}" diff --git a/benchmarks-remote/pulumi/destroy.sh b/benchmarks-remote/pulumi/destroy.sh index 4936d1c..5557b82 100755 --- a/benchmarks-remote/pulumi/destroy.sh +++ b/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 diff --git a/benchmarks-remote/test/run-benchmark.test.ts b/benchmarks-remote/test/run-benchmark.test.ts index 57145b8..e9b7a33 100644 --- a/benchmarks-remote/test/run-benchmark.test.ts +++ b/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",