HHH-18377 Support for uuid v6 and v7 generated ids #8719

cigaly · 2024-07-27T09:28:13Z

Implementation of UUID Versioin 6 and UUID Version 7

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

CaptainGlac1er · 2024-08-07T17:57:35Z

How will it handle if a separate server generates the same id into the same database? Will it recreate a new ID automatically and try again?

sebersole · 2024-09-17T17:02:09Z

How will it handle if a separate server generates the same id into the same database? Will it recreate a new ID automatically and try again?

No.

sebersole

I left a few comments about odd references to UUID#randomUUID(). Let's clean those up.

Additionally, you write great detail about what each value generator does. But you do it on #generateUuid. I'd prefer to see those on the class Javadoc, because...

Another thing we should do here is to add values for these into UuidGenerator.Style. Its probably best to have these link to the class Javadoc for the corresponding value-generator.

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidV6ValueGenerator.java

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidV7ValueGenerator.java

sebersole · 2024-09-19T12:26:01Z

A few naming related points here...

Not a fan of the enum names (UUIDV6, UUIDV7). How about VERSION6 and VERSION7 (or VERSION_6...)? Or since they are RFC compliant, RFC_6 and RFC_7?
The value generator names should use {Type}Strategy pattern to match with the others. E.g. Version6Strategy.

I'll need to pull this locally to squash the commits before I apply. I can apply the name changes at that time if you prefer.

Thanks again for the work on this!

cigaly · 2024-09-19T13:16:06Z

A few naming related points here...

1. Not a fan of the enum names (UUIDV6, UUIDV7).  How about VERSION6 and VERSION7 (or VERSION_6...)?  Or since they are RFC compliant, RFC_6 and RFC_7?

2. The value generator names should use `{Type}Strategy` pattern to match with the others.  E.g. `Version6Strategy`.

Changed both things.

I'll need to pull this locally to squash the commits before I apply. I can apply the name changes at that time if you prefer.

I've squashed everything into two commits, one for implementation, other for test case. Hope that this is OK?

Thanks again for the work on this!

You're welcome. Happy if I can help. Not that I was too busy lately.

sebersole · 2024-09-19T13:19:01Z

In general, we like the test/impl split for the PR as it shows (1) what is addressed and (2) how it was addressed.

When we apply them, though, we generally (not always) squash to one as it makes it easier to cherry-pick to other branches. This we won't be backporting, so its less important. Long story short, 2 commits is fine here.

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidGenerator.java

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion7Strategy.java

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidGenerator.java

sebersole

A few minor suggestions/requests. Let me know what you think. FWIW I left "suggested changes" in those places to make it easy to apply if you agree.

sebersole · 2024-09-19T13:38:39Z

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion6Strategy.java

+ *     <li>14 bits - the clock sequence, resets to 0 when timestamp changes. </li>
+ *     <li>48 bits - pseudorandom data to provide uniqueness.</li>
+ * </ul>
+ *


Suggested change

*

*

* @apiNote Version 6 is field-compatible with Version 1, with the time bits reordered for improved DB locality.

*

sebersole · 2024-09-19T13:53:14Z

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion6Strategy.java

+	 */
+	@Override
+	public int getGeneratedVersion() {
+		// UUIDv6 is a field-compatible version of UUIDv1, reordered for improved DB locality


Suggested change

// UUIDv6 is a field-compatible version of UUIDv1, reordered for improved DB locality

sebersole · 2024-09-19T13:56:01Z

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion6Strategy.java

+	 * @param session session
+	 *
+	 * @return UUID version 6
+	 * @see UuidValueGenerator#generateUuid(SharedSessionContractImplementor)


Suggested change

* @param session session

*

* @return UUID version 6

* @see UuidValueGenerator#generateUuid(SharedSessionContractImplementor)

* Generates a Version 6 compliant {@linkplain UUID} value.

*

* @return A Version 6 compliant {@linkplain UUID}.

*

* @see UuidValueGenerator#generateUuid

sebersole · 2024-09-19T14:00:36Z

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion7Strategy.java

+ *     <li>14 bits - counter to guarantee additional monotonicity, resets to 0 when timestamp changes. </li>
+ *     <li>48 bits - pseudorandom data to provide uniqueness.</li>
+ * </ul>
+ *


Suggested change

*

*

* @apiNote A Version 7 UUID features a time-ordered value field derived from the widely

* implemented and well known Unix Epoch timestamp source, the number of milliseconds

* since midnight 1 Jan 1970 UTC, leap seconds excluded.

*

sebersole · 2024-09-19T14:01:34Z

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion7Strategy.java

+		/*
+		 *	UUIDv7 features a time-ordered value field derived from the widely implemented and well-
+		 * known Unix Epoch timestamp source, the number of milliseconds since midnight 1 Jan 1970 UTC,
+		 * leap seconds excluded.
+		 */


Suggested change

/*

* UUIDv7 features a time-ordered value field derived from the widely implemented and well-

* known Unix Epoch timestamp source, the number of milliseconds since midnight 1 Jan 1970 UTC,

* leap seconds excluded.

*/

sebersole · 2024-09-19T14:02:44Z

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion7Strategy.java

+	 * @param session session
+	 *
+	 * @return UUID version 7
+	 * @see UuidValueGenerator#generateUuid(SharedSessionContractImplementor)


Suggested change

* @param session session

*

* @return UUID version 7

* @see UuidValueGenerator#generateUuid(SharedSessionContractImplementor)

* Generates a Version 7 {@linkplain UUID}.

*

* @return A Version version 7 {@linkplain UUID}.

* @see UuidValueGenerator#generateUuid(SharedSessionContractImplementor)

sebersole · 2024-09-19T14:19:58Z

Also, clearly we need to address the failures :)

Execution failed for task ':hibernate-core:compileTestJava'.
> Compilation failed; see the compiler error output for details.

cigaly · 2024-09-19T14:43:39Z

Also, clearly we need to address the failures :)

Execution failed for task ':hibernate-core:compileTestJava'.
> Compilation failed; see the compiler error output for details.

One will expect that IDE should know to make basic refactoring things like renaming classes :-)

…ersion 7 UUID's

sebersole · 2024-09-23T18:53:49Z

Applied manually. Thanks!

beikov · 2024-09-26T09:47:55Z

I'm getting test errors quite consistently. I didn't have a single success of the v6 test so far:

java.lang.AssertionError: 
Expecting actual:
  "1ef7be82-786f-69e5-8000-5b0eb9699e10"
to be greater than:
  "1ef7be82-786f-69e5-bfff-e4cda4de4e50"

	at org.hibernate.orm.test.id.uuid.rfc9562.UUidV6V7GenetartorTest.testMonotonicity(UUidV6V7GenetartorTest.java:43)
	at org.hibernate.orm.test.id.uuid.rfc9562.UUidV6V7GenetartorTest.testMonotonicityUuid6(UUidV6V7GenetartorTest.java:26)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Running with jdk-17.0.12+7 on Windows 11 if it matters. I don't know how the implementation works, but it seems broken.

cigaly · 2024-09-26T11:17:57Z

It seems that there is pretty stupid bug :-( I'll check and fix that.

beikov · 2024-09-26T11:19:03Z

I just ran into another (unrelated but possibly similar) error, which is related to the fact that on Windows, the time source has a precision of 7 digits after the decimal separator.

cigaly · 2024-09-26T12:56:43Z

I've located that problem and will fix it ASAP

cigaly · 2024-09-26T14:46:33Z

@beikov I've created new pull request #9020 with updates to both UUID v6 and v7 generation

cigaly force-pushed the HHH-18377 branch 3 times, most recently from 5b0017a to 066a676 Compare July 29, 2024 15:28

cigaly marked this pull request as ready for review July 29, 2024 16:21

cigaly force-pushed the HHH-18377 branch from 066a676 to 182b8e0 Compare July 29, 2024 16:21

cigaly force-pushed the HHH-18377 branch from 182b8e0 to 08435f6 Compare August 9, 2024 16:47

sebersole requested changes Sep 17, 2024

View reviewed changes

cigaly force-pushed the HHH-18377 branch 2 times, most recently from 5134d12 to e44c59b Compare September 19, 2024 05:43

cigaly force-pushed the HHH-18377 branch from e44c59b to ba08ad0 Compare September 19, 2024 13:11

sebersole approved these changes Sep 19, 2024

View reviewed changes

CaptainGlac1er reviewed Sep 19, 2024

View reviewed changes

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidGenerator.java Show resolved Hide resolved

CaptainGlac1er reviewed Sep 19, 2024

View reviewed changes

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion7Strategy.java Outdated Show resolved Hide resolved

sebersole reviewed Sep 19, 2024

View reviewed changes

hibernate-core/src/main/java/org/hibernate/id/uuid/UuidGenerator.java Show resolved Hide resolved

cigaly force-pushed the HHH-18377 branch from ba08ad0 to 20467ee Compare September 19, 2024 13:56

sebersole reviewed Sep 19, 2024

View reviewed changes

cigaly added 2 commits September 19, 2024 17:02

HHH-18377 UUID Version 6 & UUID Version 7 implementations

b57e84e

HHH-18377 Test cases to check monofonicity of generated version 6 & v…

90e4f06

…ersion 7 UUID's

cigaly force-pushed the HHH-18377 branch from 20467ee to 90e4f06 Compare September 19, 2024 15:02

sebersole closed this Sep 23, 2024

cigaly mentioned this pull request Sep 26, 2024

Hhh 18377a #9020

Merged

- *
+ *
+ * @apiNote Version 6 is field-compatible with Version 1, with the time bits reordered for improved DB locality.
+ *

- *
+ *
+ * @apiNote A Version 7 UUID features a time-ordered value field derived from the widely
+ * implemented and well known Unix Epoch timestamp source, the number of milliseconds
+ * since midnight 1 Jan 1970 UTC, leap seconds excluded.
+ *

Uh oh!

HHH-18377 Support for uuid v6 and v7 generated ids #8719

HHH-18377 Support for uuid v6 and v7 generated ids #8719

Uh oh!

Conversation

cigaly commented Jul 27, 2024 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CaptainGlac1er commented Aug 7, 2024

Uh oh!

sebersole commented Sep 17, 2024

Uh oh!

sebersole left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sebersole commented Sep 19, 2024

Uh oh!

cigaly commented Sep 19, 2024

Uh oh!

sebersole commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sebersole left a comment

Choose a reason for hiding this comment

Uh oh!

sebersole Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

sebersole Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

sebersole Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

sebersole Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

sebersole Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

sebersole Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

sebersole commented Sep 19, 2024

Uh oh!

cigaly commented Sep 19, 2024

Uh oh!

sebersole commented Sep 23, 2024

Uh oh!

beikov commented Sep 26, 2024

Uh oh!

cigaly commented Sep 26, 2024

Uh oh!

beikov commented Sep 26, 2024

Uh oh!

cigaly commented Sep 26, 2024

Uh oh!

cigaly commented Sep 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cigaly commented Jul 27, 2024 •

edited by atlassian bot

Loading

sebersole commented Sep 19, 2024 •

edited

Loading