diff --git a/.github/actions/launch-benchmark/action.yml b/.github/actions/launch-benchmark/action.yml index 92915ac0e..85e4f148c 100644 --- a/.github/actions/launch-benchmark/action.yml +++ b/.github/actions/launch-benchmark/action.yml @@ -15,14 +15,21 @@ inputs: description: "EC2 instance types, space-separated" default: "c6a.4xlarge" repo: - description: "GitHub repository (owner/name) the machine will clone" - default: "ClickHouse/ClickBench" + description: "GitHub repository (owner/name) the machine will clone. + Empty: the PR's head repository if pr is given, else ClickHouse/ClickBench." + default: "" branch: - description: "Branch of that repository to clone" - default: "main" + description: "Branch of that repository to clone. + Empty: the PR's head branch if pr is given, else main." + default: "" timeout: description: "Time limit for benchmark.sh on the machine, seconds" default: "36000" + pr: + description: "Pull request number that launched the run, if any. Recorded + in the log and parsed into the clickbench_pr column of sink.results, so + that results of PR runs can be told apart from the runs of main." + default: "" aws-region: description: "AWS region to launch the machine in" default: "us-east-1" @@ -45,6 +52,8 @@ runs: repo: ${{ inputs.repo }} branch: ${{ inputs.branch }} timeout: ${{ inputs.timeout }} + clickbench_pr: ${{ inputs.pr }} + GH_TOKEN: ${{ github.token }} run: | failed="" for m in $machines; do diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 5d309fa4b..96a898862 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -6,7 +6,10 @@ used only to launch: it assumes a federated IAM role and runs clones the repository, runs `/benchmark.sh` under cloud-init (see `cloud-init.sh.in`), sends its log to the results sink at play.clickhouse.com, and shuts down. Collecting the results back into `/results/` is a -separate process, implemented separately. +separate process, implemented separately. Runs launched for a pull request +carry the PR number in the log (the `ClickBench PR:` line), which the sink +parses into the `clickbench_pr` column of `sink.results`, so they can be told +apart from the runs of main and are excluded by `collect-results.sh`. | Workflow | Trigger | What it launches | |----------|---------|------------------| diff --git a/.github/workflows/benchmark-manual.yml b/.github/workflows/benchmark-manual.yml index 8ca1f4d25..0df2cb41c 100644 --- a/.github/workflows/benchmark-manual.yml +++ b/.github/workflows/benchmark-manual.yml @@ -1,6 +1,6 @@ name: "Run a benchmark" -run-name: "Benchmark ${{ inputs.systems }} on ${{ inputs.machines }} from ${{ inputs.repo }}@${{ inputs.branch }}" +run-name: "Benchmark ${{ inputs.systems }} on ${{ inputs.machines }} ${{ inputs.pr && format('for PR #{0}', inputs.pr) || format('from {0}@{1}', inputs.repo || 'ClickHouse/ClickBench', inputs.branch || 'main') }}" on: workflow_dispatch: @@ -11,12 +11,15 @@ on: machines: description: "EC2 instance types (space-separated)" default: "c6a.4xlarge" + pr: + description: "Pull request number: recorded in the results, and the PR's head repo/branch are benchmarked unless repo/branch are given" + default: "" repo: - description: "Repository to clone on the machine" - default: "ClickHouse/ClickBench" + description: "Repository to clone on the machine (empty: from the PR, or ClickHouse/ClickBench)" + default: "" branch: - description: "Branch to clone" - default: "main" + description: "Branch to clone (empty: from the PR, or main)" + default: "" timeout: description: "Time limit for benchmark.sh, seconds" default: "36000" @@ -35,6 +38,7 @@ jobs: with: systems: ${{ inputs.systems }} machines: ${{ inputs.machines }} + pr: ${{ inputs.pr }} repo: ${{ inputs.repo }} branch: ${{ inputs.branch }} timeout: ${{ inputs.timeout }} diff --git a/.github/workflows/benchmark-pr.yml b/.github/workflows/benchmark-pr.yml index 738e35427..fc1d4ad7f 100644 --- a/.github/workflows/benchmark-pr.yml +++ b/.github/workflows/benchmark-pr.yml @@ -67,5 +67,6 @@ jobs: - uses: ./.github/actions/launch-benchmark with: systems: ${{ needs.detect.outputs.systems }} - repo: ${{ github.event.pull_request.head.repo.full_name }} - branch: ${{ github.event.pull_request.head.ref }} + # The head repository and branch to benchmark are derived from the + # PR number by run-benchmark.sh. + pr: ${{ github.event.pull_request.number }} diff --git a/cloud-init.sh.in b/cloud-init.sh.in index 9017df65a..9fe5e2365 100644 --- a/cloud-init.sh.in +++ b/cloud-init.sh.in @@ -43,6 +43,10 @@ jq -r '.tuned' template.json | tee -a log echo -n 'Tags: ' | tee -a log jq -c -r '.tags' template.json | tee -a log +# The pull request number when launched by the PR workflow, empty otherwise, +# so that results of PR runs can be told apart from the runs of main. +echo 'ClickBench PR: @clickbench_pr@' | tee -a log + echo -n 'Disk usage before: ' | tee -a log df -B1 / | tail -n1 | awk '{ print $3 }' | tee -a log diff --git a/collect-results.sh b/collect-results.sh index cc5b42ab5..20738defb 100755 --- a/collect-results.sh +++ b/collect-results.sh @@ -10,10 +10,12 @@ # content. The result was JSON files landing in the wrong dated subdir # and `generate-results.sh` never seeing today's entries. Order # explicitly so LIMIT 1 BY keeps the latest row for each pair. +# clickbench_pr = 0 keeps runs launched for pull requests out of the +# published results - they are for the PR's review, not for the website. clickhouse-client $CONNECTION_PARAMS --query " SELECT format(\$\$ - SELECT output FROM sink.results WHERE system = '{0}' AND machine = '{1}' ORDER BY time DESC LIMIT 1 + SELECT output FROM sink.results WHERE system = '{0}' AND machine = '{1}' AND clickbench_pr = 0 ORDER BY time DESC LIMIT 1 INTO OUTFILE '{0}/results/{2}/{1}.json' TRUNCATE FORMAT Raw SETTINGS into_outfile_create_parent_directories = 1; \$\$, system, machine, formatDateTime(time, '%Y%m%d', 'UTC')) FROM sink.results -WHERE time >= today() - INTERVAL 2 DAY AND system NOT LIKE '%\\'%' ORDER BY time DESC LIMIT 1 BY system, machine FORMAT Raw +WHERE time >= today() - INTERVAL 2 DAY AND clickbench_pr = 0 AND system NOT LIKE '%\\'%' ORDER BY time DESC LIMIT 1 BY system, machine FORMAT Raw " | clickhouse-client $CONNECTION_PARAMS diff --git a/prepare-database.sql b/prepare-database.sql index 9ea4e68c9..e5b9ef511 100644 --- a/prepare-database.sql +++ b/prepare-database.sql @@ -31,6 +31,9 @@ CREATE TABLE sink.results -- BENCH_CONCURRENT_DURATION was set to 0). concurrent_qps Nullable(Float64), concurrent_error_ratio Nullable(Float64), + -- The pull request number when the run was launched by the PR workflow; + -- 0 for runs of main and for logs predating this column. + clickbench_pr UInt32, output String, ) ENGINE = ReplicatedMergeTree('/clickhouse/tables/sink/results/{shard}', '{replica}') ORDER BY (time); @@ -62,11 +65,16 @@ WITH toFloat64OrNull(extract(content, 'Concurrent QPS: ([0-9.]+|null)')) AS concurrent_qps, toFloat64OrNull(extract(content, 'Concurrent error ratio: ([0-9.]+|null)')) AS concurrent_error_ratio, + -- \d* rather than \d+: the header line is 'ClickBench PR: ' with no number + -- for runs of main, and the empty match there stops the search before any + -- 'ClickBench PR: ' that may occur later in untrusted bench output. + toUInt32OrZero(extract(content, 'ClickBench PR: (\d*)')) AS clickbench_pr, + load_time IS NOT NULL AND length(runtimes) = 43 AND data_size >= 5000000000 AND arrayExists(x -> arrayExists(y -> toFloat64OrZero(y) > 0.1, x), runtimes) AS good SELECT time, system, machine, system_name, proprietary, tuned, tags, total_time, disk_space_diff, load_time, data_size, length(runtimes) AS num_results, runtimes, runtimes_formatted, - concurrent_qps, concurrent_error_ratio, + concurrent_qps, concurrent_error_ratio, clickbench_pr, '{ "system": "' || system_name || '", "date": "' || time::Date || '", diff --git a/run-benchmark.sh b/run-benchmark.sh index bc6bc2e40..aaac95534 100755 --- a/run-benchmark.sh +++ b/run-benchmark.sh @@ -2,6 +2,24 @@ machine=${machine:=c6a.4xlarge} system=${system:=clickhouse} +clickbench_pr=${clickbench_pr:=} + +# When launched for a pull request and no explicit repo/branch are given, +# benchmark the PR's own head repository and branch. GH_TOKEN is optional: +# unauthenticated GitHub API calls are rate-limited per IP, which bites on +# the shared IPs of GitHub-hosted runners. +if [ -n "$clickbench_pr" ] && { [ -z "$repo" ] || [ -z "$branch" ]; }; then + auth=() + [ -n "$GH_TOKEN" ] && auth=(-H "Authorization: Bearer $GH_TOKEN") + pr_head=$(curl -sSf "${auth[@]}" "https://api.github.com/repos/ClickHouse/ClickBench/pulls/${clickbench_pr}") + repo=${repo:-$(jq -r '.head.repo.full_name' <<< "$pr_head")} + branch=${branch:-$(jq -r '.head.ref' <<< "$pr_head")} + if [ -z "$repo" ] || [ "$repo" = "null" ] || [ -z "$branch" ] || [ "$branch" = "null" ]; then + echo "Cannot resolve the head repository and branch of PR #${clickbench_pr}" >&2 + exit 1 + fi +fi + repo=${repo:=ClickHouse/ClickBench} branch=${branch:=main} @@ -12,12 +30,13 @@ ami=$(aws ec2 describe-images --owners amazon --filters "Name=name,Values=ubuntu # Default keeps the 10h cap that worked for the slowest OLTP systems. timeout="${timeout:-36000}" -awk -v sys="$system" -v repo="$repo" -v branch="$branch" -v t="$timeout" ' +awk -v sys="$system" -v repo="$repo" -v branch="$branch" -v t="$timeout" -v pr="$clickbench_pr" ' { gsub(/@system@/, sys) gsub(/@repo@/, repo) gsub(/@branch@/, branch) gsub(/@timeout@/, t) + gsub(/@clickbench_pr@/, pr) print }' cloud-init.sh.in > cloud-init.sh