line protocol #23
line protocol #23
Conversation
|
closes #5 |
|
Nice one @ngong! I'll try to take a look at this soon. Maybe we don't need to leave the json protocol around and can just remove it? And in that case we could remove the jackson dependency too. |
ngong
force-pushed
the
line
branch
2 times, most recently
from
ab4f48d
to
47a93ce
Compare
|
Tested, now it is working. |
| return sb.toString(); | ||
| } | ||
|
|
||
| public String lineProtocol(InfluxDbPoint point, TimeUnit precision) { |
There was a problem hiding this comment.
This should also return StringBuilder, alternatively you could add the StringBuilder as a parameter to avoid having to initialize a new one every time. If you want to keep the String return type for the public methods, then you should refactor it, so that the StringBuilder logic is in a private method which is used by getLineProtocolString and the public method just uses toString.
There was a problem hiding this comment.
Actually, there is no need to use StringBuilder at all here
There was a problem hiding this comment.
Why not? You create a new String locally just to append it to in getLineProtocolString to a StringBuilder, I think it would be better to have the StringBuilder as parameter and also to pass it down to the other private methods, that currently create their own StringBuilder, e.g. concatenatedTags, concatenateFields, formattedTime... like @woidda pointed out
There was a problem hiding this comment.
I mean to build string in this method. Sure, it is nice to take StringBuilder as parameter
There was a problem hiding this comment.
I think it is not necessary to keep this method public anymore.
|
Great to see progress on this issue 👍 . |
|
Are there any plans to merge the branch? 0.12.0 is out as @dkerwin mentioned and there seem to be a lot of improvements that would make an upgrade interesting... |
| if (null == time) { | ||
| time = System.nanoTime(); | ||
| } | ||
| sb.append(" ").append(TimeUnit.NANOSECONDS.convert(precision.convert(time, TimeUnit.MILLISECONDS), precision)); |
There was a problem hiding this comment.
I'm guessing that this means we have ms precision where we might want second or minute precision?
There was a problem hiding this comment.
The code here is, to convert ms (which is what we collected from system) to the precision you specified. And then, we need to convert it back to NANOSECONDS which is required by line protocol on passing time stamp
There was a problem hiding this comment.
And the time = System.nanoTime(); is wrong, it should be time = System.currentTimeMillis();
There was a problem hiding this comment.
Or Instant.now().toEpochMilli() to avoid calling the System object? I don't know if this project supports java8 though.
v0.12.0 of influx has removed the deprecated json protocol, this replaces that with their line protocol. References: - https://docs.influxdata.com/influxdb/v0.12/write_protocols/line/ - https://github.com/influxdata/influxdb-java/
- remove string builder from lineProtocol method in InfluxDbWriteObjectSerializer - fix a little bug in time format - used pre-compiled Pattern to escape data - optimized url construction
|
Currently, timestamps are sent in nano seconds, and precision is resolve in the code. However, according to InfluxDb's page, the best practice should be sending precision data in query string, and truncate the date in line protocol to that precision. I will address this option and try if this is working later. |
|
Updated. Tested working fine. So it could potentially reduce some data size when pushing to InfluxDb server. |
ping @nzroller