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

Negative timestamps when using go client and passing precision #3044

Closed
vladlopes opened this issue Jun 18, 2015 · 3 comments
Closed

Negative timestamps when using go client and passing precision #3044

vladlopes opened this issue Jun 18, 2015 · 3 comments

Comments

@vladlopes
Copy link
Contributor

After a lot of digging I finally found why I was getting negative timestamps on my shard groups (see comments on PR #3020) and points.

There is a possibility of an integer overflow, when using the go client, on this line https://github.com/influxdb/influxdb/blob/master/tsdb/points.go#L156.

The go client always send the timestamp in nanoseconds, as it uses this function https://github.com/influxdb/influxdb/blob/master/tsdb/points.go#L780 to convert points to the line protocol.

The problem occurs if the go client send the precision. When the point is parsed, the timestamp, that was already in nanoseconds get multiplied, and can overflow.

One way to solve this would be:

  • Make the GetPrecisionMultiplier on tsdb/points a method not attached to point (as it no longer uses any point information);
  • On the client, divide every point time by the multiplier, when there is a precision, before marshalling it to string.

Thoughts? @corylanou @jwilder

@beckettsean beckettsean added this to the 0.9.1 milestone Jun 18, 2015
@beckettsean
Copy link
Contributor

@vladlopes What motivated you to provide a non-nanosecond precision along with your nanosecond timestamps?

@vladlopes
Copy link
Contributor Author

@beckettsean On the go client, we don't provide the timestamps directly (only when using the new Raw variable). We provide a time.Time and it gets converted to a nanosecond timestamp.

So there is a mismatch between this timestamp, that is always sent in nanosecond, when you set the precision field. When this timestamp is received on the server side, it thinks that is on the precision specified, which is not, hence, I think, we need to adjust the marshaling on the client side.

My use case only needed second precision. I have already changed it, as a workaround, while this issue is not resolved.

@toddboom toddboom modified the milestones: 0.9.1, 0.9.2 Jul 2, 2015
@toddboom toddboom modified the milestones: 0.9.2, 0.9.3 Jul 29, 2015
@beckettsean beckettsean modified the milestones: Next Point Release, 0.9.3 Aug 6, 2015
@jsternberg jsternberg assigned jsternberg and unassigned gunnaraasen Aug 26, 2016
@jsternberg
Copy link
Contributor

I took a look at this and it appears the client has already been updated to do this. I'm going to close this issue since I think it's been resolved at some point in the past. Please make a comment or open a new issue if this still exists. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants