Skip to content
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

Add UUID v7 #14732

Merged
merged 7 commits into from
Jun 26, 2024
Merged

Add UUID v7 #14732

merged 7 commits into from
Jun 26, 2024

Conversation

jgaskins
Copy link
Contributor

Part of #14288. This PR doesn't implement all of the requested UUID versions, but v7 is gaining in popularity for its ability to sort UUID values chronologically and RFC 9562 was published last month.

It's notable that RFC 4122 has been obsoleted via RFC 9562, so I included an alias for the new RFC in the Variant enum.

Spec: https://www.rfc-editor.org/rfc/rfc9562#name-uuid-version-7

Maybe there was a reason this didn't use the predicate methods?
Copy link
Contributor

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

But the spec is weird as the format has optional non-random parts to increase precision + counter 😕

I understand the more precise, the better for database indexes, but less randomness means more chances for collisions, so we should give some control over it.

Do you think we could specify the precision (second vs millisecond), and later a counter to control this aspect?

src/uuid.cr Outdated Show resolved Hide resolved
Co-authored-by: Julien Portalier <julien@portalier.com>
@jgaskins
Copy link
Contributor Author

the spec is weird as the format has optional non-random parts to increase precision + counter 😕

I understand the more precise, the better for database indexes, but less randomness means more chances for collisions, so we should give some control over it.

Do you think we could specify the precision (second vs millisecond), and later a counter to control this aspect?

I hear you, reducing entropy honestly sounds scary. I've given it the maximum randomness and minimum monotonicity the spec allows for. Millisecond precision is prescribed by the spec.

In thinking about the failure modes of a UUID collision (admittedly from my own perspective, which mostly revolves around using them as DB primary keys), the most likely scenario would be an exception due to INSERTing 2 rows into a table with the same primary key. With a keyspace of 2**74 (roughly 1.9e22) values per millisecond, this would happen infrequently enough that a simple retry should work.

Since there are other UUID solutions that provide more randomness and less precision, if that's what you need, I'd argue that a v7 UUID is not.

@ysbaddaden
Copy link
Contributor

Sorry, I misread the spec, the timestamp is in milliseconds 👍

# NetworkEndian.encode call here, but we only need 48 bits of it so we chop
# off the first 2 bytes.
IO::ByteFormat::NetworkEndian.encode Time.utc.to_unix_ms, value
value = value[2..]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (nitpick): maybe extract a timestamp variable and rearrange a bit the above to make the steps more explicit?

timestamp = Time.utc.to_unix_ms
buffer = uninitialized UInt8[18] # 2 bytes (skipped, see below) + 16 bytes (uuid)

# convert timestamp integer into bytes and skip the upper bytes (...)
IO::ByteFormat::NetworkEndian.encode timestamp, buffer.to_slice
value = buffer.to_slice[2..]

@ysbaddaden
Copy link
Contributor

Feel free to ignore the above suggestions, they're just nitpicks. But WASM32 doesn't like the call to sleep in the spec. I think the eventloop isn't implemented.

@jgaskins
Copy link
Contributor Author

Looks like pending! also doesn't work on WASM? 🤔

Crystal::Wasi::EventLoop.create_resume_event is not implemented on WASM,
which breaks the `sleep` call.
@jkthorne
Copy link
Contributor

nice, I didn't know this existed. I will be switching a few projects to V7.

@straight-shoota
Copy link
Member

straight-shoota commented Jun 25, 2024

Dang, didn't realize that. We can use pending_wasm32 instead of a custom method.

@straight-shoota straight-shoota added this to the 1.13.0 milestone Jun 25, 2024
@straight-shoota straight-shoota merged commit 6c8542a into crystal-lang:master Jun 26, 2024
61 checks passed
@jgaskins jgaskins deleted the add-uuid-v7 branch July 5, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants