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

Fix #614: drop NaN and infinity values from fields #616

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

piotrp
Copy link
Contributor

@piotrp piotrp commented Aug 21, 2019

Fox for #614 - drop NaN and +/- infinity values from fields while serializing Point to line protocol. They are unsupported by InfluxDB and all issues relating to them are old and got no progress.

I went with the simplest solution, assuming that if these values will be supported in the future then probably a change in driver will be required, unless InfluxDB aligns exactly with current Java serialization (NaN to U+FFFD - invalid character, and infinity to ).

@majst01
Copy link
Collaborator

majst01 commented Aug 22, 2019

Hi @Crack,
thanks for the clean patch, would you please add a changelog entry as well ? thanks.

@piotrp
Copy link
Contributor Author

piotrp commented Aug 22, 2019

Sure - added.

@majst01
Copy link
Collaborator

majst01 commented Aug 22, 2019

Hi,
one more nit, can you please clarify more (in the javadoc) where influxdb doc states that behavior

@piotrp
Copy link
Contributor Author

piotrp commented Aug 22, 2019

Added. Officially InfluxDB supports IEEE-754 floating points, but documentation makes no mention on NaN and infinity being unsupported, so I linked to GitHub issue: influxdata/influxdb#4089

They are unsupported by InfluxDB
@majst01
Copy link
Collaborator

majst01 commented Aug 22, 2019

But reading this issue, there is no response from some of the influx guys ? I am unsure if we should drop these values silently ?

@piotrp
Copy link
Contributor Author

piotrp commented Aug 22, 2019

Use case I was thinking about when developing this is an application reporting Kafka metrics, namely: apache/flink#8513 (and some of my internal ones that use Spring Boot).

There are two goals I'd like to achieve:

  1. Remove useless exception that is written to application log with each collection. That I can work around with selectively targeting log reporter and disabling it (but this can hide some real problems)
  2. Remove useless error logs and make data usable in applications that expect valid line protocol (InfluxDB / Telegraf).

If we want some signaling of these values then I can do some changes around InfluxDB#write to throw an exception after a successful write to InfluxDB, and a flag to switch these off.


Anyway, if InfluxDB starts supporting NaNs or infinities in the future, influxdb-java would need to be changed anyway, because current NaN serialization format outputs unicode garbage and that would need to be changed to something else.

@majst01 majst01 merged commit e20f5b4 into influxdata:master Aug 22, 2019
@majst01
Copy link
Collaborator

majst01 commented Aug 22, 2019

Thanks for the clarification and contribution !

@piotrp
Copy link
Contributor Author

piotrp commented Aug 22, 2019

Thanks!

When do you plan to release a new version?

@piotrp
Copy link
Contributor Author

piotrp commented Sep 18, 2019

@majst01 when do you plan to release a new version?

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.

2 participants