Skip to content

Conversation

@huaxingao
Copy link
Contributor

This is the 2nd PR for Idempotency Key. This PR adds a UUIDv7 generator to REST Util.

First PR: Add idempotency-key-lifetime to ConfigResponse

Follow-ups (separate PRs)

  • Client opt-in wiring in RESTSessionCatalog and mutation calls.
  • E2E tests with test server idempotency layer.

@github-actions github-actions bot added the core label Nov 27, 2025
@huaxingao huaxingao requested a review from nastra November 27, 2025 04:20
}

@Test
public void generateUuidV7_hasVersionAndVariant() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we don't use _ for method names

* 2-bit variant (RFC 4122, 0b10) - 62-bit random (rand_b)
*/
public static String generateUuidV7() {
long epochMs = System.currentTimeMillis() & 0xFFFFFFFFFFFFL; // 48 bits
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't reviewed this yet but just wanted to link how the JDK generates a UUIDv7 in JDK26: openjdk/jdk@642ba4c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the pointer! I didn’t realize JDK 26 includes UUIDv7. I’ll compare my implementation with UUID.ofEpochMillis, and align as needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I compared with JDK 26’s UUID.ofEpochMillis and our layout matches (48‑bit timestamp, version 7 nibble, RFC 4122 variant, 12+62 random bits). I’d like to keep a no‑arg generateUuidV7() to mirror UUID.randomUUID() and avoid exposing timestamps; since callers need a header value, it returns String. I’ll align by using SecureRandom and adding a 48‑bit range check. I won’t copy JDK code (licensing), but the behavior is equivalent. The test is also the same: verify version() == 7 and variant()==2.

private static final Joiner NAMESPACE_ESCAPED_JOINER = Joiner.on(NAMESPACE_ESCAPED_SEPARATOR);
private static final Splitter NAMESPACE_ESCAPED_SPLITTER =
Splitter.on(NAMESPACE_ESCAPED_SEPARATOR);
private static final SecureRandom SECURE_RANDOM = new SecureRandom();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

new SecureRandom() is relatively expensive; one shared instance is thread‑safe and sufficient.

@pan3793
Copy link
Member

pan3793 commented Dec 1, 2025

I wonder if this can go to a more common package (maybe a dedicated UuidUtil under org.apache.iceberg.util), as UUIDv7 may also benefit other places that use UUID, for example, View's uuid, Field's id, then we can infer when it was generated from the uuid prefix

@github-actions github-actions bot added the API label Dec 1, 2025
Copy link
Contributor

@singhpk234 singhpk234 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @huaxingao !

Comment on lines 93 to 95
if ((epochMs >>> 48) != 0) {
throw new IllegalArgumentException("timestamp does not fit within 48 bits: " + epochMs);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We could use checkArgument method instead:

Preconditions.checkArgument(
        (epochMs >>> 48) == 0, "timestamp does not fit within 48 bits: %s", epochMs);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

public static String generateUuidV7() {
long epochMs = System.currentTimeMillis();
Preconditions.checkArgument(
(epochMs >>> 48) == 0, "timestamp does not fit within 48 bits: %s", epochMs);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this should be an IllegalStateException, since the user can't really provide an argument here and we're throwing a IAE? Also I would probably update the error msg to align with the rest of the codebase to something like Invalid timestamp: does not fit within 48 bits: ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks

Copy link
Contributor

@nastra nastra left a comment

Choose a reason for hiding this comment

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

LGTM with one comment

* <p>Layout: - 48-bit Unix epoch milliseconds - 4-bit version (0b0111) - 12-bit random (rand_a) -
* 2-bit variant (RFC 4122, 0b10) - 62-bit random (rand_b)
*/
public static String generateUuidV7() {
Copy link
Member

Choose a reason for hiding this comment

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

How about returning UUID instead of String? then the caller can call toString if necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the return type to UUID.

}

/**
* Generate a RFC 9562 UUIDv7 string.
Copy link
Contributor

Choose a reason for hiding this comment

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

The returned type isn't string anymore.

Suggested change
* Generate a RFC 9562 UUIDv7 string.
* Generate a RFC 9562 UUIDv7.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@nastra nastra changed the title Core: Add UUIDv7 generator to REST Util Core: Add UUIDv7 generator Dec 2, 2025
@nastra nastra merged commit 16e8435 into apache:main Dec 2, 2025
45 checks passed
@huaxingao huaxingao deleted the idempotency_2 branch December 2, 2025 17:55
pan3793 added a commit to apache/kyuubi that referenced this pull request Dec 18, 2025
### Why are the changes needed?

https://datatracker.ietf.org/doc/html/rfc9562#name-uuid-version-7

https://uuidv7.org/

> The Key Benefits of UUIDv7
UUIDv7 is rapidly gaining traction for various use cases across many industries. Here are some of the primary benefits:
>
> 1. Time-Ordered and Sortable: UUIDv7 encodes the time of creation as part of the identifier, making it naturally sortable. This is especially valuable for systems where data needs to be ordered by time without needing an additional timestamp field.
> ...

I think it's very useful - e.g., use UUIDv7 as session ID, then we can infer the session creation time from the UUID itself, or use UUIDv7 as a staging dir name, then we can easily clean dangling staging folders (for example, created 3 months ago) with a prefix.

Implementation is inspired by apache/iceberg#14700

### How was this patch tested?

New UTs are added.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #7277 from pan3793/uuidv7.

Closes #7277

1dc1801 [Cheng Pan] style
3c7d3c5 [Cheng Pan] Add UUID v7 generator

Authored-by: Cheng Pan <chengpan@apache.org>
Signed-off-by: Cheng Pan <chengpan@apache.org>
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