cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.15k stars 3.81k forks source link

kvserver: test *Replica like *raft.RawNode #105177

Open tbg opened 1 year ago

tbg commented 1 year ago

There are lots of "situations of interest" that we attempt to exercise by setting up a TestCluster and an often complicated web of interceptors, all to enact a desired "interesting" state of the system. This often involves a delicate dance between wanting to get into this state quickly (not adding a slow test) but also not having it flake due to excessively low timeout (or because some unexpected moving part interferes with the test). There is a time and place for such a test, but we are currently forced into them because we often lack a direct way to test at more intermediate levels. This is a problem.

We've seen the benefits of architecting for testability in etcd-io/raft (applied to *RawNode). A *RawNode has no concurrency, i.e. all access to it is serial. A set of *RawNodes forms a raft group. The test harness controls in which order messages get delivered and processed. Verbose logs that result from handling messages become part of the datadriven output. Everything is deterministic. The test harness (introduced by us!) combined with the concurrency-free, deterministic architecture has significantly improved the ease and quality of testing^1 in etcd/raft.

It stands to reason that the same approach could yield similar benefits if applied to at least the lower slices in the Replica.Send stack.

Squinting a little (lot), a *Replica that is only accessed in a single-threaded way is just a *RawNode: it can receive various kinds of messages (e.g. BatchRequest, raftpb.Message, etc), which it handles primarily via the raft handling loop; a set of Replicas (for the same range) is like a Raft group.

Not all of the surface of Replica lends itself to this approach (after all, some bugs will only surface with concurrency on the Replica, some requests today trigger work on a goroutine, etc) but my intuition and experience is that with each slice of the stack that we make accessible to the above strategy will confer a significant improvement of quality.

This broad suggestion leaves much to the imagination and perhaps doesn't make it clear that in its entirety, this is a significant undertaking. However, small slices of the stack can be tackled in a bottom-up order and in fact, this is already happening and has been well-received by all participants:

If we could unit test log application, conceivably it would be much easier to do projects such as https://github.com/cockroachdb/cockroach/pull/97779 because we'd be able to directly explore the edge cases in this code, whereas currently we have to rely on either contrived, hard-to-maintain (and non-exhaustive) tests, plus randomized tests (which come with their own challenges).

A first "slice" to tackle could be improving the test in https://github.com/cockroachdb/cockroach/pull/104401: it creates an "interesting" raft log programmatically (without even a *Replica), but currently jumps through some hoops to do so. We could fill these gaps by successive small refactors, until we have the ability to create arbitrary Raft logs.

If we then tackled https://github.com/cockroachdb/cockroach/issues/75729, i.e. gain the ability to also "apply" these logs in a setting with few moving parts, we are well underway to being able to comprehensively test the apply pipeline, which is just not possible today.

We could then work on our ability to instantiate a *Replica "without moving parts" (i.e. no queues, Raft scheduler, etc) and start writing deterministic tests.

Jira issue: CRDB-28912

Epic CRDB-39898

blathers-crl[bot] commented 1 year ago

cc @cockroachdb/replication