-
Notifications
You must be signed in to change notification settings - Fork 4.1k
util/must: improve Expensive build tag gating #107425
Description
In #106508, must.Expensive() assertions are gated on the race build tag via util.RaceEnabled. We should reconsider this. Some considerations:
-
They should not be enabled in any binaries by default.
-
They should be possible to enable when building binaries.
-
They should never be enabled in benchmarks (neither roachperf nor go bench).
-
They should be enabled in as many tests as possible.
-
They may cause timing-sensitive tests to flake. If they do, we must be able to skip certain tests under expensive assertions, similarly to
skip.UnderRaceandskip.UnderDeadlock. -
Pebble's invariant assertions should be enabled via the same mechanism. They are currently gated on
invariantsorracebuild tags. -
We may need two degrees of expensive assertions (moderate and very), similarly to how we currently gate some on
crdb_testand others onrace.
Some options:
-
Gate them on
crdb_test, currently used for all Go tests.- May violate 5: we can't exclude tests from
crdb_test(they would never run). But maybe it won't be needed. - May violate 2 and 4: does it make sense to build binaries with
crdb_test? If so, can we use them in roachtests? - May violate 7: there only one degree of expensive assertion.
- May violate 5: we can't exclude tests from
-
Gate them on
invariants, a new tag for CRDB, but already used by Pebble. Would need a new nightly run (possibly both for Go tests and roachtests).- May violate 4: depends on how widely we enable them, e.g. new nightly run, nightly stress run, CI run, roachtest run, etc.
-
Gate them on
race, currently used for Go tests under race detector.- Violates 2: nodes are too slow to run under
race. - Violates 4: only enabled in
racebuilds, where many tests are skipped, and can't be used in roachtests.
- Violates 2: nodes are too slow to run under
I think I'm leaning towards trying 1 first, and if it doesn't work because too many timing-sensitive tests start flaking, do 2. To determine this quickly, we can enable Pebble's invariant tests under crdb_test, and use Expensive for some of the most expensive assertions in our hottest code paths (e.g. MVCC iterators).
Jira issue: CRDB-30035