Skip to content
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

core: name_prefix names now start with a timestamp #8249

Merged
merged 1 commit into from
Aug 23, 2016

Conversation

glasser
Copy link
Contributor

@glasser glasser commented Aug 17, 2016

This means that two resources created by the same rule during different
seconds will get names which sort in the order they are created.

The rest of the ID is still random base32 characters; we no longer set
the bit values that denote UUIDv4.

The length of the ID returned by PrefixedUniqueId is not changed by this
commit.

Fixes #8143.

@jen20
Copy link
Contributor

jen20 commented Aug 17, 2016

Hi @glasser! I definitely see the purpose of this, but think we might be best off making the second couple of digits incrementing within a second so that things always sort correctly. We'll discuss internally and take this one forward shortly. Thanks for the PR!

@glasser
Copy link
Contributor Author

glasser commented Aug 17, 2016

Sure, I can easily make it go out to nanoseconds instead of seconds. Or if that's not good enough, just have a "counter" section too.

@mitchellh
Copy link
Contributor

Agreed that we should make the granularity more fine-grained than second. I don't think there is any reason we need to stick exactly to the previous format as long as we don't introduce any new "weird" characters (stick to ASCII). So introducing a couple/few more characters that are nanoseconds sounds good to me.

@glasser
Copy link
Contributor Author

glasser commented Aug 17, 2016

Some underlying resources have name length limits, so I wouldn't want to break people who have already shrunk their prefixes to fit. But it's easy to just shrink the random part.

This means that two resources created by the same rule will get names
 which sort in the order they are created.

The rest of the ID is still random base32 characters; we no longer set
the bit values that denote UUIDv4.

The length of the ID returned by PrefixedUniqueId is not changed by this
commit; that way we don't break any resources where the underlying
resource has a name length limit.

Fixes hashicorp#8143.
@glasser glasser force-pushed the glasser/name-prefix-timestamp branch from 6c11268 to 4ad825f Compare August 17, 2016 18:06
@glasser
Copy link
Contributor Author

glasser commented Aug 17, 2016

Updated to include nanoseconds and only 3 random characters. The test now verifies the ordering property.

@jen20 jen20 merged commit df06d56 into hashicorp:master Aug 23, 2016
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.
@ghost
Copy link

ghost commented Apr 23, 2020

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 ghost locked and limited conversation to collaborators Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core: names produced via name_prefix should start with a timestamp
4 participants