Skip to content

Conversation

@bednar
Copy link
Member

@bednar bednar commented Jul 9, 2024

Closes #154

Proposed Changes

This PR modifies how the client parses timestamps. Recently, InfluxDB 3 began returning timestamps in nanoseconds, necessitating a change in the parsing method. The parsing is now more robust.

image

https://app.circleci.com/pipelines/github/InfluxCommunity/influxdb3-java/510/workflows/51f380f3-1cd0-49bd-bb11-fe2f72cb34dc/jobs/3031?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link&utm_content=summary

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • Tests pass
  • Commit messages are conventional
  • Sign CLA (if not already signed)

@bednar bednar marked this pull request as ready for review July 9, 2024 12:34
@bednar bednar requested a review from karel-rehor July 9, 2024 12:34
@codecov
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.19%. Comparing base (8892141) to head (4bafab9).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   89.03%   89.19%   +0.16%     
==========================================
  Files          16       16              
  Lines         866      879      +13     
  Branches      131      133       +2     
==========================================
+ Hits          771      784      +13     
  Misses         40       40              
  Partials       55       55              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@karel-rehor karel-rehor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests pass locally with openjdk 20.0.2. Changes make sense.

@sahil-devinci
Copy link

Are you sure it will work? I copied the above code and converted the timestamp as per above code, year is all wrong.

pointValue timestamp: 1721051820455000000000000

long nanoseconds = TimeUnit.NANOSECONDS.convert((Long) dateTime.longValue(), TimeUnit.NANOSECONDS);
		Instant.ofEpochSecond(0, nanoseconds);

2207-05-30T09:54:06.253330432Z

@karel-rehor
Copy link
Contributor

@sahil-devinci

I'm curious where you got that timestamp as it would be base femto or 10^-15.

e.g.

1721134115000000000 - nano
1721051820455000000000000 - femto 

Just to be sure I've repeated the implementation in local test code and everything looks correct.

e.g.

  Instant now = Instant.now();
  System.out.println("now " + now);
  ...
  Long seconds = now.getEpochSecond();
  long nsFromS = java.util.concurrent.TimeUnit.NANOSECONDS.convert(seconds, java.util.concurrent.TimeUnit.SECONDS);
  System.out.println("DEBUG nsFromS " + nsFromS);
  System.out.println("DEBUG nsFromS as instant " + Instant.ofEpochSecond(0, nsFromS));

From the console:

...
DEBUG nsFromS 1721134115000000000
DEBUG nsFromS as instant 2024-07-16T12:48:35Z
...

I also checked the value in your comment.

  // reduced to nano
    long nsFromX = java.util.concurrent.TimeUnit.NANOSECONDS.convert(1_721_051_820_455_000_000L,
      java.util.concurrent.TimeUnit.NANOSECONDS);
    System.out.println("DEBUG nsFromX " + nsFromX);
    System.out.println("DEBUG nsFromX as instant " + Instant.ofEpochSecond(0, nsFromX));
  // original
    BigInteger bi = new BigInteger("1721051820455000000000000");
    long nsFromX2 = java.util.concurrent.TimeUnit.NANOSECONDS.convert(
      bi.longValue(),
      java.util.concurrent.TimeUnit.NANOSECONDS);
    System.out.println("DEBUG nsFromX2 " + nsFromX2);
    System.out.println("DEBUG nsFromX2 as instant " + Instant.ofEpochSecond(0, nsFromX2));

If you reduce the value back to nano from femto you get a timestamp from Monday July 15 2024.

Some commented results:

1721133051000000000 // base nano 10^-9 => 2024-07-16T12:30:51Z
1721051820455000000 // ^^^ Ditto ^^^^^ => 2024-07-15T13:57:00.455Z

1721051820455000000000000 // base femto 10^-15  over flows MAX_LONG at least once resulting in 
7491866046253330432 // base nano 10^-9 => 2207-05-30T09:54:06.253330432Z

@sahil-devinci
Copy link

sahil-devinci commented Jul 16, 2024

@sahil-devinci

I'm curious where you got that timestamp as it would be base femto or 10^-15.

e.g.

1721134115000000000 - nano
1721051820455000000000000 - femto 

Just to be sure I've repeated the implementation in local test code and everything looks correct.

e.g.

  Instant now = Instant.now();
  System.out.println("now " + now);
  ...
  Long seconds = now.getEpochSecond();
  long nsFromS = java.util.concurrent.TimeUnit.NANOSECONDS.convert(seconds, java.util.concurrent.TimeUnit.SECONDS);
  System.out.println("DEBUG nsFromS " + nsFromS);
  System.out.println("DEBUG nsFromS as instant " + Instant.ofEpochSecond(0, nsFromS));

From the console:

...
DEBUG nsFromS 1721134115000000000
DEBUG nsFromS as instant 2024-07-16T12:48:35Z
...

I also checked the value in your comment.

  // reduced to nano
    long nsFromX = java.util.concurrent.TimeUnit.NANOSECONDS.convert(1_721_051_820_455_000_000L,
      java.util.concurrent.TimeUnit.NANOSECONDS);
    System.out.println("DEBUG nsFromX " + nsFromX);
    System.out.println("DEBUG nsFromX as instant " + Instant.ofEpochSecond(0, nsFromX));
  // original
    BigInteger bi = new BigInteger("1721051820455000000000000");
    long nsFromX2 = java.util.concurrent.TimeUnit.NANOSECONDS.convert(
      bi.longValue(),
      java.util.concurrent.TimeUnit.NANOSECONDS);
    System.out.println("DEBUG nsFromX2 " + nsFromX2);
    System.out.println("DEBUG nsFromX2 as instant " + Instant.ofEpochSecond(0, nsFromX2));

If you reduce the value back to nano from femto you get a timestamp from Monday July 15 2024.

Some commented results:

1721133051000000000 // base nano 10^-9 => 2024-07-16T12:30:51Z
1721051820455000000 // ^^^ Ditto ^^^^^ => 2024-07-15T13:57:00.455Z

1721051820455000000000000 // base femto 10^-15  over flows MAX_LONG at least once resulting in 
7491866046253330432 // base nano 10^-9 => 2207-05-30T09:54:06.253330432Z

I am getting it from pointValue.getTimestamp. I am using version 0.8.0 of this library.

`try (Stream stream = influxDBClientThreePhase.queryPoints(formattedQuery)) {
stream.forEach((PointValues p) -> extractPoints(queryHealthTrendList, p, localZoneId));
}

public List<ThreePhaseHealthTrend> extractPoints(List<ThreePhaseHealthTrend> list, PointValues p, ZoneId zoneId) {
	ThreePhaseHealthTrend metrics = new ThreePhaseHealthTrend();
	// Power
	metrics.setPA(p.getField("actPowerA", Double.class));
	metrics.setPB(p.getField("actPowerB", Double.class));
	metrics.setPC(p.getField("actPowerC", Double.class));
	// Current
	metrics.setCA(p.getField("currentA", Double.class));
	metrics.setCB(p.getField("currentB", Double.class));
	metrics.setCC(p.getField("currentC", Double.class));

	// PF
	metrics.setPfA(p.getField("pfA", Double.class));
	metrics.setPfB(p.getField("pfB", Double.class));
	metrics.setPfC(p.getField("pfC", Double.class));
	// Voltage
	metrics.setVA(p.getField("voltageA", Double.class));
	metrics.setVB(p.getField("voltageB", Double.class));
	metrics.setVC(p.getField("voltageC", Double.class));

	metrics.setTime(DateTimeUtils.fromInfluxInstantToSecond(**p.getTimestamp()**, zoneId));			
			`

p.getTimestamp() is the value I shared above. And when I run the query directly in influx db serverless it shows correct value.

@bednar
Copy link
Member Author

bednar commented Jul 17, 2024

Hi @sahil-devinci,

thanks for insight in this issue.

Are you sure it will work? I copied the above code and converted the timestamp as per above code, year is all wrong.

pointValue timestamp: 1721051820455000000000000

long nanoseconds = TimeUnit.NANOSECONDS.convert((Long) dateTime.longValue(), TimeUnit.NANOSECONDS);
		Instant.ofEpochSecond(0, nanoseconds);

2207-05-30T09:54:06.253330432Z

What type is the dateTime?

I am getting it from pointValue.getTimestamp. I am using version 0.8.0 of this library.

`try (Stream stream = influxDBClientThreePhase.queryPoints(formattedQuery)) { stream.forEach((PointValues p) -> extractPoints(queryHealthTrendList, p, localZoneId)); }

public List<ThreePhaseHealthTrend> extractPoints(List<ThreePhaseHealthTrend> list, PointValues p, ZoneId zoneId) {
	ThreePhaseHealthTrend metrics = new ThreePhaseHealthTrend();
	// Power
	metrics.setPA(p.getField("actPowerA", Double.class));
	metrics.setPB(p.getField("actPowerB", Double.class));
	metrics.setPC(p.getField("actPowerC", Double.class));
	// Current
	metrics.setCA(p.getField("currentA", Double.class));
	metrics.setCB(p.getField("currentB", Double.class));
	metrics.setCC(p.getField("currentC", Double.class));

	// PF
	metrics.setPfA(p.getField("pfA", Double.class));
	metrics.setPfB(p.getField("pfB", Double.class));
	metrics.setPfC(p.getField("pfC", Double.class));
	// Voltage
	metrics.setVA(p.getField("voltageA", Double.class));
	metrics.setVB(p.getField("voltageB", Double.class));
	metrics.setVC(p.getField("voltageC", Double.class));

	metrics.setTime(DateTimeUtils.fromInfluxInstantToSecond(**p.getTimestamp()**, zoneId));			
			`

p.getTimestamp() is the value I shared above. And when I run the query directly in influx db serverless it shows correct value.

The version 0.8.0 has an issue which is addressed by this PR. The com.influxdb.v3.client.PointValues#getTimestamp should always return nanoseconds precision.

Can you please test your code with the fixed version of the client from this PR?

@sahil-devinci
Copy link

sahil-devinci commented Jul 17, 2024 via email

@bednar
Copy link
Member Author

bednar commented Jul 22, 2024

@sahil-devinci the [com.influxdb.v3.client.PointValues#getTimestamp] should always return the timestamp in nanoseconds. I've updated the JavaDoc in this PR.

@bednar bednar merged commit 87cd880 into main Jul 22, 2024
@bednar bednar deleted the parsing-timestamp branch July 22, 2024 13:29
@bednar bednar added this to the 0.9.0 milestone Jul 22, 2024
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.

Odd Timestamp Precision Behaviour

4 participants