microsoft / microsoft/mssql-rs

write/ benchmark scenario cannot fail the perf regression gate

Open
#534 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
53
Forks
14
Avg merge
1d 15h
Merged PRs (30d)
137

Description

## Context

PR #512 added the `write/` (`executemany`/parameter-array) benchmark scenario, but flagged in review (https://github.com/microsoft/mssql-rs/pull/512#discussion_r3971... on `mssql-odbc-bench/perf-lab/run-benchmarks.sh:475` and the equivalent `run-benchmarks.ps1:373-380`) that the regression gate treats it as apples-to-oranges:

- The baseline leg is forced to `sequential` mode (looped single-row execute) because the pinned baseline driver rejects `PARAMSET_SIZE > 1` with `HYC00`.
- The candidate leg runs `parameter_array` (batched) mode.
- `compare-odbc-benchmarks.py` compares `candidate_seconds / base_seconds` for every scenario including `write/`, using `DEFAULT_REGRESSION_RATIO = 1.05` (5%) as the failure threshold.
- With this PR's own published medians (1588.03 ms baseline sequential, 25.75 ms candidate batched) the ratio is ~0.0162. The candidate would have to regress roughly **65x** before the gate would flag it — every other scenario is guarded at 5%.
- Because `initial_improvement = initial_ratio <= 1/1.05` is essentially always true for `write/`, and improvements are sorted ascending and capped to `--improvement-max 3`, `write/` permanently occupies one of the three improvement-verification slots on every perf run (each costing a full paired re-run), crowding out genuine improvements from other scenarios.
- The mode split is selected by **driver name** rather than capability (`if scenario == "write" and driver == BASELINE_DRIVER_NAME: write_mode = sequential`), so once the baseline is re-pinned to any commit at or after this PR (one that *can* run parameter arrays), the baseline leg is still forced to `sequential` and the comparison stays apples-to-oranges indefinitely.

## Suggested options (roughly by effort, least first)

1. Gate `write/` on `candidate_vs_reference_ratio` (vs. the Microsoft msodbcsql driver) instead of vs. the sequential-mode baseline.
2. Select `write_mode` by probing driver capability (e.g. attempt `SQLSetStmtAttr(SQL_ATTR_PARAMSET_SIZE, 2)` against the baseline driver) rather than matching driver name, so the split self-heals once the baseline supports parameter arrays.
3. Exclude `write/` from the improvement-verification slot allocation in `confirmation_plan` (`.pipeline/scripts/compare-odbc-benchmarks.py`) so it stops permanently consuming one of `--improvement-max` slots, independent of fixing the regression-gate threshold itself.

Any regression-gate rewrite needs a perf-lab run to validate before landing, since a mistake here silently disables (or falsely trips) the gate for CI.

Credit: flagged by @Vahid-b in PR #512 review.

Contributor guide

Open the contributing guide

Research direction

Start with mssql-odbc-bench/perf-lab/run-benchmarks.sh around line 475, run-benchmarks.ps1 around lines 373-380, and .pipeline/scripts/compare-odbc-benchmarks.py, especially the ratio and confirmation_plan logic. Run the perf-lab validation described in the issue. Done means write/ is compared fairly, does not permanently consume an improvement-verification slot, and the CI gate still detects regressions.

Written by the indexing model from the issue text.

Assessment

Tech stack
powershell, python, shell
Domain
ci-cd, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.