internal/metamorphic: generate keys with suffixes

[jbowens]

Generate keys with human-readable suffixes like '@3'. The key manager is
extended to track unique in-use prefixes as well. A fixed portion of new keys
are generated using an existing prefix but a new suffix.

New keys' suffixes are generated using a skewed-latest distribution, mimicking
MVCC timestamps

[cockroach-teamcity]

This change is 

[nicktrav]

Nice.

Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @itsbilal, @jbowens, and @sumeerbhola)

---

internal/metamorphic/generator.go, line 167 at r1 (raw file):

		return keys[g.rng.Intn(len(keys))]

	case len(keys) > 0 && g.rng.Float64() > g.cfg.newKeyExistingPrefix:

This reads as 75% new keys with existing prefixes. Is that what we want?

---

internal/metamorphic/key_manager.go, line 186 at r1 (raw file):

// addNewKey adds the given key to the key manager for global key tracking.
// Returns false iff this is not a new key.
func (k *keyManager) addNewKey(key []byte) bool {

Can we add to the test cases for addNewKey to cover the prefix / suffix case, and also cover (*keyManager).prefixes() and (*keyManager).prefixExists()?

[jbowens]

Reviewable status: 6 of 10 files reviewed, 1 unresolved discussion (waiting on @itsbilal, @nicktrav, and @sumeerbhola)

---

internal/metamorphic/generator.go, line 167 at r1 (raw file):

Previously, nicktrav (Nick Travers) wrote…

This reads as 75% new keys with existing prefixes. Is that what we want?

Good catch — not what I intended. I turned this into a newPrefix configuration parameter and flipped this case and the default.

---

internal/metamorphic/key_manager.go, line 186 at r1 (raw file):

Previously, nicktrav (Nick Travers) wrote…

Can we add to the test cases for addNewKey to cover the prefix / suffix case, and also cover (*keyManager).prefixes() and (*keyManager).prefixExists()?

Done

[nicktrav]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itsbilal, @jbowens, and @sumeerbhola)

---

internal/metamorphic/generator.go, line 167 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

Good catch — not what I intended. I turned this into a newPrefix configuration parameter and flipped this case and the default.

It looks like this block will only be entered 25% of the time. The comment in the config mentions we want 75% new keys and suffixes. Flip inequality?

[sumeerbhola]

Reviewed 6 of 9 files at r1, 4 of 4 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @itsbilal, @jbowens, and @nicktrav)

---

internal/metamorphic/generator.go, line 167 at r1 (raw file):

Previously, nicktrav (Nick Travers) wrote…

It looks like this block will only be entered 25% of the time. The comment in the config mentions we want 75% new keys and suffixes. Flip inequality?

yes, should be <= since newPrefix is the probability of a new prefix.

---

internal/metamorphic/generator.go, line 205 at r2 (raw file):

			// If the generated key already existed, increase the suffix
			// distribution to make a generating a new user key with an existing
			// prefix more likely.

We also need this for the case where we've already generated all the suffixes for the current prefixes, yes?

---

internal/metamorphic/generator.go, line 211 at r2 (raw file):

}

func (g *generator) randKey(dst []byte, minPrefixLen, maxPrefixLen, suffix int) []byte {

can you add a code comment that this is only a helper for randKeyHelper. Future changes should not accidentally undo the work we did to separate the read and write paths via randKeyToRead and randKeyToWrite.
And maybe rename it randKeyWithSuffixHelper.

---

internal/metamorphic/generator.go, line 222 at r2 (raw file):

	dst = resizeBuffer(dst, n, suffixLen)
	g.fillRand(dst[:n])
	if suffixLen > 0 {

So the 0 suffix is not written? Is this to test a mix of prefix with no suffix and prefix with suffix? A code comment would help.

[jbowens]

Reviewable status: 8 of 10 files reviewed, 4 unresolved discussions (waiting on @itsbilal, @nicktrav, and @sumeerbhola)

---

internal/metamorphic/generator.go, line 167 at r1 (raw file):

Previously, sumeerbhola wrote…

yes, should be <= since newPrefix is the probability of a new prefix.

flipped the cases, to keep the same structure as the first case

---

internal/metamorphic/generator.go, line 205 at r2 (raw file):

Previously, sumeerbhola wrote…

We also need this for the case where we've already generated all the suffixes for the current prefixes, yes?

I think that's the same case as the one handled here? This is the only case where we generate a new user key using an existing prefix but a suffix previously unused with the prefix.

---

internal/metamorphic/generator.go, line 211 at r2 (raw file):

Previously, sumeerbhola wrote…

can you add a code comment that this is only a helper for randKeyHelper. Future changes should not accidentally undo the work we did to separate the read and write paths via randKeyToRead and randKeyToWrite.
And maybe rename it randKeyWithSuffixHelper.

Done.

---

internal/metamorphic/generator.go, line 222 at r2 (raw file):

Previously, sumeerbhola wrote…

So the 0 suffix is not written? Is this to test a mix of prefix with no suffix and prefix with suffix? A code comment would help.

Yeah, that's right. Done.

[sumeerbhola]

Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed (commit messages unreviewed), 4 unresolved discussions (waiting on @itsbilal, @nicktrav, and @sumeerbhola)

[jbowens]

TFTRs!

Reviewable status: 9 of 10 files reviewed, 4 unresolved discussions (waiting on @itsbilal, @nicktrav, and @sumeerbhola)