Fixing backup tests flakiness

[rsajwani]

Description

Backup Tests TestBackupMysqlctld/TestPrimaryBackup is flaky. Here is one such instance of failure

https://github.com/vitessio/vitess/actions/runs/4477008834/jobs/7868010386

The failure is coming from this piece of test code

// Perform PRS to demote the primary tablet (primary) so that we can do a restore there and verify we don't have the
	// data from after the older/first backup
	err = localCluster.VtctlclientProcess.ExecuteCommand("PlannedReparentShard", "--",
		"--keyspace_shard", shardKsName,
		"--new_primary", replica2.Alias)
	require.Nil(t, err)

	// Delete the current primary tablet (replica2) so that the original primary tablet (primary) can be restored from the
	// older/first backup w/o it replicating the subsequent insert done after the first backup was taken
	err = localCluster.VtctlclientProcess.ExecuteCommand("DeleteTablet", "--", "--allow_primary=true", replica2.Alias)
	require.Nil(t, err)
	err = replica2.VttabletProcess.TearDown()
	require.Nil(t, err)

	// Restore the older/first backup -- using the timestamp we saved -- on the original primary tablet (primary)
	err = localCluster.VtctlclientProcess.ExecuteCommand("RestoreFromBackup", "--", "--backup_timestamp", firstBackupTimestamp, primary.Alias)
	require.Nil(t, err)

The reason is we have introduced VTOrc in the test which tries to fix Primary when it is down. When we delete the tablet, VTorc tries to elect a new primary. If it elect replica1 as new primary then we are good but if it elects primary as new one then RestoreFromBackup fails.

Related Issue(s)

closes #12687

Solution

There were few ways to resolve it.

1. We could have remove VTorc entirely from backup tests
2. We disable VTorc in that specific test which is failing because of VTOrc intervention.
3. We can choose the right replica at the point and use that for restore API.

I went with option#2, as it looked neat and doesn't result in a lot of changes in code.

Testing

Ran test few times locally and then on github action. It looked much stable now.

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on the CI
• Documentation was added or is not required

Deployment Notes

[rsajwani]

This was creating multiple instances of VTorc during testing. I don't think this was intentional. If we want to test multi instance VTorc then we should write a separate test for it. This is backup test and should try to test backup functionality.

[GuptaManan100]

Yes, this wasn't intentional.

[deepthi]

I don't understand what the test was trying to do with deleting replica2 right after making it the primary. Ideally we rewrite the test to make sense. However, it is valid to say that we don't want to run vtorc recoveries in backup/restore tests, so this PR is still good.

[GuptaManan100]

LGTM!