[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15876529#comment-15876529
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua closed the pull request at:

https://github.com/apache/drill/pull/739


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix, ready-to-commit
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15876528#comment-15876528
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/739
  
This closes PR #739 


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix, ready-to-commit
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15861781#comment-15861781
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/739
  
@paul-rogers Created a new SimpleDurationFormat class. We can expand to 
have more formats, or reimplement on lines of SimpleDateFormat by passing 
format strings in the future. Hope this helps.


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854954#comment-15854954
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99711436
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -352,16 +369,26 @@ public String cancelQuery(@PathParam("queryid") 
String queryId) {
   private void checkOrThrowProfileViewAuthorization(final QueryProfile 
profile) {
 if (!principal.canManageProfileOf(profile.getUser())) {
   throw UserException.permissionError()
-  .message("Not authorized to view the profile of query '%s'", 
profile.getId())
-  .build(logger);
+  .message("Not authorized to view the profile of query '%s'", 
profile.getId())
+  .build(logger);
 }
   }
 
   private void checkOrThrowQueryCancelAuthorization(final String 
queryUser, final String queryId) {
 if (!principal.canManageQueryOf(queryUser)) {
   throw UserException.permissionError()
-  .message("Not authorized to cancel the query '%s'", queryId)
-  .build(logger);
+  .message("Not authorized to cancel the query '%s'", queryId)
+  .build(logger);
 }
   }
+
+  /**
+   * Duration format definition
+   * @author kkhatua
--- End diff --

Drill practice seems to be to omit the author. Many IDE's helpfully add 
this, but you can turn off that option.


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854955#comment-15854955
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99688930
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -73,18 +73,35 @@
* e.g. getPrettyDuration(1468368841695,1468394096016) = '7 hr 00 min 
54.321 sec'
* @param startTimeMillis Start Time in milliseconds
* @param endTimeMillis   End Time in milliseconds
+   * @param format  Display format
* @returnHuman-Readable Elapsed Time
*/
-  public static String getPrettyDuration(long startTimeMillis, long 
endTimeMillis) {
+  public static String getPrettyDuration(long startTimeMillis, long 
endTimeMillis, DurationFormat format) {
--- End diff --

It is often cleaner to just have two methods, rather than one method with a 
"command". Since we need to split out the data into a bunch of fields, this can 
be done by another method that creates a structure. Then, since you've created 
the structure, it might as well be the class that does the format, and offer 
two format methods: compact and verbose.


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854909#comment-15854909
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/739
  
@paul-rogers , @sudheeshkatkam 
Committed changes based on your recommendations. I also noticed that some 
of the calls were passing fragment IDs as links, which were never applied. 
Based on the file history, my hunch is that it was meant for debugging and 
forgotten ever since. Corrected it by replacing these with the appropriate 
`null` values.


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15851874#comment-15851874
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99393776
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
 ---
@@ -56,6 +47,30 @@ public TableBuilder(final String[] columns) {
 sb.append("\n");
   }
 
+  /**
+   * Representation of a millisecond duration in a short readable text
+   * @param millis  Duration in milliseconds
+   * @returnHuman-Readable Duration Time
+   */
+  public String shortDurationFormat(long millis) {
+long days = TimeUnit.MILLISECONDS.toDays(millis);
+long hours = TimeUnit.MILLISECONDS.toHours(millis) - 
TimeUnit.DAYS.toHours(TimeUnit.MILLISECONDS.toDays(millis));
+long minutes = TimeUnit.MILLISECONDS.toMinutes(millis) - 
TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(millis));
+long seconds = TimeUnit.MILLISECONDS.toSeconds(millis) - 
TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(millis));
+long milliSeconds = millis - 
TimeUnit.SECONDS.toMillis(TimeUnit.MILLISECONDS.toSeconds(millis));
+String formattedDuration = "";
+if (days >= 1) {
+  formattedDuration = days + "d" + hours + "h" + minutes + "m";
+} else if (hours >= 1) {
+  formattedDuration = hours + "h" + minutes + "m";
+} else if (minutes >= 1) {
+  formattedDuration = minutes + "m" + seconds + "s";
+} else {
+  formattedDuration = seconds + "." + milliSeconds + "s";
--- End diff --

Good catch. I'll update the test case as well and apply the String.format()


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15851871#comment-15851871
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99393441
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
 ---
@@ -56,6 +47,30 @@ public TableBuilder(final String[] columns) {
 sb.append("\n");
   }
 
+  /**
+   * Representation of a millisecond duration in a short readable text
+   * @param millis  Duration in milliseconds
+   * @returnHuman-Readable Duration Time
+   */
+  public String shortDurationFormat(long millis) {
+long days = TimeUnit.MILLISECONDS.toDays(millis);
+long hours = TimeUnit.MILLISECONDS.toHours(millis) - 
TimeUnit.DAYS.toHours(TimeUnit.MILLISECONDS.toDays(millis));
+long minutes = TimeUnit.MILLISECONDS.toMinutes(millis) - 
TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(millis));
+long seconds = TimeUnit.MILLISECONDS.toSeconds(millis) - 
TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(millis));
+long milliSeconds = millis - 
TimeUnit.SECONDS.toMillis(TimeUnit.MILLISECONDS.toSeconds(millis));
+String formattedDuration = "";
+if (days >= 1) {
+  formattedDuration = days + "d" + hours + "h" + minutes + "m";
--- End diff --

This display format is kept compact due to limited space within the cells 
of the table. Hence, we aim to keep only the first two major units. Hence, if 
something runs in hours, we report down to the minute, but not any more 
granular.


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15851805#comment-15851805
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99385089
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
 ---
@@ -56,6 +47,30 @@ public TableBuilder(final String[] columns) {
 sb.append("\n");
   }
 
+  /**
+   * Representation of a millisecond duration in a short readable text
+   * @param millis  Duration in milliseconds
+   * @returnHuman-Readable Duration Time
+   */
+  public String shortDurationFormat(long millis) {
+long days = TimeUnit.MILLISECONDS.toDays(millis);
+long hours = TimeUnit.MILLISECONDS.toHours(millis) - 
TimeUnit.DAYS.toHours(TimeUnit.MILLISECONDS.toDays(millis));
+long minutes = TimeUnit.MILLISECONDS.toMinutes(millis) - 
TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(millis));
+long seconds = TimeUnit.MILLISECONDS.toSeconds(millis) - 
TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(millis));
+long milliSeconds = millis - 
TimeUnit.SECONDS.toMillis(TimeUnit.MILLISECONDS.toSeconds(millis));
+String formattedDuration = "";
+if (days >= 1) {
+  formattedDuration = days + "d" + hours + "h" + minutes + "m";
+} else if (hours >= 1) {
+  formattedDuration = hours + "h" + minutes + "m";
+} else if (minutes >= 1) {
+  formattedDuration = minutes + "m" + seconds + "s";
+} else {
+  formattedDuration = seconds + "." + milliSeconds + "s";
--- End diff --

The above won't work in general. If the time is 2 ms, you'll get an output 
of 0.2s, which is not quite right...

Here, consider using the very handy `String.format` method:
```
String.format("%.3f", seconds + milliSeconds / 1000.0 );
```


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix, ready-to-commit
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15851804#comment-15851804
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99385541
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
 ---
@@ -56,6 +47,30 @@ public TableBuilder(final String[] columns) {
 sb.append("\n");
   }
 
+  /**
+   * Representation of a millisecond duration in a short readable text
+   * @param millis  Duration in milliseconds
+   * @returnHuman-Readable Duration Time
+   */
+  public String shortDurationFormat(long millis) {
--- End diff --

Used elsewhere? If so, consider making static (so we don't need an 
instance.) If only used here, consider making private.

In fact, maybe move this out to a utility class. This logic can be used 
anywhere we want to display a duration such as in logs, in tests, etc.


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix, ready-to-commit
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15851803#comment-15851803
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99384748
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
 ---
@@ -56,6 +47,30 @@ public TableBuilder(final String[] columns) {
 sb.append("\n");
   }
 
+  /**
+   * Representation of a millisecond duration in a short readable text
+   * @param millis  Duration in milliseconds
+   * @returnHuman-Readable Duration Time
+   */
+  public String shortDurationFormat(long millis) {
+long days = TimeUnit.MILLISECONDS.toDays(millis);
+long hours = TimeUnit.MILLISECONDS.toHours(millis) - 
TimeUnit.DAYS.toHours(TimeUnit.MILLISECONDS.toDays(millis));
+long minutes = TimeUnit.MILLISECONDS.toMinutes(millis) - 
TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(millis));
+long seconds = TimeUnit.MILLISECONDS.toSeconds(millis) - 
TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(millis));
+long milliSeconds = millis - 
TimeUnit.SECONDS.toMillis(TimeUnit.MILLISECONDS.toSeconds(millis));
+String formattedDuration = "";
+if (days >= 1) {
+  formattedDuration = days + "d" + hours + "h" + minutes + "m";
--- End diff --

Is this the display format? Maybe add some space:

```
5d 14h 2m
```

Instead of:
```
5d14h2m
```


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix, ready-to-commit
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15851806#comment-15851806
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99385729
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
 ---
@@ -78,29 +93,8 @@ public void appendTime(final long d, final String link) {
   }
 
   public void appendMillis(final long p, final String link) {
-final double secs = p/1000.0;
-final double mins = secs/60;
-final double hours = mins/60;
-final double days = hours / 24;
-SimpleDateFormat timeFormat = null;
-if (days >= 10) {
-  timeFormat = this.days;
-} else if (days >= 1) {
-  timeFormat = this.sdays;
-} else if (hours >= 10) {
-  timeFormat = this.hours;
-}else if(hours >= 1){
-  timeFormat = this.shours;
-}else if (mins >= 10){
-  timeFormat = this.mins;
-}else if (mins >= 1){
-  timeFormat = this.smins;
-}else if (secs >= 10){
-  timeFormat = this.secs;
-}else {
-  timeFormat = this.ssecs;
-}
-appendCell(timeFormat.format(new Date(p)), null);
+String shortReadableDuration = shortDurationFormat(p);
+appendCell(shortReadableDuration, link);
--- End diff --

In Drill, we like brevity:
```
appendCell(shortDurationFormat(p), link);
```


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix, ready-to-commit
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15851807#comment-15851807
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/739#discussion_r99385390
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
 ---
@@ -56,6 +47,30 @@ public TableBuilder(final String[] columns) {
 sb.append("\n");
   }
 
+  /**
+   * Representation of a millisecond duration in a short readable text
+   * @param millis  Duration in milliseconds
+   * @returnHuman-Readable Duration Time
+   */
+  public String shortDurationFormat(long millis) {
+long days = TimeUnit.MILLISECONDS.toDays(millis);
+long hours = TimeUnit.MILLISECONDS.toHours(millis) - 
TimeUnit.DAYS.toHours(TimeUnit.MILLISECONDS.toDays(millis));
+long minutes = TimeUnit.MILLISECONDS.toMinutes(millis) - 
TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(millis));
+long seconds = TimeUnit.MILLISECONDS.toSeconds(millis) - 
TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(millis));
+long milliSeconds = millis - 
TimeUnit.SECONDS.toMillis(TimeUnit.MILLISECONDS.toSeconds(millis));
+String formattedDuration = "";
+if (days >= 1) {
+  formattedDuration = days + "d" + hours + "h" + minutes + "m";
+} else if (hours >= 1) {
+  formattedDuration = hours + "h" + minutes + "m";
+} else if (minutes >= 1) {
+  formattedDuration = minutes + "m" + seconds + "s";
+} else {
+  formattedDuration = seconds + "." + milliSeconds + "s";
+}
+return formattedDuration;
--- End diff --

A nit, but the code is clearer if you do:

```
if (cond1) {
return expr1;
} else if (cond2) {
return expr2;
} else {
return expr3;
}
```

The above eliminates the temp variable and makes it very clear what is 
happening.


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix, ready-to-commit
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15850906#comment-15850906
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user sudheeshkatkam commented on the issue:

https://github.com/apache/drill/pull/739
  
+1


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15850828#comment-15850828
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

GitHub user kkhatua opened a pull request:

https://github.com/apache/drill/pull/739

DRILL-5230: Translation of millisecond duration into hours is incorrect

Fixed invalid representation of readable elapsed time using `TimeUnit` 
class in JDK.
e.g. 4545 sec is now correctly translated as `1h15m` instead of `17h15m`
TestCase has been added

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kkhatua/drill DRILL-5230

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/739.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #739


commit bc9ed385d991ac3b6bab5a1ee9a10c649af5be6a
Author: Kunal Khatua 
Date:   2017-01-30T07:08:12Z

DRILL-5230: Translation of millisecond duration into hours is incorrect
Fixed invalid representation of readable elapsed time using `TimeUnit` 
class in JDK.
e.g. 4545 sec is now correctly translated as `1h15m` instead of `17h15m`




> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15850567#comment-15850567
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua closed the pull request at:

https://github.com/apache/drill/pull/732


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5230) Translation of millisecond duration into hours is incorrect

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-5230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15850566#comment-15850566
 ] 

ASF GitHub Bot commented on DRILL-5230:
---

Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/732
  
Pending testcases


> Translation of millisecond duration into hours is incorrect
> ---
>
> Key: DRILL-5230
> URL: https://issues.apache.org/jira/browse/DRILL-5230
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Web Server
>Affects Versions: 1.9.0
>Reporter: Kunal Khatua
>  Labels: easyfix
> Fix For: 1.10.0
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The method 
> {code:JAVA}org.apache.drill.exec.server.rest.profile.TableBuilder.appendMillis(long,
>  String){code}
> has a bug where the human readable translation of a 1+ hr duration in 
> milliseconds is reported incorrectly. 
> This has to do with the {code:JAVA}SimpleDateFormat.format() {code} method 
> incorectly translating it. 
> For e.g.
> {code:JAVA}
> long x = 4545342L; //1 hour 15 min 45.342 sec
> public void appendMillis(x, null);
> {code}
> This formats the value as {noformat}17h15m{noformat} instead of 
> {noformat}1h15m{noformat}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)