cockroachdb / cockroachdb/cockroach
roachtest: replace Snowflake-based test selection with Go-side selection backed by a CRDB cluster
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
### Background
Today, nightly roachtest selection (`pkg/cmd/roachtest/testselector`) works by running `snowflake_query.sql` against `DATAMART_PROD.TEAMCITY` and trusting the resulting `selected/avg_duration/last_failure_is_preempt` columns. The host-side code in `updateSpecForSelectiveTests` then layers a 35%-of-stable-tests rule on top.
This shape has caused a string of operational issues:
- **#155343 (bootstrapping):** a freshly cut release branch has no rows in the `BUILDS` CTE, so the query returns zero rows, the host-side map is empty, and `testShouldBeSkipped` returns false for every spec. All tests run for ~20 nightly cycles.
- **Mass-add regressions:** when a wave of new tests appears (e.g., new perf variants), the `first_run > -20d` predicate flags all of them, starving the 35% bucket and again ballooning the run.
- **Snowflake outages stall CI.** We added a random-35% fallback for query errors and a `recover()` for a `gosnowflake` nil-rows panic on deadline; both are workarounds for an external dependency we don't actually need on the test path.
- **Selection logic is split across SQL and Go**, hard to unit-test in isolation, and any rule change requires a Snowflake round-trip to validate.
### Goal
Move the selection decision entirely into Go, and persist the underlying test-run history in a CockroachDB Cloud instance we own. Result: a pure, unit-testable selection function; no external SQL warehouse on the critical path; the bootstrapping bug becomes a deterministic input case; we get to dogfood CRDB.
### Proposed design
**1. Pure-Go selection.** Replace the SQL `case when ...` with a function:
```go
func Categorise(history []TestRun, opts Options) []TestDetails
```
`history` is a flat slice of `(test_name, started_at, status, details, ignore_details, duration_ms)` rows. `Categorise` aggregates per test, applies the rules (recent failure / new test / stale test / preemption / unknown-status), and returns the same `[]TestDetails` shape callers expect today. No I/O.
**2. History provider interface.**
```go
type HistoryProvider interface {
// Recent returns runs in [now-window, now] for (branch, suite, cloud).
Recent(ctx context.Context, branch, suite string, cloud spec.Cloud, window time.Duration) ([]TestRun, error)
// Record persists the result of a single test run.
Record(ctx context.Context, run TestRun) error
}
```
Two implementations:
- `crdbHistory` — backed by a CRDB Cloud cluster we own.
- `snowflakeHistory` — temporary, wraps a simplified "raw rows" version of the existing query so we can ship the refactor before the CRDB cluster is wired up.
**3. CRDB schema (sketch).**
```sql
CREATE TABLE roachtest_runs (
branch STRING NOT NULL,
suite STRING NOT NULL,
cloud STRING NOT NULL,
test_name STRING NOT NULL,
build_id INT8 NOT NULL,
started_at TIMESTAMPTZ NOT NULL,
finished_at TIMESTAMPTZ,
status STRING NOT NULL, -- SUCCESS / FAILURE / UNKNOWN
duration_ms INT8,
details STRING,
ignore_details STRING,
PRIMARY KEY (branch, suite, cloud, test_name, started_at DESC)
);
-- TTL ~60d, plenty for a 30d selection window plus headroom.
```
The PK is chosen so the read path (`Recent` for one (branch, suite, cloud)) is a single contiguous scan. Writes are append-only.
**4. Bootstrapping.** Make the empty-history case explicit in `Categorise`:
- If the branch has no rows: read master's history and use it.
- If master is also empty (cold start): deterministic shuffle that respects the "every test runs at least once over a 3-day window" invariant — same shape as the current random-35% fallback but seeded by date so re-runs are reproducible.
Both behaviors are pure-function inputs, exercised by unit tests.
**5. Mass-add safety valve.** Cap the "newly-added" bucket at, say, 25% of the run budget. Excess new tests are rotated in over subsequent days. Avoids the perf-variant cliff.
**6. Invariant alerting (#130315).** The whole point of selection is "every active test runs at least once per N days" (currently 7). Today nothing actively verifies this — selector bugs (#155343 being a recent example) only surface when someone notices runtime ballooning or a test going quiet. The query itself is trivial in either system, but four things make it materially easier on a CRDB store we own:
- **Registry visibility.** Snowflake's `TESTS` only knows about tests that have *ever* produced a row. A test that's in the registry but has never been selected to run — exactly the case we want to alert on — is invisible. With our own writer, the runner can record registry-known-but-not-run tests directly, or the alert can diff against the registry without a second cross-system pipeline.
- **Write path we own.** Snowflake is downstream of a TC→Snowflake ETL owned by another team. Anything we want to track that isn't already in TC's test result rows (e.g., "this run was selected by the selector vs. forced by an override") requires cross-team schema work. With CRDB, the runner writes whatever fields the alert needs.
- **Freshness.** The TC→Snowflake ETL runs on its own cadence (hours), so an alert built on it is always slightly stale and a flaky ETL day creates false positives. Direct CRDB writes are real-time.
- **Ownership boundary.** A scheduled query + Slack hook on Snowflake means another job runner with the RSA-key service account, owned by whichever team owns Snowflake access. With CRDB owned by TestEng, the alert lives in the same blast radius as the rest of the selector.
### Phasing
Each phase is independently shippable.
1. **Extract pure `Categorise`.** No behavior change. Keep the existing query, but reduce it to raw rows; move all `case when` logic into Go. Add table-driven tests covering: recent failure, new test, stale test, preemption, unknown-status, **empty history**, **mass-add wave**. Closes the testability gap and unblocks #155343 by fixing the bootstrap branch in Go.
2. **Stand up the CRDB cluster + schema.** Provision via CC, add migration tooling, document credentials handling for TC. No production traffic yet.
3. **Dual-write.** Roachtest writes every test result to CRDB at end-of-run, in addition to whatever TC/Snowflake already captures. Read path still Snowflake. Lets us validate ingestion volume and query latency against real load.
4. **Cut over reads.** Once CRDB has ≥30d of coverage on master + active release branches, switch `HistoryProvider` to `crdbHistory`. Keep Snowflake as a fallback for one release cycle.
5. **Remove Snowflake dependency.** Delete `snowflake_query.sql`, the gosnowflake driver, the RSA key handling, and `SNOWFLAKE_*` TC env vars.
6. **Wire up the staleness alert (#130315).** Periodic job (TC scheduled build or a small cron in our infra) that queries `roachtest_runs` for tests with no execution in >7d on master / each active release branch, and posts to `#test-eng` with the offending list. Implemented after cutover so the data is authoritative.
### Non-goals
- Changing the selection *heuristic* itself (failure / new / stale / preempt). That can evolve in follow-ups once the rules live in Go.
- Replacing TeamCity as the source of truth for build/test events.
- Cross-branch analytics / dashboards. Snowflake stays for human-facing analytics; this is only about the selector's hot path.
### Risks
- **CRDB cluster reliability becomes CI-critical.** Mitigated by: (a) the host-side fallback to a deterministic-shuffle selection when the provider errors, same shape we already do for Snowflake errors; (b) CRDB cluster sizing for the small read load (one query per nightly run per branch).
- **Backfill at cutover.** First read after the switch needs ≥30d of history. Either backfill from Snowflake once, or run dual-write long enough.
- **Selection drift during phase 1.** Reducing the SQL to raw rows could change ordering or rounding. Add a one-shot diff test comparing old vs. new categorisation for a recent week to bound the diff.
### Related
- Bug this unblocks: #155343
- Open partial-fix that targets the same bug at the SQL/host layer: #164842 (will be superseded by phase 1)
- Active alerting on the "every test runs within 7d" invariant: #130315 (becomes straightforward once history lives in CRDB)
- Epic: CRDB-55391
Jira issue: CRDB-63614
Contributor guide
Assessment
This issue has not been assessed yet.