Generate meaningful segment ids using UUIDv7 - #3012
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1f6174898
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // - chronologically sortable (ids sort by creation time), and | ||
| // - self-describing (the creation time can be recovered, see | ||
| // `SegmentId::creation_time`). | ||
| Uuid::now_v7() |
There was a problem hiding this comment.
Preserve uniqueness in the displayed UUID prefix
When production segments are created in the same 65.536-second timestamp bucket, their UUIDv7 values share the first eight hexadecimal characters because those characters now contain only the high 32 bits of the millisecond timestamp. short_uuid_string, SegmentId formatting, and SegmentRegister formatting still use exactly that prefix, so multiple distinct active segments routinely appear under the same short identifier in diagnostics. Include some of the random suffix in the short form when switching generation to v7.
Useful? React with 👍 / 👎.
0289ecb to
0b82481
Compare
Closes quickwit-oss#971. Segment ids were generated as random UUIDv4. Those 16 bytes carried no information: they were not sortable and told you nothing about when a segment was created. This switches non-test segment id generation to UUIDv7 (RFC 9562), which embeds a 48-bit millisecond creation timestamp in the most significant bits followed by random bits. The result is still universally unique and still lock-free to generate from any indexing thread (the properties the original UUIDv4 was chosen for), while additionally being: - chronologically sortable: `SegmentId`'s `Ord` is a byte comparison, so ids now sort by creation time. - self-describing: the new `SegmentId::creation_time()` recovers the embedded timestamp for debugging. This is fully backward compatible. The on-disk format is unchanged (a 32-char hex uuid). Ids from older indices are UUIDv4 and keep working; `creation_time()` returns `None` for them (and for the autoincrement ids used in tests). Note: this deliberately does not encode a hostname/label or merge origin into the id. As discussed in quickwit-oss#971, that information is a better fit for segment attributes on `SegmentMeta` (quickwit-oss#999) than for the id itself, and avoids the unresolved question of how labels should be combined on merge.
0b82481 to
cff8f9b
Compare
Closes #971.
Segment ids were generated as random UUIDv4. Those 16 bytes carried no information: they were not sortable and told you nothing about when a segment was created.
This switches non-test segment id generation to UUIDv7 (RFC 9562), which embeds a 48-bit millisecond creation timestamp in the most significant bits followed by random bits. The result is still universally unique and still lock-free to generate from any indexing thread (the properties the original UUIDv4 was chosen for), while additionally being:
SegmentId'sOrdis a byte comparison, so ids now sort by creation time.SegmentId::creation_time()recovers the embedded timestamp for debugging.This is fully backward compatible. The on-disk format is unchanged (a 32-char hex uuid). Ids from older indices are UUIDv4 and keep working;
creation_time()returnsNonefor them (and for the autoincrement ids used in tests).Note: this deliberately does not encode a hostname/label or merge origin into the id. As discussed in #971, that information is a better fit for segment attributes on
SegmentMeta(#999) than for the id itself, and avoids the unresolved question of how labels should be combined on merge.