[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/1197 ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179557189 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -190,8 +223,59 @@ public void addSummary(TableBuilder tb, HashMap majorFragmentBusyT tb.appendFormattedInteger(recordSum); final ImmutablePair peakMem = Collections.max(opList, Comparators.operatorPeakMemory); -tb.appendBytes(Math.round(memSum / size)); -tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated()); + +//Inject spill-to-disk attributes +Map avgSpillMap = null; +Map maxSpillMap = null; +if (hasSpilledToDisk) { + avgSpillMap = new HashMap<>(); + //Average SpillCycle + float avgSpillCycle = (float) spillCycleSum/size; --- End diff -- My bad. I was thinking the sum was a long. I'll fix this. ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179552510 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -38,7 +39,16 @@ * Wrapper class for profiles of ALL operator instances of the same operator type within a major fragment. */ public class OperatorWrapper { + //private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(OperatorWrapper.class); --- End diff -- Ok. I see a lot of other places where it is commented out. I'll uncomment it with an unused tag for the compler to ignore. Comes very handy in debugging when making changes ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179552379 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -38,7 +39,16 @@ * Wrapper class for profiles of ALL operator instances of the same operator type within a major fragment. */ public class OperatorWrapper { + //private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(OperatorWrapper.class); + + private static final String HTML_ATTRIB_SPILLS = "spills"; + private static final String HTML_ATTRIB_CLASS = "class"; + private static final String HTML_ATTRIB_STYLE = "style"; + private static final String HTML_ATTRIB_TITLE = "title"; + private static final DecimalFormat DECIMAL_FORMATTER = new DecimalFormat("#.##"); private static final String UNKNOWN_OPERATOR = "UNKNOWN_OPERATOR"; + //-ve value to indicate absence of metric --- End diff -- Using a negative ("-ve") value constant. Will fix the comment ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179552086 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -190,8 +223,59 @@ public void addSummary(TableBuilder tb, HashMap majorFragmentBusyT tb.appendFormattedInteger(recordSum); final ImmutablePair peakMem = Collections.max(opList, Comparators.operatorPeakMemory); -tb.appendBytes(Math.round(memSum / size)); -tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated()); + +//Inject spill-to-disk attributes +Map avgSpillMap = null; +Map maxSpillMap = null; +if (hasSpilledToDisk) { + avgSpillMap = new HashMap<>(); + //Average SpillCycle + float avgSpillCycle = (float) spillCycleSum/size; --- End diff -- Needed the value to be in decimal (formatter will limit it to 2 decimal spaces). ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179551936 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -190,8 +223,59 @@ public void addSummary(TableBuilder tb, HashMap majorFragmentBusyT tb.appendFormattedInteger(recordSum); final ImmutablePair peakMem = Collections.max(opList, Comparators.operatorPeakMemory); -tb.appendBytes(Math.round(memSum / size)); -tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated()); + +//Inject spill-to-disk attributes +Map avgSpillMap = null; +Map maxSpillMap = null; +if (hasSpilledToDisk) { + avgSpillMap = new HashMap<>(); + //Average SpillCycle + float avgSpillCycle = (float) spillCycleSum/size; + avgSpillMap.put(HTML_ATTRIB_TITLE, DECIMAL_FORMATTER.format(avgSpillCycle) + " Spills on average"); --- End diff -- Agreed. Will fix it. ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179551687 --- Diff: exec/java-exec/src/main/resources/rest/profile/profile.ftl --- @@ -372,6 +372,16 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179406094 --- Diff: exec/java-exec/src/main/resources/rest/profile/profile.ftl --- @@ -372,6 +372,16 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179403081 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -38,7 +39,16 @@ * Wrapper class for profiles of ALL operator instances of the same operator type within a major fragment. */ public class OperatorWrapper { + //private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(OperatorWrapper.class); + + private static final String HTML_ATTRIB_SPILLS = "spills"; + private static final String HTML_ATTRIB_CLASS = "class"; + private static final String HTML_ATTRIB_STYLE = "style"; + private static final String HTML_ATTRIB_TITLE = "title"; + private static final DecimalFormat DECIMAL_FORMATTER = new DecimalFormat("#.##"); private static final String UNKNOWN_OPERATOR = "UNKNOWN_OPERATOR"; + //-ve value to indicate absence of metric --- End diff -- `-ve value`? ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179405507 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -190,8 +223,59 @@ public void addSummary(TableBuilder tb, HashMap majorFragmentBusyT tb.appendFormattedInteger(recordSum); final ImmutablePair peakMem = Collections.max(opList, Comparators.operatorPeakMemory); -tb.appendBytes(Math.round(memSum / size)); -tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated()); + +//Inject spill-to-disk attributes +Map avgSpillMap = null; +Map maxSpillMap = null; +if (hasSpilledToDisk) { + avgSpillMap = new HashMap<>(); + //Average SpillCycle + float avgSpillCycle = (float) spillCycleSum/size; --- End diff -- Why float? You are dividing double by int. ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179405848 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -190,8 +223,59 @@ public void addSummary(TableBuilder tb, HashMap majorFragmentBusyT tb.appendFormattedInteger(recordSum); final ImmutablePair peakMem = Collections.max(opList, Comparators.operatorPeakMemory); -tb.appendBytes(Math.round(memSum / size)); -tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated()); + +//Inject spill-to-disk attributes +Map avgSpillMap = null; +Map maxSpillMap = null; +if (hasSpilledToDisk) { + avgSpillMap = new HashMap<>(); + //Average SpillCycle + float avgSpillCycle = (float) spillCycleSum/size; + avgSpillMap.put(HTML_ATTRIB_TITLE, DECIMAL_FORMATTER.format(avgSpillCycle) + " Spills on average"); --- End diff -- I think `s` can be in lower case: `spills on average`. ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179402877 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -38,7 +39,16 @@ * Wrapper class for profiles of ALL operator instances of the same operator type within a major fragment. */ public class OperatorWrapper { + //private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(OperatorWrapper.class); --- End diff -- If logger is not needed, remote it. ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/1197#discussion_r179403811 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -190,8 +223,59 @@ public void addSummary(TableBuilder tb, HashMap majorFragmentBusyT tb.appendFormattedInteger(recordSum); final ImmutablePair peakMem = Collections.max(opList, Comparators.operatorPeakMemory); -tb.appendBytes(Math.round(memSum / size)); -tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated()); + +//Inject spill-to-disk attributes +Map avgSpillMap = null; +Map maxSpillMap = null; +if (hasSpilledToDisk) { + avgSpillMap = new HashMap<>(); + //Average SpillCycle + float avgSpillCycle = (float) spillCycleSum/size; + avgSpillMap.put(HTML_ATTRIB_TITLE, DECIMAL_FORMATTER.format(avgSpillCycle) + " Spills on average"); + avgSpillMap.put(HTML_ATTRIB_STYLE, "cursor:help;" + spillCycleMax); + avgSpillMap.put(HTML_ATTRIB_CLASS, "spill-tag"); //JScript will inject Icon + avgSpillMap.put(HTML_ATTRIB_SPILLS, DECIMAL_FORMATTER.format(avgSpillCycle)); //JScript will inject Count + maxSpillMap = new HashMap<>(); + maxSpillMap.put(HTML_ATTRIB_TITLE, "Most # Spills: " + spillCycleMax); + maxSpillMap.put(HTML_ATTRIB_STYLE, "cursor:help;" + spillCycleMax); + maxSpillMap.put(HTML_ATTRIB_CLASS, "spill-tag"); //JScript will inject Icon + maxSpillMap.put(HTML_ATTRIB_SPILLS, String.valueOf(spillCycleMax)); //JScript will inject Count +} + +tb.appendBytes(Math.round(memSum / size), avgSpillMap); +tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated(), maxSpillMap); + } + + /** + * Returns index of Spill Count/Cycle metric + * @param operatorType + * @return index of spill metric + */ + private int getSpillCycleMetricIndex(CoreOperatorType operatorType) { +String metricName = null; --- End diff -- Initializing with null is redundant. ---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
GitHub user kkhatua opened a pull request: https://github.com/apache/drill/pull/1197 DRILL-6279: UI indicates operators that spilled in-memory data to disk 1. Detect the presence of an operator that has spilled to disk, in the Operators Overview section of a query's profile page. 2. Introduced API to inject html attributes into a cell. This is used to inject details about the spill in the avg peak and max peak memory usage 3. Javascript is leveraged to detect spilled operators' HTML elements, and inject a symbolic font for indicating the operator as having spilt. 4. Mouse-over effect indicates the average and max number of spill cycles the operator went through. NOTE: Bootstrap library comes with Glyphicons Halfling Fonts and meets the Apache license requirements. http://glyphicons.com/license#halflingsbootstrap You can merge this pull request into a Git repository by running: $ git pull https://github.com/kkhatua/drill DRILL-6279 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1197.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 #1197 commit 230d739c05e2f2da93232fe31dec55adf627937e Author: Kunal Khatua Date: 2018-03-30T04:58:53Z DRILL-6279: UI indicates operators that spilled in-memory data to disk 1. Detect the presence of an operator that has spilled to disk, in the Operators Overview section of a query's profile page. 2. Introduced API to inject html attributes into a cell. This is used to inject details about the spill in the avg peak and max peak memory usage 3. Javascript is leveraged to detect spilled operators' HTML elements, and inject a symbolic font for indicating the operator as having spilt. 4. Mouse-over effect indicates the average and max number of spill cycles the operator went through. commit 5641e713888db6e3e028a5749bd902dc3bdc6562 Author: Kunal Khatua Date: 2018-03-26T05:05:06Z Addition of Bootstrap's Glyphicons As part of the Bootstrap's components, this meets Apache License criteria ---