Fix resource.UniqueId to be properly ordered #10152
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbardin
force-pushed
the
jbardin/unique-id
branch
2 times, most recently
from
c8d5eb4 to
8795ee4
Compare
UniqueId attempted to provide an ordered unique id by using a nanosecond timestamp, but doesn't take into account that time is not monotonic increasing. This provides an implementation that will always be increasing.
jbardin
force-pushed
the
jbardin/unique-id
branch
from
8795ee4 to
e28e11d
Compare
mitchellh
approved these changes
Nov 19, 2016
|
I know this was a while ago, but I only noticed it recently. The goal of introducing the timestamp (see #8143 and #8249) was less about getting ordering between resources created in a single invocation of For example, when creating a new AWS ASG and moving instances slowly from the old one to the new one, I wanted to be able to easily see "this is the old one, this is the new one". It also makes it immediately obvious "I made this ASG about two weeks ago" which is helpful. So we've moved from "purely random" to "mostly ordered and indicates the general time when something started, with random prefix" to "consistent within a single TF iteration but otherwise not helpful". Would you accept a PR that restored the timestamp prefix but with an incrementing suffix? |
glasser
added a commit
to meteor/terraform
that referenced
this pull request
Jun 13, 2017
The timestamp prefix added in hashicorp#8249 was removed in hashicorp#10152 to ensure that returned IDs really are properly ordered. However, this meant that IDs were no longer ordered over multiple invocations of terraform, which was the main motivation for adding the timestamp in the first place. This commit does a hybrid: timestamp-plus-incrementing-counter instead of just incrementing counter or timestamp-plus-random.
glasser
added a commit
to meteor/terraform
that referenced
this pull request
Jun 13, 2017
The timestamp prefix added in hashicorp#8249 was removed in hashicorp#10152 to ensure that returned IDs really are properly ordered. However, this meant that IDs were no longer ordered over multiple invocations of terraform, which was the main motivation for adding the timestamp in the first place. This commit does a hybrid: timestamp-plus-incrementing-counter instead of just incrementing counter or timestamp-plus-random.
glasser
added a commit
to meteor/terraform
that referenced
this pull request
Jun 13, 2017
The timestamp prefix added in hashicorp#8249 was removed in hashicorp#10152 to ensure that returned IDs really are properly ordered. However, this meant that IDs were no longer ordered over multiple invocations of terraform, which was the main motivation for adding the timestamp in the first place. This commit does a hybrid: timestamp-plus-incrementing-counter instead of just incrementing counter or timestamp-plus-random.
glasser
added a commit
to meteor/terraform
that referenced
this pull request
Jun 15, 2017
The timestamp prefix added in hashicorp#8249 was removed in hashicorp#10152 to ensure that returned IDs really are properly ordered. However, this meant that IDs were no longer ordered over multiple invocations of terraform, which was the main motivation for adding the timestamp in the first place. This commit does a hybrid: timestamp-plus-incrementing-counter instead of just incrementing counter or timestamp-plus-random.
glasser
added a commit
to meteor/terraform
that referenced
this pull request
Jun 15, 2017
The timestamp prefix added in hashicorp#8249 was removed in hashicorp#10152 to ensure that returned IDs really are properly ordered. However, this meant that IDs were no longer ordered over multiple invocations of terraform, which was the main motivation for adding the timestamp in the first place. This commit does a hybrid: timestamp-plus-incrementing-counter instead of just incrementing counter or timestamp-plus-random.
glasser
added a commit
to meteor/terraform
that referenced
this pull request
Jun 15, 2017
The timestamp prefix added in hashicorp#8249 was removed in hashicorp#10152 to ensure that returned IDs really are properly ordered. However, this meant that IDs were no longer ordered over multiple invocations of terraform, which was the main motivation for adding the timestamp in the first place. This commit does a hybrid: timestamp-plus-incrementing-counter instead of just incrementing counter or timestamp-plus-random.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This isn't a major issue, and probably never encountered in real usage, but I was tired of my unit tests failing because time was used as a counter.
UniqueIdattempted to provide an ordered unique id by using a nanosecondtimestamp, but doesn't take into account that time is not monotonic
increasing, nor do many system clocks have the resolution to return different times
when called in quick succession. This provides an implementation that will always be
increasing.