InfluxCommunity / influxdb-ruby

Ruby client for InfluxDB
MIT License
370 stars 133 forks source link

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

Closed vassilevsky closed 4 years ago

vassilevsky commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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: https://github.com/influxdata/influxdb-ruby/issues/219 , do you know if I am doing anything wrong?

vassilevsky commented 6 years ago

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 commented 6 years ago

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

Esity commented 4 years ago

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