release-22.2: roachtest/cdc/mixed-versions: allow for changefeed failures

[jayshrivastava]

This test may flake due to the upgrade from 22.1->22.2. The
test asserts a changefeed remains running by checking for
resolved timestamps being emitted on a regular basis. The
problem with this is that, during the rolling upgrade,
the changefeed may fail with a "draining" error.
This issue is fixed in 22.2 onwards by treating all errors
as retryable.

Rather than skipping this test because 22.1 is EOLed, it is
preferable to still run this test regularly because it tests
22.2 functionality. This change adds a fix where the test
will poll the changefeed every 1s and recreate it if it fails.

Closes: #106878
Release note: None
Epic: None

[cockroach-teamcity]

This change is 

[renatolabs]

but the changefeed may fail with a "draining" error

In a real deployment, what is an operator expected to do when this happens? Restart the changefeed? If we do that (or whatever the operator is expected to do), would the test pass?

I'm wondering if we can detect this class of error and "recover" automatically. Or, if that's not possible, t.Skip the run.

While 22.1 is EOLed, 22.2 is still supported and receives the occasional backport, so I'm thinking if there's a possibility of not disabling the test altogether.

[jayshrivastava]

@renatolabs I agree. I added some logic to re-create the changefeed. This should be fine since the test only checks for resolved timestamps and doesn't care about the individual changefeed job.

[jayshrivastava]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @renatolabs)

---

pkg/cmd/roachtest/tests/mixed_version_cdc.go line 359 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

Let's replace this with cmvt.monitor.Go() instead. go is discouraged in roachtests as a panic in the goroutine would lead to the entire roachtest process crashing (unless you add a recover to all of them, which is easy to forget).

Done.

I also added this terminateWatcherCh that is closed by the tester near the end of the test. The watcher only terminates when it checks for the context is being cancelled. After the context is cancelled but before the watcher sees the context being cancelled, it may encounter an error trying to query the DB. If this happens, it would return an error and fail the test. Using the channel should avoid this problem.

---

pkg/cmd/roachtest/tests/mixed_version_cdc.go line 365 at r2 (raw file):

			cmvt.terminateWatcherCh = make(chan struct{})
		}
		cmvt.monitor.Go(func(ctx2 context.Context) error {

One goroutine per changefeed is not ideal, but this test only creates one, so its ok. This is only in 22.2 and I doubt we will update this test to create a large number of changefeeds anyways.

[renatolabs]

Approving as the changes LGTM. Thanks for working on this!

If the test does flake as I suspect, I'll let you decide how you want to deal with it (manually as it happens, in this PR, as a followup).