util/uuid: move to a non-deprecated randomness source

[rafiss]

This usage of math/rand is deprecated as of go1.20:

cockroach/pkg/util/uuid/uuid_wrapper.go

Lines 112 to 118 in f80ec9c

	// defaultRandReader is an io.Reader that calls through to "math/rand".Read
	// which is safe for concurrent use.
	type defaultRandReader struct{}
	func (r defaultRandReader) Read(p []byte) (n int, err error) {
	return rand.Read(p)
	}

https://pkg.go.dev/math/rand#Read

Read generates len(p) random bytes from the default Source and writes them into p. It always returns len(p) and a nil error. Read, unlike the Rand.Read method, is safe for concurrent use.

Deprecated: For almost all use cases, crypto/rand.Read is more appropriate.

However, using crypto/rand has previously been shown to be a performance issue. See #30236 (comment) for that investigation.

For now, we will stick with the deprecated usage, but eventually we will have to update it.

Also note that go 1.21 (which we do not use as of 2023-09-13) includes this change, which improves the performance of math/rand: golang/go@0b9974d

We may want to use something similar to that, since UUID generation is something that can happen with high concurrency. Before the aforementioned go1.21 change, math/rand.Read was synchronized around a single mutex.

Jira issue: CRDB-31518

[bdarnell]

This is subtle and poorly documented. math/rand is not deprecated; only math/rand.Read is deprecated (and the main concern appears to be that it's too easy for goimports to pick the wrong package when it sees rand.Read()). Generating a pair of int64s from math/rand is not deprecated. Much more detail about the math/rand interfaces is available in golang/go#61716.

Also note that the uuid package already supports both crypto/rand and math/rand. uuid.MakeV4 uses crypto/rand, while uuid.FastMakeV4 uses math/rand.

[bdarnell]

We may also want to reevaluate the use of crypto/rand. We saw the performance issue in 2018; but there have been improvements in the meantime.

[rafiss]

@srosenberg saw worse performance with crypto/rand (due to waiting for the entropy pool) in August 2023: https://cockroachlabs.slack.com/archives/C4X2J0RH6/p1692725321047249

[rafiss]

Also note that the uuid package already supports both crypto/rand and math/rand. uuid.MakeV4 uses crypto/rand, while uuid.FastMakeV4 uses math/rand.

I'm aware, but this issue is just about deciding what to do with the current math/rand usage.

Generating a pair of int64s from math/rand is not deprecated.

That's a good point, and it looks like we can update our code to generate ints instead of bytes, with a fair bit of refactoring. I can take that on for a Friday project, and if I have trouble, I'll leave this issue to KV as per https://cockroachlabs.slack.com/archives/C05B1BPMJLE/p1694633033861299?thread_ts=1694632185.354129&cid=C05B1BPMJLE

[rafiss]

I tried implementing this by generating random uint64s, and there was a ~28% performance hit. See #110656

I am unassigning myself for now and leaving this with the KV team as per cockroachlabs.slack.com/archives/C05B1BPMJLE/p1694633033861299?thread_ts=1694632185.354129&cid=C05B1BPMJLE

There isn't any particular urgency to this - rand.Read will likely be around for a while.