[GitHub] [hive] pgaref commented on a change in pull request #2812: HIVE-25666: Realtime memory usage in beeline progress

[GitBox <gi...@apache.org>]

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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org