roachtest: improve testing of mixed-version clusters

[andreimatei]

As it became apparent in #15819, we don't have tests for the upgrade of a cluster from one version to another. I believe we have a "mixed version" acceptance test, but I think all it does is it writes data using an old version and reads it with a new one.

We should have a test checking the interoperability of versions in a running cluster. We should test that as many operations as possible work when the operation involves nodes on different versions. Examples of things to test: DistSQL reads, replication, lease transfers.

In the specific case of #15819, what would have helped is one with a (lease-related?) Raft command being proposed by the new version and applied by the old one.

Jira issue: CRDB-6080

[tbg]

In the specific case of #15819, what would have helped is one with a (lease-related?) Raft command being proposed by the new version and applied by the old one.

Actually I think a non-lease-but-still-state-updating one. For example, any replica change.

[tbg]

In particular, it seems that this should've fired by having an old node join a new (under-replicated) cluster.

[cuongdo]

@tschottdorf I believe you're already working on some form of this for migrations?

[tbg]

Yeah, this seems like it'll naturally fall out of the cluster version migration tests.

[tbg]

Moving to 1.2.

[tbg]

Yeah, this seems like it'll naturally fall out of the cluster version migration tests.

And so it did, though we could do more.

I'll unassign myself and assign @cuongdo to hold on to this issue with the aim that some real manpower is allotted to it in the future. Running comprehensive stability tests requires more effort than I can provide ad-hoc.

[tbg]

See also #18811.

[tbg]

• augment the TestVersionUpgrade (acceptance) test so that it (while running in mixed version mode at the beginning) tests various migrations (such as ClearRange, AdjustStats, ...) to make sure that they don't have unintended side effects
• add commentary in cockroach_versions.go to stipulate this for future migrations
• update the test so that it starts the "old" node with 1.1.0 instead of 1.0.x
• update the test so that it always bumps to the latest available cluster version possible (available via SELECT crdb_internal.node_executable_version() against a node running the latest code).

[bdarnell]

We broke version upgrades with #22487, but TestVersionUpgrade only became flaky instead of failing consistently. We need to figure out how to make this test detect broken upgrades more reliably. (I think the issue in this case was whether the processes live long enough to send multiple liveness heartbeats).

[benesch]

Yeah, I think the real solution is to take one of these roachtest nightly tests that's serving kv traffic and bump versions every, say, 30m instead of as quickly as possible.

[petermattis]

@benesh We'd need to add a facility to roachtest for downloading additional binaries, probably via util/binfetcher. Shouldn't be difficult.

[bdarnell]

I'd like to get something in CI for every PR, not just a nightly. CI should have blocked the merge of the checksum PR instead of leaving us with a debugging puzzle with a day's worth of possible PRs to blame.

[benesch]

For the per-PR acceptance tests, a time.Sleep(10*time.Second) in between version bumps would go a long way, but we still need something more serious to catch incompatibilities in e.g. the DistSQL layer.

[tbg]

We have rudimentary coverage here. Both roachtest and CI have tests that migrate a cluster through multiple versions, with some foreground activity thrown in. What we need to achieve over time is to exercise more interesting code paths in these tests. That is, we want to be decommissioning, running DistSQL queries, SCRUB, compaction queue activity, all workload generators, ... and fail the test if anything goes wrong or becomes unexpectedly slow. This is a bit of a herculean task, so I think it makes sense to split this into SQL and core components. For the core components, we can audit the introduced cluster versions and retrofit test coverage where appropriate, and make sure that any new cluster version comes with an addition to the roachtest and, if possible, CI.

[tbg]

@petermattis this is now a meta-issue and more of a request for process/culture/infrastructure. Could you decide what to do with it? At the very least, we should be running a weekly mixed-version roachtest (#31223) but the problem is the missing diversity of workloads.

[petermattis]

Ack. Let me think on this.

[github-actions]

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

[andreimatei]

Testing have improved over the years. Closing.