You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ar...@apache.org on 2017/11/13 12:07:19 UTC

[05/11] drill git commit: DRILL-5863: Sortable table incorrectly sorts fragments/time lexically

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 <kk...@maprtech.com>
Authored: Wed Oct 11 21:35:47 2017 -0700
Committer: Arina Ielchiieva <ar...@gmail.com>
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) {