-
Notifications
You must be signed in to change notification settings - Fork 372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Monotonicity in UUIDv7 #150
Changes from 2 commits
960071a
f27ed75
654bc6c
64f9de4
5986a71
4b1aab9
314e206
8888abc
a6e034a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -825,7 +825,7 @@ func TestVersion6(t *testing.T) { | |
func TestVersion7(t *testing.T) { | ||
SetRand(nil) | ||
m := make(map[string]bool) | ||
for x := 1; x < 32; x++ { | ||
for x := 1; x < 128; x++ { | ||
uuid, err := NewV7() | ||
if err != nil { | ||
t.Fatalf("could not create UUID: %v", err) | ||
|
@@ -887,7 +887,26 @@ func TestVersion7FromReader(t *testing.T) { | |
if err != nil { | ||
t.Errorf("failed generating UUID from a reader") | ||
} | ||
if uuid1 != uuid3 { | ||
if uuid1 == uuid3 { // Montonicity | ||
t.Errorf("expected duplicates, got %q and %q", uuid1, uuid3) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this message is no longer accurate. |
||
} | ||
} | ||
|
||
func TestVersion7Monotonicity(t *testing.T) { | ||
length := 4097 // [0x000 - 0xfff] | ||
myString := "8059ddhdle77cb52" | ||
|
||
SetClockSequence(0) | ||
|
||
uuids := make([]string, length) | ||
for i := 0; i < length; i++ { | ||
uuidString, _ := NewV7FromReader(bytes.NewReader([]byte(myString))) | ||
uuids[i] = uuidString.String() | ||
} | ||
|
||
for i := 1; i < len(uuids); i++ { | ||
if uuids[i-1] > uuids[i] { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be >= ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
t.Errorf("expected seq got %s > %s", uuids[i-1], uuids[i]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/expected/unexpected There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ import ( | |
// NewV7 returns a Version 7 UUID based on the current time(Unix Epoch). | ||
// Uses the randomness pool if it was enabled with EnableRandPool. | ||
// On error, NewV7 returns Nil and an error | ||
// Note: this implement only has 12 bit seq, maximum of 4096 uuids are generated in 1 milliseconds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I understand it, this is not a limitation of the implementation but rather of the standard.
|
||
func NewV7() (UUID, error) { | ||
uuid, err := NewRandom() | ||
if err != nil { | ||
|
@@ -44,15 +45,15 @@ func NewV7FromReader(r io.Reader) (UUID, error) { | |
|
||
// makeV7 fill 48 bits time (uuid[0] - uuid[5]), set version b0111 (uuid[6]) | ||
// uuid[8] already has the right version number (Variant is 10) | ||
// see function NewV7 and NewV7FromReader | ||
// see function NewV7 and NewV7FromReader | ||
func makeV7(uuid []byte) { | ||
/* | ||
0 1 2 3 | ||
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 | ||
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | ||
| unix_ts_ms | | ||
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | ||
| unix_ts_ms | ver | rand_a | | ||
| unix_ts_ms | ver |rand_a (12 bit counter)| | ||
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | ||
|var| rand_b | | ||
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | ||
|
@@ -61,7 +62,7 @@ func makeV7(uuid []byte) { | |
*/ | ||
_ = uuid[15] // bounds check | ||
|
||
t := timeNow().UnixMilli() | ||
t, s := getTimeV7() | ||
|
||
uuid[0] = byte(t >> 40) | ||
uuid[1] = byte(t >> 32) | ||
|
@@ -70,6 +71,23 @@ func makeV7(uuid []byte) { | |
uuid[4] = byte(t >> 8) | ||
uuid[5] = byte(t) | ||
|
||
uuid[6] = 0x70 | (uuid[6] & 0x0F) | ||
// uuid[8] has already has right version | ||
uuid[6] = 0x70 | (0x0F & byte(s>>8)) | ||
uuid[7] = byte(s) | ||
} | ||
|
||
func getTimeV7() (int64, uint16) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don 't think this is quite right. Maybe something along the lines of:
There is nothing in the standard that requires Version 7 UUIDs to be monotonically increasing within a given millisecond but it is acceptable to use sub-millisecond values. It is also legitimate for the first 64 bits of two Version 7 UUIDs to be identical. The randomness in the last 62 bits is what makes them unique. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I misread part of the standard, it should be monotonic, but the above is still basically correct. What you need to do is have a protected global variable:
By using 1/4 nanoseconds we can generate 4 UUIDs within a nanosecond that do not cause us to return a time that is actually in the future by a nanosecond. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. v7 using millisecond (1/1000) timestamp, we are using microsecond (1/1000000) lower 12 bit set to rand_a now := timeNow().UnixMicro()
t, s := now/1000, now&4095 // 2^12-1, 12 bits
uuid[6] = 0x70 | (0x0F & byte(s>>8))
uuid[7] = byte(s) |
||
|
||
defer timeMu.Unlock() | ||
timeMu.Lock() | ||
|
||
if clockSeq == 0 { | ||
setClockSequence(-1) | ||
} | ||
now := timeNow().UnixMilli() | ||
|
||
if now <= lasttimev7 { | ||
clockSeq = clockSeq + 1 | ||
} | ||
lasttimev7 = now | ||
return now, clockSeq | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The period was lost here. In general, do not reformat comments unless they are incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed