Uploaded image for project: 'Apache NiFi'
  1. Apache NiFi
  2. NIFI-3719

Timezone error in 5-minute statistics processing time

    Details

    • Type: Bug
    • Status: Patch Available
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.2.0, 1.1.1
    • Fix Version/s: None
    • Component/s: Core UI
    • Labels:
    • Environment:
      Windows 10 and Java 1.8.0_121.
      Also CentOS 7 with unknown Java version, but at least 1.7.

      Description

      With timezone set to Australia\Sydney (AEST, UCT+10), each processor shows Task / Time of 0 / 00:00:00.000.
      With timezone set to Australia\Adelaide (ACST, UCT+9:30), each processor shows Task / Time of 0 / 00:30:00.000.

        Activity

        Hide
        mhopskip Michael Hopton added a comment -

        Nifi-3719 patch

        Show
        mhopskip Michael Hopton added a comment - Nifi-3719 patch
        Hide
        joewitt Joseph Witt added a comment -

        Thanks for your contribution.

        Have removed fix version until there is review traction/close to merge.

        Given the utility nature of this method I dont think we can simply force it to use UTC as it might cause behavior changes elsewhere. We should consider adding new method signatures that let the effective timezone be set explicitly, based on UTC, or whatever comes from the local system.

        Show
        joewitt Joseph Witt added a comment - Thanks for your contribution. Have removed fix version until there is review traction/close to merge. Given the utility nature of this method I dont think we can simply force it to use UTC as it might cause behavior changes elsewhere. We should consider adding new method signatures that let the effective timezone be set explicitly, based on UTC, or whatever comes from the local system.
        Hide
        mhopskip Michael Hopton added a comment -

        The methods are used to display durations, not times. Durations should not have a timezone. 20 seconds is 20 seconds irrespective of where in the world you are. However, Nifi thinks 20 seconds is 30:20 because of where I live.

        In Java 8, the Duration class from java.date should be used instead of using the Date class to represent a duration.
        The bug is caused by formatting durations as times.

        Show
        mhopskip Michael Hopton added a comment - The methods are used to display durations, not times. Durations should not have a timezone. 20 seconds is 20 seconds irrespective of where in the world you are. However, Nifi thinks 20 seconds is 30:20 because of where I live. In Java 8, the Duration class from java.date should be used instead of using the Date class to represent a duration. The bug is caused by formatting durations as times.
        Hide
        mcgilman Matt Gilman added a comment -

        Thanks for the patch!

        I believe the signatures of the methods in question would allow us to implement the formatting however we want to. If the bug is caused by formatting durations as times, should the proposed patch be updated to use the appropriate Duration class that Michael Hopton is suggesting?

        Show
        mcgilman Matt Gilman added a comment - Thanks for the patch! I believe the signatures of the methods in question would allow us to implement the formatting however we want to. If the bug is caused by formatting durations as times, should the proposed patch be updated to use the appropriate Duration class that Michael Hopton is suggesting?
        Hide
        mhopskip Michael Hopton added a comment -

        I think it would be ideal to migrate all dates, times and duration to the new Java 8 classes, but clearly that is a huge amount of effort. It also requires design to consider which class is correct for each date/time/duration. As such I suggest it might be a low priority.

        Show
        mhopskip Michael Hopton added a comment - I think it would be ideal to migrate all dates, times and duration to the new Java 8 classes, but clearly that is a huge amount of effort. It also requires design to consider which class is correct for each date/time/duration. As such I suggest it might be a low priority.
        Hide
        mcgilman Matt Gilman added a comment -

        Your correct in that it would be a large effort. Some of this effort would even affect some public APIs (specifically around the data model into/out of the REST API). Due to these additional hurdles, it may make sense to introduce these fixes iteratively as they surface. I'm happy to continue working with you on this specific issue if you'd like. Thanks again!

        Show
        mcgilman Matt Gilman added a comment - Your correct in that it would be a large effort. Some of this effort would even affect some public APIs (specifically around the data model into/out of the REST API). Due to these additional hurdles, it may make sense to introduce these fixes iteratively as they surface. I'm happy to continue working with you on this specific issue if you'd like. Thanks again!
        Hide
        mhopskip Michael Hopton added a comment -

        I've looked a bit more closely and java.time does not provide formatting for Duration, which seems to me to be a significant omission. As a consequence, changing to Duration ends up being completely unrelated to the bug reported and could be raised as a separate issue if there is a desire to refactor the code.

        I think we should stick with the patch provided to resolve the bug.

        Show
        mhopskip Michael Hopton added a comment - I've looked a bit more closely and java.time does not provide formatting for Duration, which seems to me to be a significant omission. As a consequence, changing to Duration ends up being completely unrelated to the bug reported and could be raised as a separate issue if there is a desire to refactor the code. I think we should stick with the patch provided to resolve the bug.

          People

          • Assignee:
            Unassigned
            Reporter:
            mhopskip Michael Hopton
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:

              Development