Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kkhatua wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/FragmentWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ public String getId() {
public static final int NUM_NULLABLE_OVERVIEW_COLUMNS = FRAGMENT_OVERVIEW_COLUMNS.length - 2;

public void addSummary(TableBuilder tb) {
final String fmt = " (%d)";

// Use only minor fragments that have complete profiles
// Complete iff the fragment profile has at least one operator profile, and start and end times.
final List<MinorFragmentProfile> complete = new ArrayList<>(
Expand All @@ -75,13 +73,13 @@ public void addSummary(TableBuilder tb) {

final MinorFragmentProfile firstStart = Collections.min(complete, Comparators.startTime);
final MinorFragmentProfile lastStart = Collections.max(complete, Comparators.startTime);
tb.appendMillis(firstStart.getStartTime() - start, String.format(fmt, firstStart.getMinorFragmentId()));
tb.appendMillis(lastStart.getStartTime() - start, String.format(fmt, lastStart.getMinorFragmentId()));
tb.appendMillis(firstStart.getStartTime() - start, null);
tb.appendMillis(lastStart.getStartTime() - start, null);

final MinorFragmentProfile firstEnd = Collections.min(complete, Comparators.endTime);
final MinorFragmentProfile lastEnd = Collections.max(complete, Comparators.endTime);
tb.appendMillis(firstEnd.getEndTime() - start, String.format(fmt, firstEnd.getMinorFragmentId()));
tb.appendMillis(lastEnd.getEndTime() - start, String.format(fmt, lastEnd.getMinorFragmentId()));
tb.appendMillis(firstEnd.getEndTime() - start, null);
tb.appendMillis(lastEnd.getEndTime() - start, null);

long total = 0;
for (final MinorFragmentProfile p : complete) {
Expand All @@ -90,9 +88,9 @@ public void addSummary(TableBuilder tb) {

final MinorFragmentProfile shortRun = Collections.min(complete, Comparators.runTime);
final MinorFragmentProfile longRun = Collections.max(complete, Comparators.runTime);
tb.appendMillis(shortRun.getEndTime() - shortRun.getStartTime(), String.format(fmt, shortRun.getMinorFragmentId()));
tb.appendMillis(shortRun.getEndTime() - shortRun.getStartTime(), null);
tb.appendMillis(total / complete.size(), null);
tb.appendMillis(longRun.getEndTime() - longRun.getStartTime(), String.format(fmt, longRun.getMinorFragmentId()));
tb.appendMillis(longRun.getEndTime() - longRun.getStartTime(), null);

final MinorFragmentProfile lastUpdate = Collections.max(complete, Comparators.lastUpdate);
tb.appendTime(lastUpdate.getLastUpdate(), null);
Expand Down
14 changes: 6 additions & 8 deletions exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
* Wrapper class for profiles of ALL operator instances of the same operator type within a major fragment.
*/
public class OperatorWrapper {
private static final String format = " (%s)";

private final int major;
private final List<ImmutablePair<OperatorProfile, Integer>> ops; // operator profile --> minor fragment number
private final OperatorProfile firstProfile;
Expand Down Expand Up @@ -114,21 +112,21 @@ public void addSummary(TableBuilder tb) {

final ImmutablePair<OperatorProfile, Integer> shortSetup = Collections.min(ops, Comparators.setupTime);
final ImmutablePair<OperatorProfile, Integer> longSetup = Collections.max(ops, Comparators.setupTime);
tb.appendNanos(shortSetup.getLeft().getSetupNanos(), String.format(format, shortSetup.getRight()));
tb.appendNanos(shortSetup.getLeft().getSetupNanos(), null);
tb.appendNanos(Math.round(setupSum / size), null);
tb.appendNanos(longSetup.getLeft().getSetupNanos(), String.format(format, longSetup.getRight()));
tb.appendNanos(longSetup.getLeft().getSetupNanos(), null);

final ImmutablePair<OperatorProfile, Integer> shortProcess = Collections.min(ops, Comparators.processTime);
final ImmutablePair<OperatorProfile, Integer> longProcess = Collections.max(ops, Comparators.processTime);
tb.appendNanos(shortProcess.getLeft().getProcessNanos(), String.format(format, shortProcess.getRight()));
tb.appendNanos(shortProcess.getLeft().getProcessNanos(), null);
tb.appendNanos(Math.round(processSum / size), null);
tb.appendNanos(longProcess.getLeft().getProcessNanos(), String.format(format, longProcess.getRight()));
tb.appendNanos(longProcess.getLeft().getProcessNanos(), null);

final ImmutablePair<OperatorProfile, Integer> shortWait = Collections.min(ops, Comparators.waitTime);
final ImmutablePair<OperatorProfile, Integer> longWait = Collections.max(ops, Comparators.waitTime);
tb.appendNanos(shortWait.getLeft().getWaitNanos(), String.format(format, shortWait.getRight()));
tb.appendNanos(shortWait.getLeft().getWaitNanos(), null);
tb.appendNanos(Math.round(waitSum / size), null);
tb.appendNanos(longWait.getLeft().getWaitNanos(), String.format(format, longWait.getRight()));
tb.appendNanos(longWait.getLeft().getWaitNanos(), null);

final ImmutablePair<OperatorProfile, Integer> peakMem = Collections.max(ops, Comparators.operatorPeakMemory);
tb.appendBytes(Math.round(memSum / size), null);
Expand Down
38 changes: 10 additions & 28 deletions exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,6 @@ public class ProfileResources {
@Inject DrillUserPrincipal principal;
@Inject SecurityContext sc;

/**
* Returns elapsed time a human-readable format. If end time is less than the start time, current epoch time is assumed as the end time.
* e.g. getPrettyDuration(1468368841695,1468394096016) = '7 hr 00 min 54.321 sec'
* @param startTimeMillis Start Time in milliseconds
* @param endTimeMillis End Time in milliseconds
* @return Human-Readable Elapsed Time
*/
public static String getPrettyDuration(long startTimeMillis, long endTimeMillis) {
long durationInMillis = (startTimeMillis > endTimeMillis ? System.currentTimeMillis() : endTimeMillis) - startTimeMillis;
long hours = TimeUnit.MILLISECONDS.toHours(durationInMillis);
long minutes = TimeUnit.MILLISECONDS.toMinutes(durationInMillis) - TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(durationInMillis));
long seconds = TimeUnit.MILLISECONDS.toSeconds(durationInMillis) - TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(durationInMillis));
long milliSeconds = durationInMillis - TimeUnit.SECONDS.toMillis(TimeUnit.MILLISECONDS.toSeconds(durationInMillis));
String formattedDuration = (hours > 0 ? hours + " hr " : "") +
((minutes + hours) > 0 ? String.format("%02d min ", minutes) : "") +
seconds + "." + String.format("%03d sec", milliSeconds) ;
return formattedDuration;
}

public static class ProfileInfo implements Comparable<ProfileInfo> {
public static final SimpleDateFormat format = new SimpleDateFormat("MM/dd/yyyy HH:mm:ss");

Expand Down Expand Up @@ -137,7 +118,7 @@ public long getEndTime() {
}

public String getDuration() {
return getPrettyDuration(startTime, endTime);
return (new SimpleDurationFormat(startTime, endTime)).verbose();
}

public String getState() {
Expand Down Expand Up @@ -237,8 +218,8 @@ public QProfiles getProfilesJSON() {
return new QProfiles(runningQueries, finishedQueries, errors);
} catch (Exception e) {
throw UserException.resourceError(e)
.message("Failed to get profiles from persistent or ephemeral store.")
.build(logger);
.message("Failed to get profiles from persistent or ephemeral store.")
.build(logger);
}
}

Expand Down Expand Up @@ -291,8 +272,8 @@ private QueryProfile getQueryProfile(String queryId) {
}

throw UserException.validationError()
.message("No profile with given query id '%s' exists. Please verify the query id.", queryId)
.build(logger);
.message("No profile with given query id '%s' exists. Please verify the query id.", queryId)
.build(logger);
}


Expand Down Expand Up @@ -352,16 +333,17 @@ 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);
}
}
}

2 changes: 1 addition & 1 deletion exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public QueryProfile getProfile() {
}

public String getProfileDuration() {
return ProfileResources.getPrettyDuration(profile.getStart(), profile.getEnd());
return (new SimpleDurationFormat(profile.getStart(), profile.getEnd())).verbose();
}

public String getQueryId() {
Expand Down
77 changes: 77 additions & 0 deletions ...va-exec/src/main/java/org/apache/drill/exec/server/rest/profile/SimpleDurationFormat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.drill.exec.server.rest.profile;

import java.util.concurrent.TimeUnit;

/**
* Representation of a millisecond duration in a human-readable format
*/
public class SimpleDurationFormat {
private long days;
private long hours;
private long minutes;
private long seconds;
private long milliSeconds;
private long durationInMillis;

//Block creation of any default objects
@SuppressWarnings("unused")
private SimpleDurationFormat() {}

/**
* If end time is less than the start time, current epoch time is assumed as the end time.
* @param startTimeMillis
* @param endTimeMillis
*/
public SimpleDurationFormat(long startTimeMillis, long endTimeMillis) {
durationInMillis = (startTimeMillis > endTimeMillis ? System.currentTimeMillis() : endTimeMillis) - startTimeMillis;
days = TimeUnit.MILLISECONDS.toDays(durationInMillis);
hours = TimeUnit.MILLISECONDS.toHours(durationInMillis) - TimeUnit.DAYS.toHours(TimeUnit.MILLISECONDS.toDays(durationInMillis));
minutes = TimeUnit.MILLISECONDS.toMinutes(durationInMillis) - TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(durationInMillis));
seconds = TimeUnit.MILLISECONDS.toSeconds(durationInMillis) - TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(durationInMillis));
milliSeconds = durationInMillis - TimeUnit.SECONDS.toMillis(TimeUnit.MILLISECONDS.toSeconds(durationInMillis));
}

/**
* Return a compact representation of elapsed time with only the most significant time units and no spaces
* @return duration
*/
public String compact() {
if (days >= 1) {
return days + "d" + hours + "h" + minutes + "m";
} else if (hours >= 1) {
return hours + "h" + minutes + "m";
} else if (minutes >= 1) {
return minutes + "m" + seconds + "s";
} else {
return String.format("%.3fs", seconds + milliSeconds/1000.0);
}
}

/**
* Return a verbose representation of elapsed time down to millisecond granularity
* @return duration
*/
public String verbose() {
return (days > 0 ? days + " day " : "") +
((hours + days) > 0 ? hours + " hr " : "") +
((minutes + hours + days) > 0 ? String.format("%02d min ", minutes) : "") +
seconds + "." + String.format("%03d sec", milliSeconds) ;
}
}
36 changes: 2 additions & 34 deletions exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,10 @@
import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Locale;

class TableBuilder {
public class TableBuilder {
private final NumberFormat format = NumberFormat.getInstance(Locale.US);
private final SimpleDateFormat days = new SimpleDateFormat("DD'd'hh'h'mm'm'");
private final SimpleDateFormat sdays = new SimpleDateFormat("DD'd'hh'h'mm'm'");
private final SimpleDateFormat hours = new SimpleDateFormat("HH'h'mm'm'");
private final SimpleDateFormat shours = new SimpleDateFormat("H'h'mm'm'");
private final SimpleDateFormat mins = new SimpleDateFormat("mm'm'ss's'");
private final SimpleDateFormat smins = new SimpleDateFormat("m'm'ss's'");

private final SimpleDateFormat secs = new SimpleDateFormat("ss.SSS's'");
private final SimpleDateFormat ssecs = new SimpleDateFormat("s.SSS's'");
private final DateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
private final DecimalFormat dec = new DecimalFormat("0.00");
private final DecimalFormat intformat = new DecimalFormat("#,###");
Expand Down Expand Up @@ -78,29 +68,7 @@ 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);
appendCell((new SimpleDurationFormat(0, p)).compact(), link);
}

public void appendNanos(final long p, final String link) {
Expand Down
Loading