storage: add setting to seed up consistency checks in tests

[itsbilal]

Previously, we'd fall back to the 3 second consistency checker EventuallyFileOnlySnapshot (EFOS) wait in roachtests, which was slowing all of
them down when we ran every replica through the consistency checker in post-test assertions. This change speeds up that consistency check in roachtest post-test assertions by flipping a new cluster setting to speed up EFOS waits for consistency checks after a roachtest finishes.

Epic: none
Unblocks #116330.

Release note: None

[cockroach-teamcity]

This change is 

[erikgrinaker]

Does this imply that running crdb_internal.check_consistency() is going to be generally slow?

[RaduBerinde]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @aadityasondhi, @DarrylWong, @itsbilal, and @srosenberg)

---

pkg/storage/engine.go line 638 at r1 (raw file):

	// argument specifies how long to wait for before attempting a flush to
	// force a transition to a file-only snapshot.
	WaitForFileOnly(context.Context, time.Duration) error

[nit] I'd name give the parameter a useful name, maybe gracePeriodBeforeFlush. Ideally we'd plumb that through the implementation - dur is not useful, and in fact is misleading: a generic duration argument to a method like Wait usually means that we'll give up waiting after that duration.

[itsbilal]

TFTR!

@erikgrinaker that's correct but only if we're running that command with little/no background writes happening. Flushes will result in forward progress on WaitForFileOnly without the 3 second wait.

In general when we were making this change, we were under the impression that consistency checks are usually a background operation. If this is an undesirable knock-on effect of this change we can look into ways to speed up consistency checks too.

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @aadityasondhi, @DarrylWong, and @srosenberg)

---

pkg/storage/engine.go line 638 at r1 (raw file):

Previously, RaduBerinde wrote…

[nit] I'd name give the parameter a useful name, maybe gracePeriodBeforeFlush. Ideally we'd plumb that through the implementation - dur is not useful, and in fact is misleading: a generic duration argument to a method like Wait usually means that we'll give up waiting after that duration.

Done.

[itsbilal]

bors r=RaduBerinde

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[erikgrinaker]

we were under the impression that consistency checks are usually a background operation. If this is an undesirable knock-on effect of this change we can look into ways to speed up consistency checks too.

That's generally true, except for crdb_internal.check_consistency(). I doubt it's used much outside of tests. We could consider disabling EFOS for foreground consistency checks.

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

[nvanbenschoten]

That's generally true, except for crdb_internal.check_consistency(). I doubt it's used much outside of tests. We could consider disabling EFOS for foreground consistency checks.

We do know at this level whether we're running on behalf of the consistency queue or on behalf of the crdb_internal.check_consistency() builtin. The mode param will have a value of ChecksumMode_CHECK_VIA_QUEUE when from the queue and either ChecksumMode_CHECK_FULL or ChecksumMode_CHECK_STATS when manually run.

It would probably make sense to disable EFOS for both. If not, we should at least do it for ChecksumMode_CHECK_STATS, which doesn't iterate over the range and should be quick.

[nvanbenschoten]

I'll make the less contentious change for now. #117297.

[erikgrinaker]

blathers backport 23.2

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from f2ae78b to blathers/backport-release-23.2-116892: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.2 failed. See errors above.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[erikgrinaker]

Nevermind, I guess we didn't enable this by default for 23.2.