roachtest: Add sstable-corruption roachtest

[itsbilal]

Adds a new roachtest, sstable-corruption/table, that
imports TPCC, then finds an sstable that contains
table keys (which are replicated) on one node, then
calls dd to write random bytes into the middle
of them. It then checks that the corrupt node
either crashes on restart, or crashes after the
tpcc workload is run on it.

Informs #67568.

Release note: None.

[cockroach-teamcity]

This change is 

[itsbilal]

TFTR! I did the monitor refactor and got rid of the channel. The test also passes more reliably now, as we're no longer tied to the more finicky output of NumExpectedDeaths.

[tbg]

after comments

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @stevendanna)

---

pkg/cmd/roachtest/tests/sstable_corruption.go, line 46 at r2 (raw file):

			return nil
		})
		m.Wait()

Could you indent this whole block so that m is in a nested scope?

{
  m := ...
  ...
  m.Wait()
}

This prevents reuse of monitors, which has lots of snags (actually using just a single monitor already unfortunately has snags :sadface:)

---

pkg/cmd/roachtest/tests/sstable_corruption.go, line 47 at r2 (raw file):

		})
		m.Wait()
		m.ExpectDeaths(3)

This line isn't doing anything, since you're not using this m any more and it also isn't watching the nodes any more (since Wait() returned).

---

pkg/cmd/roachtest/tests/sstable_corruption.go, line 88 at r2 (raw file):

		m.Go(func(ctx context.Context) error {
			_ = c.RunE(ctx, workloadNode, "./cockroach workload run tpcc --warehouses=100 "+
				fmt.Sprintf("--tolerate-errors --duration=%s", 2*time.Minute))

nit: unusual use of Sprintf here, why don't you Sprintf the entire args if you're going to do it to part of it?

---

pkg/cmd/roachtest/tests/sstable_corruption.go, line 89 at r2 (raw file):

			_ = c.RunE(ctx, workloadNode, "./cockroach workload run tpcc --warehouses=100 "+
				fmt.Sprintf("--tolerate-errors --duration=%s", 2*time.Minute))
			return nil

// Don't return an error from the workload. We want outcome of WaitE to be determined by the monitor noticing that a node died. The workload may also fail, despite --tolerate-errors, if a node crashes too early.

---

pkg/cmd/roachtest/tests/sstable_corruption.go, line 92 at r2 (raw file):

		})
		require.Error(t, m.WaitE())
		_ = c.WipeE(ctx, t.L(), c.Node(corruptNode))

// Exempt corrupted node from roachtest harness' post-test liveness checks.

[itsbilal]

TFTR!

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

---

pkg/cmd/roachtest/tests/sstable_corruption.go, line 46 at r2 (raw file):

Previously, tbg (Tobias Grieger) wrote…

Could you indent this whole block so that m is in a nested scope?

{
  m := ...
  ...
  m.Wait()
}

This prevents reuse of monitors, which has lots of snags (actually using just a single monitor already unfortunately has snags :sadface:)

I was already putting all uses of m in this one nested scope, but I outdented the lines below that aren't using m to make it clear. Hope that works!

[tbg]

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

---

pkg/cmd/roachtest/tests/sstable_corruption.go, line 46 at r2 (raw file):

Previously, itsbilal (Bilal Akhtar) wrote…

I was already putting all uses of m in this one nested scope, but I outdented the lines below that aren't using m to make it clear. Hope that works!

The goal is that m goes out of scope once m.Wait() returns. Otherwise I'm just nervously scanning everything after that that has m visible for errant uses.

[itsbilal]

Oops, forgot a push. Here we go.

[tbg]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @stevendanna)

[itsbilal]

TFTR! Latest push now corrupts 6 table SSTs on every node, not just on one node. Should eliminate the flakiness.

[itsbilal]

bors r=tbg

[craig]

Build succeeded:

• GitHub CI (Cockroach)