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

RFC: Timestamp points before queuing to preserve original time #211

Closed

Conversation

vassilevsky
Copy link
Contributor

@vassilevsky vassilevsky commented Dec 1, 2017

Hi 🖖

I thought about all these points that hang out in the queue while it is being filled. If the user did not specify timestamp, then they are stamped with server time upon arrival. This is not strictly very precise, eh?

I don't like the need for a separate method. Can something be done to avoid it?

WDYT? 🤔

@dmke
Copy link
Contributor

dmke commented Dec 2, 2017

This looks interesting. Two immediate thougts:

  1. I'm not a fan of mutating methods (i.e. writer.preprocess(data) modifying data) for pure readability reasons. preprocess should instead return a new array.
  2. The preprocessing should be independant of the writer (by moving it into the InfluxDB::Client class) and it should be opt-in (i.e. with a config option).

@kaspergrubbe
Copy link

I was digging into the source code to understand what happens in the case of setting the clients timeprecision to ns but leaving it out when writing data, I think the README covers it by:

If you write data points with sub-second resolution, you have to configure your client instance with a more granular time_precision option and you need to provide timestamp values which reflect this precision. If you don't do this, your points will be squashed!

I believe @vassilevsky changes are very needed, I mean, why specify it in the client if the writes are ignoring this setting?

This forces us to write a wrapper class that handles this, but it would be nice if the client by itself did it.

The preprocessing should be independant of the writer (by moving it into the InfluxDB::Client class) and it should be opt-in (i.e. with a config option).

Couldn't the opt-in be setting the timeprecision setting to sub-second values? Are there a case for having the client set with a sub-second timeprecision without wanting sub-second precision?

@kaspergrubbe
Copy link

@vassilevsky There seems to be a bit of a problem with how #clock_gettime deals with input. If it gets passed nil it will still work, so we might want to throw an error in case someone passes a time_precision that are not understood.

@kaspergrubbe
Copy link

Hi @vassilevsky, I tried to use your code on OSX to test with, and the result didn't seem correct to me, I have written about it here: #219 , do you know if I am doing anything wrong?

@vassilevsky
Copy link
Contributor Author

Thank you for feedback. I posted this as an idea in the first place, to get the conversation going.

Sadly, I don't have energy to make a polished solution. I switched from Ruby to BEAM languages recently. Feel free to use my code or roll your own.

@dmke dmke closed this in 72cfa27 Aug 23, 2018
@dmke
Copy link
Contributor

dmke commented Aug 23, 2018

Oops. This was accidentally closed by a typo. The commit 72cfa27 ought to have referenced #221, not #211.

@dmke dmke reopened this Aug 23, 2018
@Esity
Copy link
Collaborator

Esity commented Apr 29, 2020

I am going to close this PR. Feel free to reopen if we have start working on this again

@Esity Esity closed this Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants