Truncate the shard group end time if it exceeds MaxNanoTime #6693
Conversation
|
By analyzing the blame information on this pull request, we identified @e-dard to be a potential reviewer |
jsternberg
force-pushed
the
js-6599-fix-shard-groups-near-max-time
branch
2 times, most recently
from
ab1b1d0
to
be4cd2d
Compare
| } else if pb.EndTime != nil { | ||
| sgi.EndTime = UnmarshalTime(pb.GetEndTime()) | ||
| } else { | ||
| return errors.New("unable to unmarshal EndTime, missing both EndTime and Duration") |
There was a problem hiding this comment.
s/missing both EndTime and Duration/missing either EndTime or Duration
There was a problem hiding this comment.
Should this be a panic instead? It should never happen, but I feel like I have to account for this situation since both of the items are optional.
There was a problem hiding this comment.
Also it's missing both of them since this error only happens when pb.Duration == nil && pb.EndTime == nil.
jsternberg
force-pushed
the
js-6599-fix-shard-groups-near-max-time
branch
from
be4cd2d
to
3ca2741
Compare
|
I updated the PR to serialize both the end time and the duration, but to always prefer the duration if it's there. This keeps it compatible if someone upgrades and wants to go back a version. Should I add the |
jsternberg
force-pushed
the
js-6599-fix-shard-groups-near-max-time
branch
from
3ca2741
to
3384f7c
Compare
| @@ -371,6 +372,9 @@ func (data *Data) CreateShardGroup(database, policy string, timestamp time.Time) | |||
| sgi.ID = data.MaxShardGroupID | |||
| sgi.StartTime = timestamp.Truncate(rpi.ShardGroupDuration).UTC() | |||
| sgi.EndTime = sgi.StartTime.Add(rpi.ShardGroupDuration).UTC() | |||
| if sgi.EndTime.After(models.MaxNanoTime) { | |||
| sgi.EndTime = models.MaxNanoTime | |||
There was a problem hiding this comment.
In here also need to do sgi.StartTime = sgi.EndTime.Add(-models.MaxNanoTime.UnixNano()) or similar.
jsternberg
changed the title
jsternberg
force-pushed
the
js-6599-fix-shard-groups-near-max-time
branch
2 times, most recently
from
b712781
to
83dd10c
Compare
jsternberg
force-pushed
the
js-6599-fix-shard-groups-near-max-time
branch
from
83dd10c
to
0d386e1
Compare
|
@jsternberg this LGTM now 👍 |
jsternberg
force-pushed
the
js-6599-fix-shard-groups-near-max-time
branch
from
0d386e1
to
907c88d
Compare
Related #6599.