iotdb icon indicating copy to clipboard operation
iotdb copied to clipboard

[Bug] Time-Zone problems in MonthIntervalFillFilterTest

Open chrisdutz opened this issue 1 year ago • 1 comments

Search before asking

  • [X] I searched in the issues and found nothing similar.

Version

1.3.2-SNAPSHOT

Describe the bug and provide the minimal reproduce step

When building IoTDB including tests in my timezone "Europe/Berlin" the tests in MonthIntervalFillFilterTest are failing.

What did you expect to see?

I would have expected the tests to pass.

What did you see instead?

[ERROR] Failures: [ERROR] MonthIntervalFillFilterTest.testMonthIntervalMSFillFilter:51 [ERROR] MonthIntervalFillFilterTest.testMonthIntervalNSFillFilter:133 [ERROR] MonthIntervalFillFilterTest.testMonthIntervalUSFillFilter:90

Anything else?

I have already found the problem, but am not sure how to correctly fix the issue.

The problem is that now 16.05.2024 in Germany we are in Daylight-Saving, which changes the timezone offset from 1h to 2h. The AbstractMonthIntervalFillFilter uses the current timezone offset at the time of creating the instance.

Therefore the interval used in the test is "one hour less than one month", which makes the tests fail.

If I change:

    this.zoneOffset = zone.getRules().getOffset(Instant.now());

To:

     this.zoneOffset = zone.getRules().getStandardOffset(Instant.now());

The test passes again, but I am not sure this is the correct fix for it. Another alternative would be to calculate the zoneOffset in the test based on the current time and not of that of the reference date in February (which is before daylight-saving)

Are you willing to submit a PR?

  • [x] I'm willing to submit a PR!

chrisdutz avatar May 16 '24 08:05 chrisdutz

Digging a bit further, I think it's probably better to not calculate the offset in the constructor of the filter, but to get it from the dates being used.

So removing the zoneOffset from the base class and doing things like this also made the test return to green.

public class MonthIntervalUSFillFilter extends AbstractMonthIntervalFillFilter {

  public MonthIntervalUSFillFilter(int monthDuration, long nonMonthDuration, ZoneId zone) {
    super(monthDuration, nonMonthDuration, zone);
  }

  @Override
  public boolean needFill(long time, long previousTime) {
    long smaller = Math.min(time, previousTime);
    long greater = Math.max(time, previousTime);
    Instant smallerInstant = Instant.ofEpochSecond(smaller / 1_000_000L, smaller % 1_000_000L);
    LocalDateTime smallerDateTime = LocalDateTime.ofInstant(smallerInstant, zone);
    ZoneOffset smallerOffset = zone.getRules().getStandardOffset(smallerInstant);
    Instant upper =
            smallerDateTime
            .plusMonths(monthDuration)
            .plus(nonMonthDuration, MICROS)
            .toInstant(smallerOffset);
    long timeInUs =
        upper.getLong(ChronoField.MICRO_OF_SECOND) + upper.getEpochSecond() * 1_000_000L;
    return timeInUs >= greater;
  }
}

chrisdutz avatar May 16 '24 08:05 chrisdutz