You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by kkhatua <gi...@git.apache.org> on 2018/03/30 06:12:46 UTC
[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 <kk...@...>
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 <kk...@...>
Date: 2018-03-26T05:05:06Z
Addition of Bootstrap's Glyphicons
As part of the Bootstrap's components, this meets Apache License criteria
----
---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Posted by arina-ielchiieva <gi...@git.apache.org>.
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
</div> <br>
<script>
+ //Inject Spilled Tags
+ $(window).on('load', function () {
+ var spillLabel = document.getElementsByClassName("spill-tag");
+ var i;
+ for (i = 0; i < spillLabel.length; i++) {
+ var content = spillLabel[i].innerHTML;
+ spillLabel[i].innerHTML = "<span class=\"glyphicon glyphicon-download-alt\"> </span>"+content;
+ }
+ });
+
--- End diff --
Why do you need exec/java-exec/src/main/resources/rest/static/img/popout.png?
---
[GitHub] drill issue #1197: DRILL-6279: UI indicates operators that spilled in-memory...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/1197
+1
---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Posted by arina-ielchiieva <gi...@git.apache.org>.
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<String, Long> majorFragmentBusyT
tb.appendFormattedInteger(recordSum);
final ImmutablePair<OperatorProfile, Integer> peakMem = Collections.max(opList, Comparators.operatorPeakMemory);
- tb.appendBytes(Math.round(memSum / size));
- tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated());
+
+ //Inject spill-to-disk attributes
+ Map<String, String> avgSpillMap = null;
+ Map<String, String> 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...
Posted by kkhatua <gi...@git.apache.org>.
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
</div> <br>
<script>
+ //Inject Spilled Tags
+ $(window).on('load', function () {
+ var spillLabel = document.getElementsByClassName("spill-tag");
+ var i;
+ for (i = 0; i < spillLabel.length; i++) {
+ var content = spillLabel[i].innerHTML;
+ spillLabel[i].innerHTML = "<span class=\"glyphicon glyphicon-download-alt\"> </span>"+content;
+ }
+ });
+
--- End diff --
It was actually meant as a part of DRILL-6289. Will remove it.
---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Posted by kkhatua <gi...@git.apache.org>.
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<String, Long> majorFragmentBusyT
tb.appendFormattedInteger(recordSum);
final ImmutablePair<OperatorProfile, Integer> peakMem = Collections.max(opList, Comparators.operatorPeakMemory);
- tb.appendBytes(Math.round(memSum / size));
- tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated());
+
+ //Inject spill-to-disk attributes
+ Map<String, String> avgSpillMap = null;
+ Map<String, String> 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...
Posted by kkhatua <gi...@git.apache.org>.
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...
Posted by arina-ielchiieva <gi...@git.apache.org>.
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 issue #1197: DRILL-6279: UI indicates operators that spilled in-memory...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/1197
@arina-ielchiieva done the requested changes.
---
[GitHub] drill pull request #1197: DRILL-6279: UI indicates operators that spilled in...
Posted by kkhatua <gi...@git.apache.org>.
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<String, Long> majorFragmentBusyT
tb.appendFormattedInteger(recordSum);
final ImmutablePair<OperatorProfile, Integer> peakMem = Collections.max(opList, Comparators.operatorPeakMemory);
- tb.appendBytes(Math.round(memSum / size));
- tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated());
+
+ //Inject spill-to-disk attributes
+ Map<String, String> avgSpillMap = null;
+ Map<String, String> 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...
Posted by arina-ielchiieva <gi...@git.apache.org>.
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<String, Long> majorFragmentBusyT
tb.appendFormattedInteger(recordSum);
final ImmutablePair<OperatorProfile, Integer> peakMem = Collections.max(opList, Comparators.operatorPeakMemory);
- tb.appendBytes(Math.round(memSum / size));
- tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated());
+
+ //Inject spill-to-disk attributes
+ Map<String, String> avgSpillMap = null;
+ Map<String, String> 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...
Posted by kkhatua <gi...@git.apache.org>.
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<String, Long> majorFragmentBusyT
tb.appendFormattedInteger(recordSum);
final ImmutablePair<OperatorProfile, Integer> peakMem = Collections.max(opList, Comparators.operatorPeakMemory);
- tb.appendBytes(Math.round(memSum / size));
- tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated());
+
+ //Inject spill-to-disk attributes
+ Map<String, String> avgSpillMap = null;
+ Map<String, String> 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...
Posted by arina-ielchiieva <gi...@git.apache.org>.
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<String, Long> majorFragmentBusyT
tb.appendFormattedInteger(recordSum);
final ImmutablePair<OperatorProfile, Integer> peakMem = Collections.max(opList, Comparators.operatorPeakMemory);
- tb.appendBytes(Math.round(memSum / size));
- tb.appendBytes(peakMem.getLeft().getPeakLocalMemoryAllocated());
+
+ //Inject spill-to-disk attributes
+ Map<String, String> avgSpillMap = null;
+ Map<String, String> 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...
Posted by arina-ielchiieva <gi...@git.apache.org>.
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...
Posted by asfgit <gi...@git.apache.org>.
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...
Posted by kkhatua <gi...@git.apache.org>.
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 issue #1197: DRILL-6279: UI indicates operators that spilled in-memory...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/1197
@arina-ielchiieva could you review this? I will be using this commit as the basis for [DRILL-6289](https://issues.apache.org/jira/browse/DRILL-6289)
---