HDDS-9933. Recon datanode 'Last Heartbeat' should print relative values

[xBis7]

What changes were proposed in this pull request?

Recon's frontend receives a timestamp number from the backend and then formats the number as a date. It's more useful to the cluster admin and practical, to view the Last Heartbeat as a relative value than an entire date.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-9933

How was this patch tested?

The patch was tested manually, see the attached screenshot.

Also, because there are no unit tests for Recon's frontend, the method was tested externally. Here are some sample values

[LOG]: "31s ago" 
[LOG]: "50s ago" 
[LOG]: "1m 0s ago" 
[LOG]: "1m 2s ago" 
[LOG]: "1m 15s ago" 
[LOG]: "1m 16s ago" 
[LOG]: "1m 33s ago" 
[LOG]: "2m 54s ago"
[LOG]: "7m 17s ago" 

BTW, this what 'Last Heartbeat' looks like in master

[kerneltime]

cc @devmadhuu

[devmadhuu]

@xBis7 thanks for working on this patch. Changes LGTM +1 .

[xBis7]

@devmadhuu Thanks for the review. Do you know anyone else that I can ping, who's working on Recon's frontend?

[devmadhuu]

@devmadhuu Thanks for the review. Do you know anyone else that I can ping, who's working on Recon's frontend?

@devabhishekpal @smitajoshi12 could you pls review

[smitajoshi12]

@xBis7

We have implemneted similar functionality uisng one line code using moment library. Can you check associated code so we can apply similar logic on column.

moment(lastUpdatedOMDBDelta).fromNow()

[xBis7]

moment(lastUpdatedOMDBDelta).fromNow()

@smitajoshi12 That was my first approach to this but moment().fromNow() gives pretty vague results like a few seconds ago, a minute ago, ...

In the last heartbeat scenario default values are up to 30/60 seconds and we need the UI values to be more precise than a few seconds ago.

https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsConfigKeys.java#L25-L32

[smitajoshi12]

@xBis7
Can You check values with DB.json manually editing old values and UI some negative test cases also and cluster data

[devabhishekpal]

@xBis7, fromNow() is expected to give vague descriptions like this.
Here is the breakdown of what is displayed for what time range.

[xBis7]

Can You check values with DB.json manually editing old values and UI some negative test cases also and cluster data

@smitajoshi12 Yes, sure. I'll post some screenshots. When saying negative test cases, you mean like provide values from dates later than the current? Can you explain more?

@xBis7, fromNow() is expected to give vague descriptions like this.
Here is the breakdown of what is displayed for what time range.

@abhishekaypurohit Thanks for the link and the clarification. That's what I'm saying, that moment().fromNow() might be convenient but the values are pretty vague for this use case. Our operations team wants to see precise values, 2s ago, 3s ago, ...

[devabhishekpal]

Hello @xBis7,
Can you refactor the code for getTimeDiffFromTimestamp to shorter form?

const getTimeDiffFromTimestamp = (timestamp: number): string => {
  const timestampDate = new Date(timestamp);
  const currentDate = new Date();

  let elapsedTime = "";
  let duration: moment.Duration = moment.duration(
    moment(currentDate).diff(moment(timestampDate))
  )

  const durationKeys = ["seconds", "minutes", "hours", "days", "months", "years"]
  durationKeys.forEach((k) => {
    let time = duration["_data"][k]
    if (time !== 0){
      elapsedTime = time + `${k.substring(0, 1)} ` + elapsedTime
    }
  })

  return elapsedTime.trim().length === 0 ? "Just now" : elapsedTime.trim() + " ago";
}

This would give the appropriate output. Please let me know if you have any questions and thanks a lot for working on this improvement

[xBis7]

Thanks @devabhishekpal, your code snippet looks good. I'll push the changes shortly.

@smitajoshi12 Here is a screenshot from pnpm run dev with some more recent values in db.json. Let me know if this isn't what you meant.

[devabhishekpal]

This looks good to me @xBis7.
Thanks a lot for working on this improvement.
+1 from me.

[xBis7]

@smitajoshi12 Any comments? Do you want to another look? Otherwise, I'll merge it once there is a green CI.

[smitajoshi12]

@smitajoshi12 Any comments? Do you want to another look? Otherwise, I'll merge it once there is a green CI.

@xBis7
Thanks Before substring can check with && otherwise substring will fail UI will break. If We get 0 value LastHeartbeat or if there is no data then check UI is failing or not. Apart from It looks good to me. Thanks for this patch

[xBis7]

If We get 0 value LastHeartbeat or if there is no data then check UI is failing or not.

@smitajoshi12 If heartbeat is anything other than a positive number, we get NA.

return heartbeat > 0 ? getTimeDiffFromTimestamp(heartbeat) : 'NA';

The values I provided in db.json were

• asd for localhost2
• -1 for localhost3
• 0 for localhost4

Check screenshot. We get NA in all cases.

[smitajoshi12]

If We get 0 value LastHeartbeat or if there is no data then check UI is failing or not.

@smitajoshi12 If heartbeat is anything other than a positive number, we get NA.

return heartbeat > 0 ? getTimeDiffFromTimestamp(heartbeat) : 'NA';

The values I provided in db.json were

• asd for localhost2
• -1 for localhost3
• 0 for localhost4

Check screenshot. We get NA in all cases.

@xBis7
Thanks It looks good to me

[xBis7]

Thanks @devmadhuu @smitajoshi12 @devabhishekpal for the review.