-
-
Notifications
You must be signed in to change notification settings - Fork 71
[no-release-notes] more fixes to mini benchmarks #3088
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3f5bbce
44c72b9
677cd54
88545f8
312d872
b09368f
d8eb519
9f7a272
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,7 @@ echo ' | |
| "alert_recipients": "'$ACTOR_EMAIL'" | ||
| }, | ||
| "labels": { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🆕 New Failure: identified in this diff run
|
||
| "app": "performance-benchmarking" | ||
| "k8s-liquidata-inc-monitored-job": "created-by-static-config" | ||
| } | ||
| }, | ||
|
|
||
|
jycor marked this conversation as resolved.
jycor marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,8 @@ | |
|
|
||
| set -e | ||
|
|
||
| if [ "$#" -lt 9 ]; then | ||
| echo "Usage: ./get-job-json.sh <jobname> <fromServer> <fromVersion> <toServer> <toVersion> <timeprefix> <actorprefix> <format> <issueNumber> <initBigRepo> <nomsBinFormat> <withTpcc>" | ||
| if [ "$#" -lt 10 ]; then | ||
|
jycor marked this conversation as resolved.
|
||
| echo "Usage: ./get-job-json.sh <jobname> <fromServer> <fromVersion> <toServer> <toVersion> <timeprefix> <actorprefix> <format> <issueNumber> <initBigRepo> <nomsBinFormat> <sysbenchTestTime> <withTpcc>" | ||
| exit 1 | ||
| fi | ||
|
|
||
|
|
@@ -18,7 +18,8 @@ format="$8" | |
| issueNumber="$9" | ||
| initBigRepo="${10}" | ||
| nomsBinFormat="${11}" | ||
| withTpcc="${12}" | ||
| sysbenchTestTime="${12}" | ||
|
jycor marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🆕 New Failure: identified in this diff run
|
||
| withTpcc="${13}" | ||
| precision="1" | ||
| tpccRegex="tpcc%" | ||
|
|
||
|
|
@@ -30,6 +31,10 @@ if [ -n "$nomsBinFormat" ]; then | |
| nomsBinFormat="\"--noms-bin-format=$nomsBinFormat\"," | ||
| fi | ||
|
|
||
| if [ -n "$sysbenchTestTime" ]; then | ||
| sysbenchTestTime="\"--sysbench-test-time=$sysbenchTestTime\"," | ||
| fi | ||
|
|
||
| if [ -n "$withTpcc" ]; then | ||
| withTpcc="\"--withTpcc=$withTpcc\"," | ||
| fi | ||
|
|
@@ -101,6 +106,7 @@ echo ' | |
| "--region=us-west-2", | ||
| "--results-dir='$timeprefix'", | ||
| "--results-prefix='$actorprefix'", | ||
| '"$sysbenchTestTime"' | ||
|
jycor marked this conversation as resolved.
|
||
| '"$withTpcc"' | ||
| '"$initBigRepo"' | ||
| '"$nomsBinFormat"' | ||
|
|
||
|
jycor marked this conversation as resolved.
jycor marked this conversation as resolved.
|

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🆕 New Failure: identified in this diff run
What failed: The benchmark job data is not valid JSON, so the pull-request benchmark cannot start.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
"app": "performance-benchmarking"followed immediately by"k8s-liquidata-inc-monitored-job": "created-by-static-config", which is invalid JSON. The template command itself exits successfully because it only prints text; the downstream submission path in run-benchmarks.sh:72-88 writes that text to job.json and passes it to kubectl, so the malformed document prevents Job creation. The smallest practical fix is to add a comma after the app label at line 66, then retain the strict JSON parse check in the test coverage.Expected separator between values at line 18 column 43, matching the missing comma in the production template. Because the new label is required by the intended benchmark metadata while the existing monitored-job label must remain, both properties need to be comma-separated; adding that comma is a targeted fix for the observed failure.Relevant code
.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:65-68.github/scripts/performance-benchmarking/run-benchmarks.sh:72-90Evidence Package
Copy prompt for an agent