pgaref commented on a change in pull request #2812:
URL: https://github.com/apache/hive/pull/2812#discussion_r756555296
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
##########
@@ -1163,4 +1165,15 @@ private QueryIdentifierProto
constructQueryIdentifierProto(int dagIdentifier) {
public String getAmHostString() {
return amHost;
}
+
+ /**
+ * Overrides TezTaskCommunicatorImpl.getTotalUsedMemory in order to provide
correct aggregated memory usage.
+ * In LLAP, every container reports the whole used heap of the daemon
they're running in, so we need to consider
+ * every usedMemory once per daemon.
+ * @return
+ */
+ @Override
+ public long getTotalUsedMemory() {
+ return pingedNodeMap.values().stream().mapToLong(c -> c.usedMemory).sum();
Review comment:
i.e., could print something like: Avg usage percent: 50%. 80th
percentile: 80%, 90th percentile: 99%
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
##########
@@ -1163,4 +1165,15 @@ private QueryIdentifierProto
constructQueryIdentifierProto(int dagIdentifier) {
public String getAmHostString() {
return amHost;
}
+
+ /**
+ * Overrides TezTaskCommunicatorImpl.getTotalUsedMemory in order to provide
correct aggregated memory usage.
+ * In LLAP, every container reports the whole used heap of the daemon
they're running in, so we need to consider
+ * every usedMemory once per daemon.
+ * @return
+ */
+ @Override
+ public long getTotalUsedMemory() {
+ return pingedNodeMap.values().stream().mapToLong(c -> c.usedMemory).sum();
Review comment:
Sum does not provide much value here. Maybe we could print it as an
average percentage across daemons, but having some kind of percentile here
could actually reveal if some daemons face more pressure than others
##########
File path:
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java
##########
@@ -431,6 +437,15 @@ protected Void callInternal() {
return null;
}
+
+ private long getUsedMemory() {
Review comment:
Cool stuff, would it make more sense to present it as used percentage?
We could even print both values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
