NIFI-15989: Improve purge threshold time formatting

[ohnoitsyou]

Update the purge threshold output to be more human readable.

Existing output will show the value in milliseconds which can be
difficult to read quickly. Convert milliseconds to a Duration to get the
threshold in a format like "30 days", "7 days 4 hours", etc.

Summary

NIFI-15989

Specifically in WriteAheadStorePartition, the maintenance task outputs the threshold as a measure of milliseconds which can be difficult to parse quickly.

Pass the value of the field olderThan that's passed to purgeOldEvents through a new formatting function in nifi-utils to convert it into a words representation. e.g.:

• 2592000000 -> "30 days 0 hours 0 minutes 0 seconds 0 ns"
• 2592010000 -> "30 days 0 hours 0 minutes 10 seconds 0 ns"
• 3600000 -> "1 hour 0 minutes 0 seconds 0 ns"

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
• Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using ./mvnw clean install -P contrib-check
  • JDK 21
  • JDK 25

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[exceptionfactory]

Thanks for proposing this improvement @ohnoitsyou.

Although other modules include the commons-lang3 dependency, introducing this dependency for the sole purpose of improving the logging message does not seem worth the addition.

[ohnoitsyou]

I had considered adding a function to the FormatUtils located in commons. Unfortunately it also doesn't have lang3 in it, but it would centralize the functionality.

The end user this request came from only mentioned this particular log. I'm sure there are other places it would make sense to have a log message formatted in this way.

[exceptionfactory]

In relation to log formatting, additional dependencies should be avoided.

There is certainly a balance to strike between readability and consistency, but in this case, the log message already includes the number of units, which also makes it more consistent for potential parsing.

With that background, this doesn't seem like a good candidate for changing.

[dan-s1]

@exceptionfactory Could the org.apache.nifi.time.DurationFormat found in the nifi-api be used for this? Or does this also have the limitation that the nifi-api is not currently a dependency?

[ohnoitsyou]

@dan-s1 Unfortunately DurationFormat doesn't include a string based output.

[dan-s1]

@dan-s1 Unfortunately DurationFormat doesn't include a string based output.

Yeah I realize. I thought it might have been included as a dependency and you would have to still add some code to produce a string output.

[ohnoitsyou]

@exceptionfactory If we still wanted to pursue something like this where the message is more expressive about the time frame, is there something you might suggest?

Obviously this in a single place doesn't make a huge impact, but I bet if I did some digging I could find lots of places where having a large number of millis formatted in words would improve readability.

[mosermw]

@ohnoitsyou I think it would be acceptable to have a new method in the nifi-utils.jar FormatUtils class that does this. Perhaps named formatTimeDuration, it would produce a String containing the largest "whole number with TimeUnit" that a duration fits in. It's a concept similar to the existing FormatUtils formatDataSize method.

To get an idea how to implement it, check out the nifi-api DurationFormat class. It has some private methods that do something similar.

[exceptionfactory]

Just a caution on adding some new method, although it might be useful, I'm not sure it would make sense to change log messages. It might in very specific cases, but it would be a case-by-case question.

[mosermw]

If this specific log message should not be changed, then we should close this PR. However, I will say that I have also wondered if this specific log message could be improved to make it more human readable.

[exceptionfactory]

Taking a fresh look at this, I could see adjusting the log message to include both the original number and units, plus something descriptive, immediately after, in parentheses. This kind of approach is similar to displaying seconds and nanoseconds on application startup. That would still require a new method in nifi-utils, but I could see that as a way forward.

[ohnoitsyou]

@exceptionfactory @mosermw I took a stab at implementing a duration to words function in nifi-utils.

I also converted the EventStorePartition interface from from TimeUnit to ChronoUnit partly because ChronoUnit is part of the java.util.temporal package vs java.util.concurrent and it felt more appropriate for dealing with time. If that's not OK, I can go back.

[ohnoitsyou]

@exceptionfactory Not to bug too much, but if this could be reviewed, I found a place where the formatting function would be useful in another related class and I'd like to take advantage of it.

[exceptionfactory]

Thanks for continuing the work on this @ohnoitsyou. The new format methods look good in general, I noted a few stylistic recommendations, but this looks close to completion.

[ohnoitsyou]

@exceptionfactory Got those updated. If I could get the workflows approved, hopefully we can push this over the finish line.

[exceptionfactory]

Thanks for the work on this @ohnoitsyou. It looks close to completion, just one general question on formatting.

The spelled out words make the description rather long (1 day 2 hours 10 minutes). What do you think about just using single letters like 1d 2h 10m 30s 100ns? It is still readable, but less verbose, and doesn't need the pluralization.

[exceptionfactory]

The other approach would be to implement some kind of rounding to the greatest two units (days and hours, hours and minutes, etc), but the single-letter abbreviation might be a way to keep the precision with readability.

[ohnoitsyou]

Personally, I'd rather abbreviate and be more precise than truncate data. It's still readable while being concise.

Edit: that is to say, I'm good with the change.

[exceptionfactory]

Personally, I'd rather abbreviate and be more precise than truncate data. It's still readable while being concise.

Edit: that is to say, I'm good with the change.

Thanks, if you like the abbreviation approach, that sounds good to me.

[ohnoitsyou]

@exceptionfactory Pushed that change, builds on my end. If you could approve the workflows, that'd be appreciated. Trying to keep it up-to-date with main

[exceptionfactory]

Thanks @ohnoitsyou, the latest set of changes look good. I rebased the branch and plan to merge on successful build completion

[exceptionfactory]

Thanks for working through this and moving it to completion @ohnoitsyou! +1 merging