Modify the max nanosecond time to be one nanosecond less #6850
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
|
By analyzing the blame information on this pull request, we identified @e-dard and @joelegasse to be potential reviewers |
jsternberg
force-pushed
the
js-shard-group-max-time
branch
3 times, most recently
from
570d609 to
68ee53e
Compare
| @@ -20,7 +20,7 @@ const ( | |||
| // | |||
| // 2262-04-11 23:47:16.854775807 +0000 UTC | |||
| // | |||
| MaxNanoTime = int64(math.MaxInt64) | |||
| MaxNanoTime = int64(math.MaxInt64) - 1 | |||
There was a problem hiding this comment.
You need to update the timestamp in the comment to be 1ns less.
|
LGTM 👍 with the comment fix |
The highest time represented by a nanosecond needs to be used for an exclusive range, so the maximum time needs to be one less than the possible maximum number of nanoseconds representable by an int64 so that we don't lose a point at that one time. Previously worked in the open source version because the timestamp used for finding a shard would be truncated by the retention policy so the lookup time didn't run into this edge case because it didn't rest on the truncation boundary. Since that point didn't really belong in that shard group and was placed there by mistake, it's best to fix this bug since the timestamp used to create the shard group should be capable of retrieving it.
jsternberg
force-pushed
the
js-shard-group-max-time
branch
from
68ee53e to
8e1b036
Compare
|
@e-dard I also fixed the influxql package change you pointed out. It looks like shard group times are |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The highest time represented by a nanosecond needs to be used for an
exclusive range, so the maximum time needs to be one less than the
possible maximum number of nanoseconds representable by an int64 so that
we don't lose a point at that one time.
Previously worked in the open source version because the timestamp used
for finding a shard would be truncated by the retention policy so the
lookup time didn't run into this edge case because it didn't rest on the
truncation boundary. Since that point didn't really belong in that shard
group and was placed there by mistake, it's best to fix this bug since
the timestamp used to create the shard group should be capable of
retrieving it.