-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-138862: fix timestamp increment after UUIDv7 counter overflow #146606
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
b3fa2d0
e60c126
299d7a5
60877e4
a6360d1
cf1d6b4
6611889
10ae4c0
9a09e62
526048e
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 |
|---|---|---|
|
|
@@ -832,6 +832,18 @@ def uuid6(node=None, clock_seq=None): | |
|
|
||
| _last_timestamp_v7 = None | ||
| _last_counter_v7 = 0 # 42-bit counter | ||
| # Indicate whether one or more counter overflow(s) happened in the same frame. | ||
| # | ||
| # Since the timestamp is incremented after a counter overflow by design, | ||
| # we must prevent incrementing the timestamp again in consecutive calls | ||
| # for which the logical timestamp millisecond remains the same. | ||
| # | ||
| # If the resampled counter hits an overflow again within the same time, | ||
| # we want to advance the timestamp again and resample the timestamp. | ||
| # | ||
| # See https://github.com/python/cpython/issues/138862. | ||
| _last_counter_v7_overflow = False | ||
|
|
||
|
|
||
| def _uuid7_get_counter_and_tail(): | ||
| rand = int.from_bytes(os.urandom(10)) | ||
|
|
@@ -862,23 +874,33 @@ def uuid7(): | |
|
|
||
| global _last_timestamp_v7 | ||
| global _last_counter_v7 | ||
| global _last_counter_v7_overflow | ||
|
|
||
| nanoseconds = time.time_ns() | ||
| timestamp_ms = nanoseconds // 1_000_000 | ||
|
|
||
| if _last_timestamp_v7 is None or timestamp_ms > _last_timestamp_v7: | ||
|
Contributor
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. Side note: if we initialize _last_timestamp_v7 to a large negative value (e.g. -2**64), then we can remove the
Member
Author
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. Or just -1 and it's ok. You won't have a negative timestamp I think.
Contributor
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. You could (according to Claude when setting the system time to before 1970). But since the timestamp is generated via a |
||
| counter, tail = _uuid7_get_counter_and_tail() | ||
| _last_counter_v7_overflow = False | ||
|
picnixz marked this conversation as resolved.
|
||
| else: | ||
| if timestamp_ms < _last_timestamp_v7: | ||
| timestamp_ms = _last_timestamp_v7 + 1 | ||
| # The clock went backwards or we are within the same timestamp | ||
| # after a counter overflow. We follow the RFC for in the former | ||
| # case. In the latter case, we re-use the already advanced | ||
| # timestamp (it was updated when we detected the overflow). | ||
| if _last_counter_v7_overflow: | ||
|
Contributor
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. The first case from the if-else is the "latter" case in the comments above. Maybe swap the order in the comments.
Member
Author
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. Oh right :') I changed so much this block that I forgot to update the comment. Thanks! |
||
| timestamp_ms = _last_timestamp_v7 | ||
| else: | ||
| timestamp_ms = _last_timestamp_v7 + 1 | ||
|
picnixz marked this conversation as resolved.
|
||
| # advance the 42-bit counter | ||
| counter = _last_counter_v7 + 1 | ||
| if counter > 0x3ff_ffff_ffff: | ||
| # advance the 48-bit timestamp | ||
| _last_counter_v7_overflow = True | ||
|
picnixz marked this conversation as resolved.
|
||
| timestamp_ms += 1 | ||
| counter, tail = _uuid7_get_counter_and_tail() | ||
| else: | ||
| # 32-bit random data | ||
| _last_counter_v7_overflow = False | ||
|
picnixz marked this conversation as resolved.
Outdated
|
||
| tail = int.from_bytes(os.urandom(4)) | ||
|
|
||
| unix_ts_ms = timestamp_ms & 0xffff_ffff_ffff | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| :mod:`uuid`: the timestamp of UUIDv7 objects generated within the same | ||
| millisecond after encountering a counter overflow is only incremented once | ||
| for the entire batch of UUIDv7 objects instead at each object creation. | ||
| Patch by Bénédikt Tran. |
Uh oh!
There was an error while loading. Please reload this page.