DRILL-5863: Sortable table incorrectly sorts fragments/time lexically The DataTables jQuery library sorts data based on the value of the element in a column. However, since Drill publishes sortable items like fragment IDs and time durations as non-numeric text, the sorting is incorrect. This PR fixes the fragment and duration ordering based on their implicit numeric values (minor ID and millisecond representation, respectively). Support memory chaining
closes #987 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/59c74472 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/59c74472 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/59c74472 Branch: refs/heads/master Commit: 59c7447262a22f7f1099f1e0f6d33d44acf8813f Parents: c1118a3 Author: Kunal Khatua <[email protected]> Authored: Wed Oct 11 21:35:47 2017 -0700 Committer: Arina Ielchiieva <[email protected]> Committed: Mon Nov 13 11:05:51 2017 +0200 ---------------------------------------------------------------------- .../server/rest/profile/FragmentWrapper.java | 7 ++- .../server/rest/profile/OperatorWrapper.java | 5 +- .../exec/server/rest/profile/TableBuilder.java | 54 +++++++++++++++----- 3 files changed, 52 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/59c74472/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/FragmentWrapper.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/FragmentWrapper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/FragmentWrapper.java index 2233f2e..5496f83 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/FragmentWrapper.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/FragmentWrapper.java @@ -19,7 +19,9 @@ package org.apache.drill.exec.server.rest.profile; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.apache.drill.exec.proto.UserBitShared.MajorFragmentProfile; import org.apache.drill.exec.proto.UserBitShared.MinorFragmentProfile; @@ -228,6 +230,8 @@ public class FragmentWrapper { Collections2.filter(major.getMinorFragmentProfileList(), Filters.missingOperatorsOrTimes)); Collections.sort(complete, Comparators.minorId); + + Map<String, String> attributeMap = new HashMap<String, String>(); //Reusing for different fragments for (final MinorFragmentProfile minor : complete) { final ArrayList<OperatorProfile> ops = new ArrayList<>(minor.getOperatorProfileList()); @@ -244,7 +248,8 @@ public class FragmentWrapper { biggestBatches = Math.max(biggestBatches, batches); } - builder.appendCell(new OperatorPathBuilder().setMajor(major).setMinor(minor).build()); + attributeMap.put("data-order", String.valueOf(minor.getMinorFragmentId())); //Overwrite values from previous fragments + builder.appendCell(new OperatorPathBuilder().setMajor(major).setMinor(minor).build(), attributeMap); builder.appendCell(minor.getEndpoint().getAddress()); builder.appendMillis(minor.getStartTime() - start); builder.appendMillis(minor.getEndTime() - start); http://git-wip-us.apache.org/repos/asf/drill/blob/59c74472/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java index cca9563..6322435 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -76,12 +77,14 @@ public class OperatorWrapper { public String getContent() { TableBuilder builder = new TableBuilder(OPERATOR_COLUMNS, OPERATOR_COLUMNS_TOOLTIP, true); + Map<String, String> attributeMap = new HashMap<String, String>(); //Reusing for different fragments for (ImmutablePair<ImmutablePair<OperatorProfile, Integer>, String> ip : opsAndHosts) { int minor = ip.getLeft().getRight(); OperatorProfile op = ip.getLeft().getLeft(); + attributeMap.put("data-order", String.valueOf(minor)); //Overwrite values from previous fragments String path = new OperatorPathBuilder().setMajor(major).setMinor(minor).setOperator(op).build(); - builder.appendCell(path); + builder.appendCell(path, attributeMap); builder.appendCell(ip.getRight()); builder.appendNanos(op.getSetupNanos()); builder.appendNanos(op.getProcessNanos()); http://git-wip-us.apache.org/repos/asf/drill/blob/59c74472/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java index b49382b..3833f51 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/TableBuilder.java @@ -21,7 +21,9 @@ import java.text.DateFormat; import java.text.DecimalFormat; import java.text.NumberFormat; import java.text.SimpleDateFormat; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; public class TableBuilder { private final NumberFormat format = NumberFormat.getInstance(Locale.US); @@ -71,16 +73,35 @@ public class TableBuilder { } public void appendCell(final String s, final String link, final String titleText, final String backgroundColor) { + appendCell(s, link, titleText, backgroundColor, null); + } + + public void appendCell(final String s, final Map<String, String> kvPairs) { + appendCell(s, null, null, null, kvPairs); + } + + public void appendCell(final String s, final String link, final String titleText, final String backgroundColor, + final Map<String, String> kvPairs) { if (w == 0) { sb.append("<tr" + (backgroundColor == null ? "" : " style=\"background-color:"+backgroundColor+"\"") + ">"); } + StringBuilder tdElemSB = new StringBuilder("<td"); + //Injecting title if specified (legacy impl) if (titleText != null && titleText.length() > 0) { - sb.append(String.format("<td title=\""+titleText+"\">%s%s</td>", s, link != null ? link : "")); - } else { - sb.append(String.format("<td>%s%s</td>", s, link != null ? link : "")); + tdElemSB.append(" title=\""+titleText+"\""); + } + //Extract other attributes for injection into element + if (kvPairs != null) { + for (String attributeName : kvPairs.keySet()) { + String attributeText = " " + attributeName + "=\"" + kvPairs.get(attributeName) + "\""; + tdElemSB.append(attributeText); + } } + //Closing <td> + tdElemSB.append(String.format(">%s%s</td>", s, link != null ? link : "")); + sb.append(tdElemSB); if (++w >= width) { sb.append("</tr>\n"); w = 0; @@ -98,27 +119,33 @@ public class TableBuilder { } public void appendTime(final long d) { - appendCell(dateFormat.format(d), null, null); + appendTime(d, null); } public void appendTime(final long d, final String link) { - appendCell(dateFormat.format(d), link, null); + appendTime(d, link, null); } public void appendTime(final long d, final String link, final String tooltip) { - appendCell(dateFormat.format(d), link, tooltip); + //Embedding dataTable's data-order attribute + Map<String, String> attributeMap = new HashMap<String, String>(); + attributeMap.put("data-order", String.valueOf(d)); + appendCell(dateFormat.format(d), link, tooltip, null, attributeMap); } public void appendMillis(final long p) { - appendCell((new SimpleDurationFormat(0, p)).compact(), null, null); + appendMillis(p, null); } public void appendMillis(final long p, final String link) { - appendCell((new SimpleDurationFormat(0, p)).compact(), link, null); + appendMillis(p, link, null); } public void appendMillis(final long p, final String link, final String tooltip) { - appendCell((new SimpleDurationFormat(0, p)).compact(), link, tooltip); + //Embedding dataTable's data-order attribute + Map<String, String> attributeMap = new HashMap<String, String>(); + attributeMap.put("data-order", String.valueOf(p)); + appendCell((new SimpleDurationFormat(0, p)).compact(), link, tooltip, null, attributeMap); } public void appendNanos(final long p) { @@ -174,15 +201,18 @@ public class TableBuilder { } public void appendBytes(final long l) { - appendCell(bytePrint(l), null, null); + appendBytes(l, null); } public void appendBytes(final long l, final String link) { - appendCell(bytePrint(l), link, null); + appendBytes(l, link, null); } public void appendBytes(final long l, final String link, final String tooltip) { - appendCell(bytePrint(l), link, tooltip); + //Embedding dataTable's data-order attribute + Map<String, String> attributeMap = new HashMap<String, String>(); + attributeMap.put("data-order", String.valueOf(l)); + appendCell(bytePrint(l), link, tooltip, null, attributeMap); } private String bytePrint(final long size) {
