Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
echo "Usage: ./get-job-json.sh <jobname> <fromServer> <fromVersion> <toServer> <toVersion> <timePrefix> <actorPrefix> <format> <issueNumber> <initBigRepo> <nomsBinFormat> <sysbenchTestTime> <withTpcc>"
exit 1
fi

Expand All @@ -18,7 +18,8 @@ format="$8"
issueNumber="$9"
initBigRepo="${10}"

Copy link
Copy Markdown

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

Medium severity Nightly benchmarks use the wrong settings

What failed: Starting a nightly benchmark with TPCC enabled produces a job with --sysbench-test-time=true and no --withTpcc=true option. The launcher accepts and applies the job instead of rejecting the incompatible caller contract.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Nightly benchmarks that enable TPCC run with the wrong duration setting and without TPCC. The job still starts, so teams may trust results that do not match the requested benchmark.
  • Steps to Reproduce:
    1. Run the benchmark launcher with the existing twelve-value caller contract and set TPCC to true.
    2. Capture the generated Kubernetes Job JSON before applying it.
    3. Inspect the workload arguments and compare the duration and TPCC options with the requested settings.
  • Stub / mock content: A local kubectl simulator captured the generated job without contacting a cluster; no production services, credentials, or application data were used.
  • Code Analysis: The existing caller in .github/scripts/performance-benchmarking/run-benchmarks.sh:72-85 invokes TEMPLATE_SCRIPT with twelve positional values after the script name, placing NOMS_BIN_FORMAT at position 11 and WITH_TPCC at position 12. The PR changes .github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:21-22 so position 12 is assigned to sysbenchTestTime and position 13 is assigned to withTpcc. Its rendering logic at :33-39 turns a non-empty sysbenchTestTime into --sysbench-test-time= and only renders --withTpcc when position 13 is present. Therefore the unchanged legacy invocation supplies true to the new duration slot and supplies no TPCC value. The direct reproduction rendered --sysbench-test-time=true, omitted --withTpcc=true, and still returned a successful job creation path. The PR diff explicitly introduced the new positional mapping and rendering; a targeted fix is to update callers to provide both new values or add an arity/contract check that fails clearly before generating the job.
  • Why this is likely a bug: This is a production automation defect rather than a harness artifact: the source paths have incompatible positional contracts, and a local direct rendering with the legacy invocation reproduces the exact wrong options. The failure is silent because kubectl accepts the generated job and the launcher reports success, while the benchmark workload no longer matches the caller's request. The fix is narrow: align the caller and template argument positions, or fail early when the old arity is used; no broad benchmark redesign is required.
Relevant code

.github/scripts/performance-benchmarking/run-benchmarks.sh:72-85

source "$TEMPLATE_SCRIPT" ... "$NOMS_BIN_FORMAT" \
  "$WITH_TPCC" > job.json

.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:19-22

initBigRepo="${10}"
nomsBinFormat="${11}"
sysbenchTestTime="${12}"
withTpcc="${13}"

.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:33-39

if [ -n "$sysbenchTestTime" ]; then
  sysbenchTestTime="\"--sysbench-test-time=$sysbenchTestTime\","
fi

if [ -n "$withTpcc" ]; then
  withTpcc="\"--withTpcc=$withTpcc\","

.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:101-107

"--results-prefix='$actorPrefix'",
              '"$sysbenchTestTime"'
              '"$withTpcc"'
              '"$initBigRepo"'
              '"$nomsBinFormat"'
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Nightly benchmarks use the wrong settings**

**What failed:** Starting a nightly benchmark with TPCC enabled produces a job with --sysbench-test-time=true and no --withTpcc=true option. The launcher accepts and applies the job instead of rejecting the incompatible caller contract.

- **Impact:** Nightly benchmarks that enable TPCC run with the wrong duration setting and without TPCC. The job still starts, so teams may trust results that do not match the requested benchmark.
- **Steps to reproduce:**
  1. Run the benchmark launcher with the existing twelve-value caller contract and set TPCC to true.
  2. Capture the generated Kubernetes Job JSON before applying it.
  3. Inspect the workload arguments and compare the duration and TPCC options with the requested settings.
- **Stub / mock content:** A local kubectl simulator captured the generated job without contacting a cluster; no production services, credentials, or application data were used.
- **Code analysis:** The existing caller in .github/scripts/performance-benchmarking/run-benchmarks.sh:72-85 invokes TEMPLATE_SCRIPT with twelve positional values after the script name, placing NOMS_BIN_FORMAT at position 11 and WITH_TPCC at position 12. The PR changes .github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:21-22 so position 12 is assigned to sysbenchTestTime and position 13 is assigned to withTpcc. Its rendering logic at :33-39 turns a non-empty sysbenchTestTime into --sysbench-test-time=<value> and only renders --withTpcc when position 13 is present. Therefore the unchanged legacy invocation supplies true to the new duration slot and supplies no TPCC value. The direct reproduction rendered --sysbench-test-time=true, omitted --withTpcc=true, and still returned a successful job creation path. The PR diff explicitly introduced the new positional mapping and rendering; a targeted fix is to update callers to provide both new values or add an arity/contract check that fails clearly before generating the job.
- **Why this is likely a bug:** This is a production automation defect rather than a harness artifact: the source paths have incompatible positional contracts, and a local direct rendering with the legacy invocation reproduces the exact wrong options. The failure is silent because kubectl accepts the generated job and the launcher reports success, while the benchmark workload no longer matches the caller's request. The fix is narrow: align the caller and template argument positions, or fail early when the old arity is used; no broad benchmark redesign is required.

**Relevant code:**

`.github/scripts/performance-benchmarking/run-benchmarks.sh:72-85`

~~~sh
source "$TEMPLATE_SCRIPT" ... "$NOMS_BIN_FORMAT" \
  "$WITH_TPCC" > job.json
~~~

`.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:19-22`

~~~sh
initBigRepo="${10}"
nomsBinFormat="${11}"
sysbenchTestTime="${12}"
withTpcc="${13}"
~~~

`.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:33-39`

~~~sh
if [ -n "$sysbenchTestTime" ]; then
  sysbenchTestTime="\"--sysbench-test-time=$sysbenchTestTime\","
fi

if [ -n "$withTpcc" ]; then
  withTpcc="\"--withTpcc=$withTpcc\","
~~~

`.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:101-107`

~~~sh
"--results-prefix='$actorPrefix'",
              '"$sysbenchTestTime"'
              '"$withTpcc"'
              '"$initBigRepo"'
              '"$nomsBinFormat"'
~~~

nomsBinFormat="${11}"
withTpcc="${12}"
sysbenchTestTime="${12}"
withTpcc="${13}"
tpccRegex="tpcc%"

if [ -n "$initBigRepo" ]; then
Expand All @@ -29,6 +30,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
Expand Down Expand Up @@ -96,6 +101,7 @@ echo '
"--issue-number='$issueNumber'",
"--results-dir='$timePrefix'",
"--results-prefix='$actorPrefix'",
'"$sysbenchTestTime"'
'"$withTpcc"'
'"$initBigRepo"'
'"$nomsBinFormat"'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ if [[ $# -ne 1 ]]; then
exit 1
fi

validcommentors="coffeegoddd bheni jennifersp Hydrocharged reltuk tbantle22 timsehn zachmu max-hoffman nicktobey fulghum"
validcommentors="coffeegoddd bheni Hydrocharged reltuk tbantle22 timsehn zachmu nicktobey fulghum macneale4 jycor angelamayxie elianddb jennifersp eric-richardson1"

contains() {
[[ $1 =~ (^|[[:space:]])$2($|[[:space:]]) ]] && echo "valid=true" >> $GITHUB_OUTPUT || exit 0
Expand Down
129 changes: 0 additions & 129 deletions .github/workflows/mini-sysbench.yml

This file was deleted.

41 changes: 29 additions & 12 deletions .github/workflows/pull-report.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,37 @@ jobs:
github-token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
script: |
const { ACTOR, FORMAT, ISSUE_NUMBER, GITHUB_WORKSPACE } = process.env;
const issue_number = parseInt(ISSUE_NUMBER, 10);
const { owner, repo } = context.repo;
fs = require('fs');
fs.readFile(`${GITHUB_WORKSPACE}/results.log`, 'utf8', function (err,data) {
if (err) {
return console.log(err);
}
return github.rest.issues.createComment({
issue_number,
owner,
repo,
body: `@${ACTOR} ${FORMAT}\n ${data}`
});
const issue_number = parseInt(ISSUE_NUMBER, 10);

const fs = require('fs').promises;

Copy link
Copy Markdown

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

Medium severity Untrusted report text can break review comments

What failed: The workflow could not safely and reliably publish the result text. The downloaded report is inserted directly into the reviewer-facing comment, and the branch for an existing marked comment is missing its if statement and update call, leaving an orphaned object property before an unmatched else.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Pull request reports may fail to appear or update, and untrusted report text may trigger unwanted mentions or mislead reviewers through rendered Markdown.
  • Steps to Reproduce:
    1. Provide a results.log file containing an @mention, Markdown or HTML comment text, and more text than the expected report size.
    2. Trigger the pull-report workflow with a valid pull request number and result object.
    3. Observe the github-script step while it searches for the report marker and builds the comment body; then inspect whether a comment is created or updated and how the result text is rendered.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The PR changes .github/workflows/pull-report.yaml from a simple createComment call to a marker-based update-or-create flow. At lines 40-41, fs.promises.readFile loads results.log into resData without a size limit, validation, or a policy for handling mention-like or comment-like content. At lines 53-58, comments.find(...) is followed immediately by comment_id, owner, repo, body, and a closing }); there is no if (comment) statement and no github.rest.issues.updateComment call. Line 59 then starts } else {, which has no matching conditional in the script as written. The createComment object at lines 61-66 repeats direct ${resData} interpolation. The result is a source-level syntax/control-flow defect that can prevent the github-script step from executing, plus an unsafe publication path that would allow untrusted result text to affect notifications and rendered comment structure if the branch were repaired without adding bounded content handling. The smallest practical fix is to restore a valid if (comment) { await github.rest.issues.updateComment({ comment_id: comment.id, owner, repo, body }) } else { await github.rest.issues.createComment({ issue_number, owner, repo, body }) } structure, and apply a narrowly scoped maximum size and safe rendering/mention policy to resData before constructing body.
  • Why this is likely a bug: The expected behavior requires adversarial report text to remain bounded and not control mentions, marker selection, or comment structure. The source always places the complete downloaded file into the comment body, so an @mention can notify an unintended user and Markdown or HTML comment syntax can change what reviewers see. Independently, the malformed lines 53-59 prevent the existing-marker path from being a valid JavaScript conditional and omit the required updateComment request, so repeated reports cannot reliably update the intended comment. The local simulator was unavailable, but this conclusion does not depend on that environment because the defect is visible in the production workflow source and the PR diff directly changed these lines.
Relevant code

.github/workflows/pull-report.yaml:40-43

const fs = require('fs').promises;
const resData = await fs.readFile(`${GITHUB_WORKSPACE}/results.log`, 'utf8');

const commentMarker = '<!-- go-run-output-sysbench -->';

.github/workflows/pull-report.yaml:52-67

// Check if a comment already exists
const comment = comments.find(comment => comment.body.includes(commentMarker))
    comment_id: comment.id,
    owner: context.repo.owner,
    repo: context.repo.repo,
    body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
  })
} else {
  // Create a new comment
  await github.rest.issues.createComment({
    issue_number: issue_number,
    owner: context.repo.owner,
    repo: context.repo.repo,
    body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
  });
}
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Untrusted report text can break review comments**

**What failed:** The workflow could not safely and reliably publish the result text. The downloaded report is inserted directly into the reviewer-facing comment, and the branch for an existing marked comment is missing its if statement and update call, leaving an orphaned object property before an unmatched else.

- **Impact:** Pull request reports may fail to appear or update, and untrusted report text may trigger unwanted mentions or mislead reviewers through rendered Markdown.
- **Steps to reproduce:**
  1. Provide a results.log file containing an @mention, Markdown or HTML comment text, and more text than the expected report size.
  2. Trigger the pull-report workflow with a valid pull request number and result object.
  3. Observe the github-script step while it searches for the report marker and builds the comment body; then inspect whether a comment is created or updated and how the result text is rendered.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR changes .github/workflows/pull-report.yaml from a simple createComment call to a marker-based update-or-create flow. At lines 40-41, fs.promises.readFile loads results.log into resData without a size limit, validation, or a policy for handling mention-like or comment-like content. At lines 53-58, comments.find(...) is followed immediately by comment_id, owner, repo, body, and a closing `})`; there is no `if (comment)` statement and no `github.rest.issues.updateComment` call. Line 59 then starts `} else {`, which has no matching conditional in the script as written. The createComment object at lines 61-66 repeats direct `${resData}` interpolation. The result is a source-level syntax/control-flow defect that can prevent the github-script step from executing, plus an unsafe publication path that would allow untrusted result text to affect notifications and rendered comment structure if the branch were repaired without adding bounded content handling. The smallest practical fix is to restore a valid `if (comment) { await github.rest.issues.updateComment({ comment_id: comment.id, owner, repo, body }) } else { await github.rest.issues.createComment({ issue_number, owner, repo, body }) }` structure, and apply a narrowly scoped maximum size and safe rendering/mention policy to resData before constructing body.
- **Why this is likely a bug:** The expected behavior requires adversarial report text to remain bounded and not control mentions, marker selection, or comment structure. The source always places the complete downloaded file into the comment body, so an @mention can notify an unintended user and Markdown or HTML comment syntax can change what reviewers see. Independently, the malformed lines 53-59 prevent the existing-marker path from being a valid JavaScript conditional and omit the required updateComment request, so repeated reports cannot reliably update the intended comment. The local simulator was unavailable, but this conclusion does not depend on that environment because the defect is visible in the production workflow source and the PR diff directly changed these lines.

**Relevant code:**

`.github/workflows/pull-report.yaml:40-43`

~~~yaml/javascript
const fs = require('fs').promises;
const resData = await fs.readFile(`${GITHUB_WORKSPACE}/results.log`, 'utf8');

const commentMarker = '<!-- go-run-output-sysbench -->';
~~~

`.github/workflows/pull-report.yaml:52-67`

~~~yaml/javascript
// Check if a comment already exists
const comment = comments.find(comment => comment.body.includes(commentMarker))
    comment_id: comment.id,
    owner: context.repo.owner,
    repo: context.repo.repo,
    body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
  })
} else {
  // Create a new comment
  await github.rest.issues.createComment({
    issue_number: issue_number,
    owner: context.repo.owner,
    repo: context.repo.repo,
    body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
  });
}
~~~

const resData = await fs.readFile(`${GITHUB_WORKSPACE}/results.log`, 'utf8');

const commentMarker = '<!-- go-run-output-sysbench -->';

// List comments on the PR

Copy link
Copy Markdown

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

Medium severity Existing benchmark comments cannot be updated

What failed: The workflow finds the marked comment, but the code immediately starts an object fragment with comment_id: comment.id and later contains an unmatched } else {. There is no valid conditional or updateComment request, so an existing marker comment cannot be updated as intended.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: When a report comment already exists, later benchmark results may fail to publish or may not update that comment. Users may miss the latest report until the workflow is fixed or the old comment is handled manually.
  • Steps to Reproduce:
    1. Run the pull-report workflow with a valid benchmark result and a pull request that already has a comment containing the benchmark marker.
    2. Let the workflow list the pull request comments and find the marked comment.
    3. Inspect the publication step and the pull request conversation for the new result.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: In /tmp/output-agent-workspace/repo/.github/workflows/pull-report.yaml, the PR changes the publication script at lines 53-67. Line 53 assigns comment from comments.find(...), but line 54 begins comment_id: comment.id without an if (comment) block or an await github.rest.issues.updateComment({ call. Lines 54-58 are therefore not a valid statement, and line 59 adds } else { without a matching opening conditional. The only complete API call in this section is the create-comment call at lines 61-66, which is reached only through the malformed branch. The smallest fix is to wrap lines 54-58 in if (comment) { and add the missing await github.rest.issues.updateComment({ issue_number, comment_id: comment.id, owner, repo, body }); retain the existing create call in the else branch.
  • Why this is likely a bug: The defect is in production workflow code, not the unavailable local simulator: the source has an orphaned object-property fragment and an unmatched else in the exact path that handles an existing marker comment. This prevents the intended update-in-place behavior from executing for normal repeat benchmark reports. The PR diff directly changed this block, so the targeted fix is to restore the missing conditional and updateComment call rather than redesigning report publication.
Relevant code

.github/workflows/pull-report.yaml:45-67

const { data: comments } = await github.rest.issues.listComments({ ... });

// Check if a comment already exists
const comment = comments.find(comment => comment.body.includes(commentMarker))
    comment_id: comment.id,
    owner: context.repo.owner,
    repo: context.repo.repo,
    body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
  })
} else {
  // Create a new comment
  await github.rest.issues.createComment({ ... });

.github/workflows/pull-report.yaml:28-41

env:
  KEY: ${{ github.event.client_payload.key }}
  BUCKET: ${{ github.event.client_payload.bucket }}
...
const issue_number = parseInt(ISSUE_NUMBER, 10);
const fs = require('fs').promises;
const resData = await fs.readFile(`${GITHUB_WORKSPACE}/results.log`, 'utf8');
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Existing benchmark comments cannot be updated**

**What failed:** The workflow finds the marked comment, but the code immediately starts an object fragment with `comment_id: comment.id` and later contains an unmatched `} else {`. There is no valid conditional or `updateComment` request, so an existing marker comment cannot be updated as intended.

- **Impact:** When a report comment already exists, later benchmark results may fail to publish or may not update that comment. Users may miss the latest report until the workflow is fixed or the old comment is handled manually.
- **Steps to reproduce:**
  1. Run the pull-report workflow with a valid benchmark result and a pull request that already has a comment containing the benchmark marker.
  2. Let the workflow list the pull request comments and find the marked comment.
  3. Inspect the publication step and the pull request conversation for the new result.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** In `/tmp/output-agent-workspace/repo/.github/workflows/pull-report.yaml`, the PR changes the publication script at lines 53-67. Line 53 assigns `comment` from `comments.find(...)`, but line 54 begins `comment_id: comment.id` without an `if (comment)` block or an `await github.rest.issues.updateComment({` call. Lines 54-58 are therefore not a valid statement, and line 59 adds `} else {` without a matching opening conditional. The only complete API call in this section is the create-comment call at lines 61-66, which is reached only through the malformed branch. The smallest fix is to wrap lines 54-58 in `if (comment) {` and add the missing `await github.rest.issues.updateComment({ issue_number, comment_id: comment.id, owner, repo, body })`; retain the existing create call in the `else` branch.
- **Why this is likely a bug:** The defect is in production workflow code, not the unavailable local simulator: the source has an orphaned object-property fragment and an unmatched else in the exact path that handles an existing marker comment. This prevents the intended update-in-place behavior from executing for normal repeat benchmark reports. The PR diff directly changed this block, so the targeted fix is to restore the missing conditional and `updateComment` call rather than redesigning report publication.

**Relevant code:**

`.github/workflows/pull-report.yaml:45-67`

~~~yaml
const { data: comments } = await github.rest.issues.listComments({ ... });

// Check if a comment already exists
const comment = comments.find(comment => comment.body.includes(commentMarker))
    comment_id: comment.id,
    owner: context.repo.owner,
    repo: context.repo.repo,
    body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
  })
} else {
  // Create a new comment
  await github.rest.issues.createComment({ ... });
~~~

`.github/workflows/pull-report.yaml:28-41`

~~~yaml
env:
  KEY: ${{ github.event.client_payload.key }}
  BUCKET: ${{ github.event.client_payload.bucket }}
...
const issue_number = parseInt(ISSUE_NUMBER, 10);
const fs = require('fs').promises;
const resData = await fs.readFile(`${GITHUB_WORKSPACE}/results.log`, 'utf8');
~~~

const { data: comments } = await github.rest.issues.listComments({
issue_number: issue_number,
owner: context.repo.owner,
repo: context.repo.repo
});

// Check if a comment already exists
const comment = comments.find(comment => comment.body.includes(commentMarker))
comment_id: comment.id,
owner: context.repo.owner,
repo: context.repo.repo,
body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
})
} else {
// Create a new comment
await github.rest.issues.createComment({
issue_number: issue_number,
owner: context.repo.owner,
repo: context.repo.repo,
body: `${commentMarker}\n@${ACTOR} ${FORMAT}\n${resData}`
});
}
env:
ACTOR: ${{ github.event.client_payload.actor }}
ISSUE_NUMBER: ${{ github.event.client_payload.issue_number }}
Expand Down
55 changes: 55 additions & 0 deletions .github/workflows/sysbench-performance.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: Mini Sysbench

on:
pull_request:
types: [opened, synchronize, reopened]

permissions:
contents: read
pull-requests: write

jobs:
mini-sysbench:
name: Trigger Mini Benchmark Latency K8s Workflow
if: github.actor != 'dependabot[bot]'
runs-on: ubuntu-22.04
steps:
- name: Build benchmark payload (main HEAD + PR HEAD)
uses: actions/github-script@v7
id: build_payload
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
// For PR benchmarks, use the PR base commit (the exact target commit this PR was opened/updated against),
// not the moving HEAD of `main`.
const fromSha = context.payload.pull_request.base.sha;

// The latest commit on the PR branch.
const toSha = context.payload.pull_request.head.sha;

const payload = {

Copy link
Copy Markdown

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

Medium severity Benchmark duration is lost before the job starts

What failed: The requested benchmark duration reaches the first dispatch payload but is dropped before the Kubernetes job is rendered. The generated job preserves the pull request revisions and other metadata, but it does not contain --sysbench-test-time=20.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Benchmark runs can use a different duration than the one requested. This can make performance results unreliable and lead maintainers to make decisions from mismatched data.
  • Steps to Reproduce:
    1. Open or synchronize a pull request with different known base and head commit SHAs.
    2. Run the automatic benchmark workflow and inspect its repository-dispatch payload; confirm it includes sysbench_test_time=20.
    3. Follow the payload into the Kubernetes benchmark workflow and render the job using run-benchmarks.sh.
    4. Inspect the rendered container arguments and confirm that --sysbench-test-time=20 is absent and the TPCC position is being used as the duration position.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The new payload in .github/workflows/sysbench-performance.yaml lines 30-40 sets sysbench_test_time to "20". However, .github/workflows/k8s-benchmark-latency.yaml lines 41-56 exports the other client_payload fields to run-benchmarks.sh and has no SYSBENCH_TEST_TIME entry. Even if that environment mapping were added, run-benchmarks.sh lines 72-85 still calls the template with init-big-repo at argument 10, NOMS_BIN_FORMAT at argument 11, and WITH_TPCC at argument 12; it supplies no duration argument. The changed template in get-doltgres-doltgres-job-json.sh lines 5-22 now treats argument 12 as sysbenchTestTime and argument 13 as withTpcc, and lines 33-39 only emit --sysbench-test-time when argument 12 is non-empty. Consequently, the existing caller's TPCC value is interpreted as duration, while the requested 20-second value is never passed. The smallest practical fix is to map github.event.client_payload.sysbench_test_time to SYSBENCH_TEST_TIME in k8s-benchmark-latency.yaml and pass "$SYSBENCH_TEST_TIME" before "$WITH_TPCC" in run-benchmarks.sh, keeping the template's new positional contract consistent.
  • Why this is likely a bug: The test's rendered job contract independently confirms that the duration requested by the pull-request workflow is absent, and source inspection explains the exact positional shift that causes it. This is not a test-only setup issue: the production GitHub Actions and shell files contain the missing handoff. The benchmark can therefore report results for a different duration than the workflow explicitly requested. The direct fix is limited to forwarding the existing payload field and restoring the intended duration-before-TPCC argument order.
Relevant code

.github/workflows/sysbench-performance.yaml:30-40

const payload = {
  from_server: "doltgres",
  from_version: fromSha,
  to_server: "doltgres",
  to_version: toSha,
  mode: "pullRequest",
  issue_number: String(context.issue.number),
  init_big_repo: "true",
  actor: context.actor,
  sysbench_test_time: "20",
  template_script: "./.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh",
};

.github/workflows/k8s-benchmark-latency.yaml:41-56

- name: Create Sysbench Performance Benchmarking K8s Job
  run: ./.github/scripts/performance-benchmarking/run-benchmarks.sh
  env:
    FROM_SERVER: ${{ github.event.client_payload.from_server }}
    FROM_VERSION: ${{ github.event.client_payload.from_version }}
    TO_SERVER: ${{ github.event.client_payload.to_server }}
    TO_VERSION: ${{ github.event.client_payload.to_version }}
    MODE: ${{ github.event.client_payload.mode }}
    ISSUE_NUMBER: ${{ github.event.client_payload.issue_number }}
    ACTOR: ${{ github.event.client_payload.actor }}
    TEMPLATE_SCRIPT: ${{ github.event.client_payload.template_script }}

.github/scripts/performance-benchmarking/run-benchmarks.sh:72-85

source \
  "$TEMPLATE_SCRIPT" \
  "$jobname" \
  "$FROM_SERVER" \
  "$FROM_VERSION" \
  "$TO_SERVER" \
  "$TO_VERSION" \
  "$timeprefix" \
  "$actorprefix" \
  "$format" \
  "$issuenumber" \
  "$INIT_BIG_REPO" \
  "$NOMS_BIN_FORMAT" \
  "$WITH_TPCC" > job.json

.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:18-38

initBigRepo="${10}"
nomsBinFormat="${11}"
sysbenchTestTime="${12}"
withTpcc="${13}"
...
if [ -n "$sysbenchTestTime" ]; then
  sysbenchTestTime="\"--sysbench-test-time=$sysbenchTestTime\","
fi
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Benchmark duration is lost before the job starts**

**What failed:** The requested benchmark duration reaches the first dispatch payload but is dropped before the Kubernetes job is rendered. The generated job preserves the pull request revisions and other metadata, but it does not contain --sysbench-test-time=20.

- **Impact:** Benchmark runs can use a different duration than the one requested. This can make performance results unreliable and lead maintainers to make decisions from mismatched data.
- **Steps to reproduce:**
  1. Open or synchronize a pull request with different known base and head commit SHAs.
  2. Run the automatic benchmark workflow and inspect its repository-dispatch payload; confirm it includes sysbench_test_time=20.
  3. Follow the payload into the Kubernetes benchmark workflow and render the job using run-benchmarks.sh.
  4. Inspect the rendered container arguments and confirm that --sysbench-test-time=20 is absent and the TPCC position is being used as the duration position.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The new payload in .github/workflows/sysbench-performance.yaml lines 30-40 sets sysbench_test_time to "20". However, .github/workflows/k8s-benchmark-latency.yaml lines 41-56 exports the other client_payload fields to run-benchmarks.sh and has no SYSBENCH_TEST_TIME entry. Even if that environment mapping were added, run-benchmarks.sh lines 72-85 still calls the template with init-big-repo at argument 10, NOMS_BIN_FORMAT at argument 11, and WITH_TPCC at argument 12; it supplies no duration argument. The changed template in get-doltgres-doltgres-job-json.sh lines 5-22 now treats argument 12 as sysbenchTestTime and argument 13 as withTpcc, and lines 33-39 only emit --sysbench-test-time when argument 12 is non-empty. Consequently, the existing caller's TPCC value is interpreted as duration, while the requested 20-second value is never passed. The smallest practical fix is to map github.event.client_payload.sysbench_test_time to SYSBENCH_TEST_TIME in k8s-benchmark-latency.yaml and pass "$SYSBENCH_TEST_TIME" before "$WITH_TPCC" in run-benchmarks.sh, keeping the template's new positional contract consistent.
- **Why this is likely a bug:** The test's rendered job contract independently confirms that the duration requested by the pull-request workflow is absent, and source inspection explains the exact positional shift that causes it. This is not a test-only setup issue: the production GitHub Actions and shell files contain the missing handoff. The benchmark can therefore report results for a different duration than the workflow explicitly requested. The direct fix is limited to forwarding the existing payload field and restoring the intended duration-before-TPCC argument order.

**Relevant code:**

`.github/workflows/sysbench-performance.yaml:30-40`

~~~yaml
const payload = {
  from_server: "doltgres",
  from_version: fromSha,
  to_server: "doltgres",
  to_version: toSha,
  mode: "pullRequest",
  issue_number: String(context.issue.number),
  init_big_repo: "true",
  actor: context.actor,
  sysbench_test_time: "20",
  template_script: "./.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh",
};
~~~

`.github/workflows/k8s-benchmark-latency.yaml:41-56`

~~~yaml
- name: Create Sysbench Performance Benchmarking K8s Job
  run: ./.github/scripts/performance-benchmarking/run-benchmarks.sh
  env:
    FROM_SERVER: ${{ github.event.client_payload.from_server }}
    FROM_VERSION: ${{ github.event.client_payload.from_version }}
    TO_SERVER: ${{ github.event.client_payload.to_server }}
    TO_VERSION: ${{ github.event.client_payload.to_version }}
    MODE: ${{ github.event.client_payload.mode }}
    ISSUE_NUMBER: ${{ github.event.client_payload.issue_number }}
    ACTOR: ${{ github.event.client_payload.actor }}
    TEMPLATE_SCRIPT: ${{ github.event.client_payload.template_script }}
~~~

`.github/scripts/performance-benchmarking/run-benchmarks.sh:72-85`

~~~shell
source \
  "$TEMPLATE_SCRIPT" \
  "$jobname" \
  "$FROM_SERVER" \
  "$FROM_VERSION" \
  "$TO_SERVER" \
  "$TO_VERSION" \
  "$timeprefix" \
  "$actorprefix" \
  "$format" \
  "$issuenumber" \
  "$INIT_BIG_REPO" \
  "$NOMS_BIN_FORMAT" \
  "$WITH_TPCC" > job.json
~~~

`.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh:18-38`

~~~shell
initBigRepo="${10}"
nomsBinFormat="${11}"
sysbenchTestTime="${12}"
withTpcc="${13}"
...
if [ -n "$sysbenchTestTime" ]; then
  sysbenchTestTime="\"--sysbench-test-time=$sysbenchTestTime\","
fi
~~~

from_server: "doltgres",
from_version: fromSha,
to_server: "doltgres",
to_version: toSha,
mode: "pullRequest",
issue_number: String(context.issue.number),
init_big_repo: "true",
actor: context.actor,
sysbench_test_time: "20",
template_script: "./.github/scripts/performance-benchmarking/get-doltgres-doltgres-job-json.sh",
};

core.setOutput("from_sha", fromSha);
core.setOutput("to_sha", toSha);
core.setOutput("payload", JSON.stringify(payload));
- name: Log benchmark versions
shell: bash
run: |
echo "from_version_sha=${{ steps.build_payload.outputs.from_sha }}"
echo "to_version_sha=${{ steps.build_payload.outputs.to_sha }}"
- uses: peter-evans/repository-dispatch@v3
with:
token: ${{ secrets.REPO_ACCESS_TOKEN }}
event-type: benchmark-latency
client-payload: ${{ steps.build_payload.outputs.payload }}
Loading