From 4ebfab045ed191b9ddf4ce141dae4eb33321ecd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Senart?= Date: Sat, 11 Jun 2022 16:47:41 +0200 Subject: [PATCH 1/6] README: improve documentation of entropy source Users of this library have opened many issues regarding the difficulty of choosing an entropy source that is safe for concurrent use. --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 630e0d2..4a2410c 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,13 @@ go get github.com/oklog/ulid/v2 An ULID is constructed with a `time.Time` and an `io.Reader` entropy source. This design allows for greater flexibility in choosing your trade-offs. -Please note that `rand.Rand` from the `math` package is *not* safe for concurrent use. -Instantiate one per long living go-routine or use a `sync.Pool` if you want to avoid the potential contention of a locked `rand.Source` as its been frequently observed in the package level functions. +Please note that `rand.Rand` from the `math` package is [*not* safe for concurrent use](https://github.com/golang/go/issues/3611), so consider the following options: + +1. Instantiate one per long living go-routine if your concurrency model permits — this option will result in no lock contention. + +2. For usage in short lived go-routines (e.g. HTTP handlers), use [`golang.org/x/exp/rand.Rand`](https://pkg.go.dev/golang.org/x/exp/rand#example-LockedSource) which is safe for concurrent use, but can result in a high amount of lock contention if many go-routines concurrently call `Read` on it. + +3. When lock contention is too high with the previous approach, use a `sync.Pool` of `rand.Rand` instances. ```go func ExampleULID() { From 9e77a89fab31ad454d30d342b5e21943862fbf86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Senart?= Date: Sat, 11 Jun 2022 18:29:39 +0200 Subject: [PATCH 2/6] ulid: add `DefaultEntropy()` and `Make()` This commit introduces a thread safe per process monotonically increase `DefaultEntropy()` function as well as an easy to use `Make()` function, aimed at users that want safe defaults chosen for them. --- ulid.go | 47 +++++++++++++++++++++++++++++++++++++++++++++-- ulid_test.go | 21 +++------------------ 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/ulid.go b/ulid.go index 9f98159..85675ce 100644 --- a/ulid.go +++ b/ulid.go @@ -23,6 +23,7 @@ import ( "math" "math/bits" "math/rand" + "sync" "time" ) @@ -121,6 +122,32 @@ func MustNew(ms uint64, entropy io.Reader) ULID { return id } +var ( + entropy io.Reader + entropyOnce sync.Once +) + +// DefaultEntropy returns a thread-safe per process monotonically increasing +// entropy source. +func DefaultEntropy() io.Reader { + entropyOnce.Do(func() { + rng := rand.New(rand.NewSource(time.Now().UnixNano())) + entropy = &LockedMonotonicReader{ + MonotonicReader: Monotonic(rng, 0), + } + }) + return entropy +} + +// Make returns an ULID with the current time in Unix milliseconds and +// monotonically increasing entropy for the same millisecond. +// It is safe for concurrent use, leveraging a sync.Pool underneath for minimal +// contention. +func Make() (id ULID) { + // NOTE: MustNew can't panic since DefaultEntropy never returns an error. + return MustNew(Now(), DefaultEntropy()) +} + // Parse parses an encoded ULID, returning an error in case of failure. // // ErrDataSize is returned if the len(ulid) is different from an encoded @@ -531,13 +558,29 @@ func Monotonic(entropy io.Reader, inc uint64) *MonotonicEntropy { m.inc = math.MaxUint32 } - if rng, ok := entropy.(*rand.Rand); ok { + if rng, ok := entropy.(rng); ok { m.rng = rng } return &m } +type rng interface{ Int63n(n int64) int64 } + +// LockedMonotonicReader wraps a MonotonicReader with a sync.Mutex for +// safe concurrent use. +type LockedMonotonicReader struct { + mu sync.Mutex + MonotonicReader +} + +func (r *LockedMonotonicReader) MonotonicRead(ms uint64, p []byte) (err error) { + r.mu.Lock() + err = r.MonotonicReader.MonotonicRead(ms, p) + r.mu.Unlock() + return err +} + // MonotonicEntropy is an opaque type that provides monotonic entropy. type MonotonicEntropy struct { io.Reader @@ -545,7 +588,7 @@ type MonotonicEntropy struct { inc uint64 entropy uint80 rand [8]byte - rng *rand.Rand + rng rng } // MonotonicRead implements the MonotonicReader interface. diff --git a/ulid_test.go b/ulid_test.go index e6c044a..3e4ec64 100644 --- a/ulid_test.go +++ b/ulid_test.go @@ -21,7 +21,6 @@ import ( "math" "math/rand" "strings" - "sync" "testing" "testing/iotest" "testing/quick" @@ -525,6 +524,7 @@ func TestMonotonic(t *testing.T) { }{ {"cryptorand", func() io.Reader { return crand.Reader }}, {"mathrand", func() io.Reader { return rand.New(rand.NewSource(int64(now))) }}, + {"poolrand", ulid.DefaultEntropy}, } { for _, inc := range []uint64{ 0, @@ -586,11 +586,8 @@ func TestMonotonicSafe(t *testing.T) { t.Parallel() var ( - src = rand.NewSource(time.Now().UnixNano()) - entropy = rand.New(src) - monotonic = ulid.Monotonic(entropy, 0) - safe = &safeMonotonicReader{MonotonicReader: monotonic} - t0 = ulid.Timestamp(time.Now()) + safe = ulid.DefaultEntropy() + t0 = ulid.Timestamp(time.Now()) ) errs := make(chan error, 100) @@ -630,18 +627,6 @@ func TestULID_Bytes(t *testing.T) { } } -type safeMonotonicReader struct { - mtx sync.Mutex - ulid.MonotonicReader -} - -func (r *safeMonotonicReader) MonotonicRead(ms uint64, p []byte) (err error) { - r.mtx.Lock() - err = r.MonotonicReader.MonotonicRead(ms, p) - r.mtx.Unlock() - return err -} - func BenchmarkNew(b *testing.B) { benchmarkMakeULID(b, func(timestamp uint64, entropy io.Reader) { _, _ = ulid.New(timestamp, entropy) From 5a0b4527ed9ac96bdfd4c466f6a42a99fe458a78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Senart?= Date: Sat, 11 Jun 2022 18:43:42 +0200 Subject: [PATCH 3/6] README: Update Usage with Make --- README.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 4a2410c..2f494a1 100644 --- a/README.md +++ b/README.md @@ -42,16 +42,25 @@ go get github.com/oklog/ulid/v2 An ULID is constructed with a `time.Time` and an `io.Reader` entropy source. This design allows for greater flexibility in choosing your trade-offs. -Please note that `rand.Rand` from the `math` package is [*not* safe for concurrent use](https://github.com/golang/go/issues/3611), so consider the following options: +If you want sane defaults, just use `ulid.Make()` which will produce a per process monotonically increasing ULID based on the current time that is safe for concurrent use. -1. Instantiate one per long living go-routine if your concurrency model permits — this option will result in no lock contention. +```go +func ExampleMake() { + fmt.Println(ulid.Make()) + // Output: 0000XSNJG0MQJHBF4QX1EFD6Y3 +} +``` + +Otherwise, you'll need to provide your own entropy source. Since `rand.Rand` from the `math` package is [*not* safe for concurrent use](https://github.com/golang/go/issues/3611), consider the following guidance: + +- Instantiate one `rand.Rand` per long living go-routine if your concurrency model permits — this option will result in no lock contention. -2. For usage in short lived go-routines (e.g. HTTP handlers), use [`golang.org/x/exp/rand.Rand`](https://pkg.go.dev/golang.org/x/exp/rand#example-LockedSource) which is safe for concurrent use, but can result in a high amount of lock contention if many go-routines concurrently call `Read` on it. +- For usage in short lived go-routines (e.g. HTTP handlers), use [`golang.org/x/exp/rand.Rand`](https://pkg.go.dev/golang.org/x/exp/rand#example-LockedSource) which is safe for concurrent use, but can result in a high amount of lock contention if many go-routines concurrently call `Read` on it. -3. When lock contention is too high with the previous approach, use a `sync.Pool` of `rand.Rand` instances. +- When lock contention is too high with the previous approach, use a `sync.Pool` of `rand.Rand` instances. This won't provide any benefits if you need to per process monotonic ULIDs, since using `LockedMonotonicReader` will synchronize all reads anyways. ```go -func ExampleULID() { +func ExampleMustNew() { t := time.Unix(1000000, 0) entropy := ulid.Monotonic(rand.New(rand.NewSource(t.UnixNano())), 0) fmt.Println(ulid.MustNew(ulid.Timestamp(t), entropy)) From 3b7efdef709fe3270aaa13f86e3011b62a4868e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Senart?= Date: Sat, 11 Jun 2022 18:47:06 +0200 Subject: [PATCH 4/6] fixup! format + add smoke test --- ulid_test.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ulid_test.go b/ulid_test.go index 3e4ec64..8cf5686 100644 --- a/ulid_test.go +++ b/ulid_test.go @@ -60,6 +60,11 @@ func TestNew(t *testing.T) { }) } +func TestMake(t *testing.T) { + t.Parallel() + t.Log(ulid.Make()) +} + func TestMustNew(t *testing.T) { t.Parallel() @@ -141,7 +146,7 @@ func TestRoundTrips(t *testing.T) { id == ulid.MustParseStrict(id.String()) } - err := quick.Check(prop, &quick.Config{MaxCount: 1E5}) + err := quick.Check(prop, &quick.Config{MaxCount: 1e5}) if err != nil { t.Fatal(err) } @@ -228,7 +233,7 @@ func TestEncoding(t *testing.T) { return true } - if err := quick.Check(prop, &quick.Config{MaxCount: 1E5}); err != nil { + if err := quick.Check(prop, &quick.Config{MaxCount: 1e5}); err != nil { t.Fatal(err) } } @@ -257,7 +262,7 @@ func TestLexicographicalOrder(t *testing.T) { top = next } - if err := quick.Check(prop, &quick.Config{MaxCount: 1E6}); err != nil { + if err := quick.Check(prop, &quick.Config{MaxCount: 1e6}); err != nil { t.Fatal(err) } } @@ -315,7 +320,7 @@ func TestParseRobustness(t *testing.T) { return err == nil } - err := quick.Check(prop, &quick.Config{MaxCount: 1E4}) + err := quick.Check(prop, &quick.Config{MaxCount: 1e4}) if err != nil { t.Fatal(err) } @@ -367,7 +372,7 @@ func TestTimestampRoundTrips(t *testing.T) { return ts == ulid.Timestamp(ulid.Time(ts)) } - err := quick.Check(prop, &quick.Config{MaxCount: 1E5}) + err := quick.Check(prop, &quick.Config{MaxCount: 1e5}) if err != nil { t.Fatal(err) } @@ -446,7 +451,7 @@ func TestEntropyRead(t *testing.T) { return eq } - if err := quick.Check(prop, &quick.Config{MaxCount: 1E4}); err != nil { + if err := quick.Check(prop, &quick.Config{MaxCount: 1e4}); err != nil { t.Fatal(err) } } @@ -462,7 +467,7 @@ func TestCompare(t *testing.T) { return a.Compare(b) } - err := quick.CheckEqual(a, b, &quick.Config{MaxCount: 1E5}) + err := quick.CheckEqual(a, b, &quick.Config{MaxCount: 1e5}) if err != nil { t.Error(err) } From ab6f4639ecfa0a34c24ce4c775d6aee21a651f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Senart?= Date: Sat, 11 Jun 2022 18:50:53 +0200 Subject: [PATCH 5/6] fixup! remove leftover --- ulid_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ulid_test.go b/ulid_test.go index 8cf5686..08b4c2b 100644 --- a/ulid_test.go +++ b/ulid_test.go @@ -529,7 +529,6 @@ func TestMonotonic(t *testing.T) { }{ {"cryptorand", func() io.Reader { return crand.Reader }}, {"mathrand", func() io.Reader { return rand.New(rand.NewSource(int64(now))) }}, - {"poolrand", ulid.DefaultEntropy}, } { for _, inc := range []uint64{ 0, From 1e6a3490f0221f0632333abedc567bdf9be8b7b8 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 22 Jun 2022 17:23:38 +0200 Subject: [PATCH 6/6] Review feedback --- README.md | 78 ++++++++++++++++++++++++++++++++++++++-------------- ulid.go | 1 + ulid_test.go | 9 +++++- 3 files changed, 66 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 2f494a1..c0094ce 100644 --- a/README.md +++ b/README.md @@ -39,35 +39,71 @@ go get github.com/oklog/ulid/v2 ## Usage -An ULID is constructed with a `time.Time` and an `io.Reader` entropy source. -This design allows for greater flexibility in choosing your trade-offs. - -If you want sane defaults, just use `ulid.Make()` which will produce a per process monotonically increasing ULID based on the current time that is safe for concurrent use. +ULIDs are constructed from two things: a timestamp with millisecond precision, +and some random data. + +Timestamps are modeled as uint64 values representing a Unix time in milliseconds. +They can be produced by passing a [time.Time](https://pkg.go.dev/time#Time) to +[ulid.Timestamp](https://pkg.go.dev/github.com/oklog/ulid/v2#Timestamp), +or by calling [time.Time.UnixMilli](https://pkg.go.dev/time#Time.UnixMilli) +and converting the returned value to `uint64`. + +Random data is taken from a provided [io.Reader](https://pkg.go.dev/io#Reader). +This design allows for greater flexibility when choosing trade-offs, but can be +a bit confusing to newcomers. + +If you just want to generate a ULID and don't (yet) care about details like +performance, cryptographic security, monotonicity, etc., use the +[ulid.Make](https://pkg.go.dev/github.com/oklog/ulid/v2#Make) helper function. +This function calls [time.Now](https://pkg.go.dev/time#Now) to get a timestamp, +and uses a source of entropy which is process-global, +[pseudo-random](https://pkg.go.dev/math/rand)), and +[monotonic](https://pkg.go.dev/oklog/ulid/v2#LockedMonotonicReader)). ```go -func ExampleMake() { - fmt.Println(ulid.Make()) - // Output: 0000XSNJG0MQJHBF4QX1EFD6Y3 -} +println(ulid.Make()) +// 01G65Z755AFWAKHE12NY0CQ9FH ``` -Otherwise, you'll need to provide your own entropy source. Since `rand.Rand` from the `math` package is [*not* safe for concurrent use](https://github.com/golang/go/issues/3611), consider the following guidance: - -- Instantiate one `rand.Rand` per long living go-routine if your concurrency model permits — this option will result in no lock contention. - -- For usage in short lived go-routines (e.g. HTTP handlers), use [`golang.org/x/exp/rand.Rand`](https://pkg.go.dev/golang.org/x/exp/rand#example-LockedSource) which is safe for concurrent use, but can result in a high amount of lock contention if many go-routines concurrently call `Read` on it. - -- When lock contention is too high with the previous approach, use a `sync.Pool` of `rand.Rand` instances. This won't provide any benefits if you need to per process monotonic ULIDs, since using `LockedMonotonicReader` will synchronize all reads anyways. +More advanced use cases should utilize +[ulid.New](https://pkg.go.dev/github.com/oklog/ulid/v2#New). ```go -func ExampleMustNew() { - t := time.Unix(1000000, 0) - entropy := ulid.Monotonic(rand.New(rand.NewSource(t.UnixNano())), 0) - fmt.Println(ulid.MustNew(ulid.Timestamp(t), entropy)) - // Output: 0000XSNJG0MQJHBF4QX1EFD6Y3 -} +entropy := rand.New(rand.NewSource(time.Now().UnixNano())) +ms := ulid.Timestamp(time.Now()) +println(ulid.New(ms, entropy)) +// 01G65Z755AFWAKHE12NY0CQ9FH ``` +Care should be taken when providing a source of entropy. + +The above example utilizes [math/rand.Rand](https://pkg.go.dev/math/rand#Rand), +which is not safe for concurrent use by multiple goroutines. Consider +alternatives such as +[x/exp/rand](https://pkg.go.dev/golang.org/x/exp/rand#LockedSource). +Security-sensitive use cases should always use cryptographically secure entropy +provided by [crypto/rand](https://pkg.go.dev/crypto/rand). + +Performance-sensitive use cases should avoid synchronization when generating +IDs. One option is to use a unique source of entropy for each concurrent +goroutine, which results in no lock contention, but cannot provide strong +guarantees about the random data, and does not provide monotonicity within a +given millisecond. One common performance optimization is to pool sources of +entropy using a [sync.Pool](https://pkg.go.dev/sync#Pool). + +Monotonicity is a property that says each ULID is "bigger than" the previous +one. ULIDs are automatically monotonic, but only to millisecond precision. ULIDs +generated within the same millisecond are ordered by their random component, +which means they are by default un-ordered. You can use +[ulid.MonotonicEntropy](https://pkg.go.dev/oklog/ulid/v2#MonotonicEntropy) or +[ulid.LockedMonotonicEntropy](https://pkg.go.dev/oklog/ulid/v2#LockedMonotonicEntropy) +to create ULIDs that are monotonic within a given millisecond, with caveats. See +the documentation for details. + +If you don't care about time-based ordering of generated IDs, then there's no +reason to use ULIDs! There are many other kinds of IDs that are easier, faster, +smaller, etc. Consider UUIDs. + ## Commandline tool This repo also provides a tool to generate and parse ULIDs at the command line. diff --git a/ulid.go b/ulid.go index 85675ce..0cb258d 100644 --- a/ulid.go +++ b/ulid.go @@ -574,6 +574,7 @@ type LockedMonotonicReader struct { MonotonicReader } +// MonotonicRead synchronizes calls to the wrapped MonotonicReader. func (r *LockedMonotonicReader) MonotonicRead(ms uint64, p []byte) (err error) { r.mu.Lock() err = r.MonotonicReader.MonotonicRead(ms, p) diff --git a/ulid_test.go b/ulid_test.go index 08b4c2b..2a9476d 100644 --- a/ulid_test.go +++ b/ulid_test.go @@ -62,7 +62,14 @@ func TestNew(t *testing.T) { func TestMake(t *testing.T) { t.Parallel() - t.Log(ulid.Make()) + id := ulid.Make() + rt, err := ulid.Parse(id.String()) + if err != nil { + t.Fatalf("parse %q: %v", id.String(), err) + } + if id != rt { + t.Fatalf("%q != %q", id.String(), rt.String()) + } } func TestMustNew(t *testing.T) {