cockroachdb / cockroachdb/cockroach
roachtest/testselector: add duration-aware selection heuristics
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
## Summary
Add duration-aware heuristics to the test selector so we can systematically cap or balance test-suite wall time, instead of treating duration as a passive output field.
## Motivation
#171671 documents that the nightly roachtest GCE suite consistently times out. That issue identifies multiple contributors (#170652 benchmark retries, GCE quota, suite growth) and explicitly suggests duration-aware selection in several places:
- Under "Stagger CI Start Times": "the test selection heuristic could be modified to aim for a 12 hour execution time"
- Under "Schedules": explicit reference to bin-packing as a longer-term solution
- Under "Benchmark Quality": prioritization as a way to deprioritize less important benchmarks
The test selection migration in [#169712](https://github.com/cockroachdb/cockroach/issues/169712) can contribute to duration-aware selection. Selection would run in a Go function we can change, against a CRDB cluster that already records per-run duration. Specifically:
- `roachtest_runs.duration_ms` records per-run duration
- `aggregate.totalDuration` / `totalSuccessful` are computed in
`aggregateHistory`
- `TestDetails.AvgDurationInMillis` is already surfaced downstream
## Proposed approach
Add a post-`Categorize` pass that demotes selected tests whose avg duration exceeds N minutes — but **only when they were selected for a discretionary reason**. The cap explicitly does NOT override the coverage-floor rules:
| Reason | Cap overrides? | Why |
|--------|----------------|-----|
| RecentFailure | No | Broken tests must run |
| Preempt | No | Same |
| UnknownNotFromSelector | No | Investigate weirdness |
| **Stale** | No | Coverage floor — every test runs at least every LastRunOn days |
| NewTest | Yes | Newness is nice-to-have, not essential |
This is "prefer cheap tests when there's slack," not "blacklist slow tests." A test that consistently exceeds the cap still runs every `LastRunOn` days via the Stale rule, so coverage is preserved — just at a lower cadence than fast tests. Selector-skip rows are recorded as UNKNOWN, which `aggregateHistory` excludes from `lastRun`, so the Stale clock correctly ticks from the test's last *real* run.
Surface a metric for tests demoted by the cap so we can tune N and observe the suite's slow-test distribution.
## What the cap intentionally doesn't account for
The cap operates on raw avg duration in isolation — a single per-test check. That's deliberate (start small), but it means several real-world factors that affect "how expensive is this test, really" are ignored. These would all be potential additions when considering an ideal test selection heuristic:
| Factor | Why a raw-duration cap misses it |
| -------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **Effective parallelism** | Wall time ≈ compute time ÷ N. A 60-min test on a high-parallelism worker pool barely registers in wall time; on a starved pool it blocks everything. The cap can't tell which is which. |
| **Node count × spec** | A 60-min test on 1 small node ≠ a 60-min test on 12 big nodes. The cap measures runtime, but the budget actually consumed is `duration × nodes × spec`. Weighting by node count would be more honest. |
| **Cluster reuse** | Tests with `ReusePolicy != None` skip the spin-up cost. A 30-min slow-but-reusable test is cheaper overall than a 20-min fresh-cluster test. The cap doesn't differentiate. |
| **Setup / teardown overhead** | A 10-min test really costs ~15 min in VM time once you include cluster spin-up + teardown. The cap on raw runtime undercounts short tests' true cost. |
The cap accepts these inaccuracies for simplicity. A future iteration could weight by node count, segregate by VM spec, or migrate the budget logic to the runner (where parallelism and quota are observable). That's out of scope here — see Non-goals.
## Risk
A poorly-tuned cap could degrade selection quality *below* the duration-blind baseline:
- N too low → too many demotions → coverage gaps the Stale rule has to spend its slots fighting against (slow tests crowd out new tests on the days they're forced to run)
- N too high → ineffective, adds complexity for no benefit
Mitigations:
- Ship behind a feature flag so we can disable cleanly if telemetry shows it making things worse
- Start with a conservative N (high enough that few tests are demoted on day 1)
- Monitor the demotion-rate metric to detect over-aggressive caps
## Non-goals
- **Fixing the immediate timeout outage.** The dominant contributor per #171671 is the benchmark-retry mechanism (#170652) plus GCE quota. This work is structural mitigation, not the answer to today's overruns.
- **Scheduling / runner orchestration.** Selection picks *what* to run; scheduling picks *when/where*. The factors above (parallelism, cluster reuse, packing) properly belong to the runner. Don't conflate.
- **VM/quota management.** The bigger root cause per #171671; this issue is one of several mitigations, not a replacement.
## Open questions
- **Which statistic does the cap read?** Avg is simple but fragile to outliers (one 90-min run on a normally-5-min test skews everything). Options: avg, median, P95, trimmed mean. Each has opposite failure modes — choosing the wrong direction could either over- or under-demote.
- **Default cap value (N).** Conservative starting point. Possibilities: fixed minutes, derived from suite percentile, per-suite tunable.
- **Slow-by-design escape hatch.** Some tests are meant to be long (compat matrices, end-to-end scenarios). Options: explicit allowlist, a `TestSpec.IgnoreDurationCap` flag, or suite-scoped cap that exempts certain suites.
## Related
- #171671 — broader CI overrun problem
- #169712 — migration of test selection from Snowflake to CRDB (provides the duration data this cap needs)
Epic: none
Jira issue: CRDB-64872
Contributor guide
Research direction
Start at the Go test-selector pass after Categorize, then trace aggregateHistory and the existing duration fields: roachtest_runs.duration_ms, aggregate.totalDuration, aggregate.totalSuccessful, and TestDetails.AvgDurationInMillis. Resolve the statistic, default cap, escape hatch, and feature-flag choices; done means discretionary tests can be demoted without overriding RecentFailure, Preempt, UnknownNotFromSelector, or Stale, with a demotion metric exposed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- testing-qa, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100