You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/03/12 01:04:37 UTC

[GitHub] [hive] Dawn2111 opened a new pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Dawn2111 opened a new pull request #2065:
URL: https://github.com/apache/hive/pull/2065


   ### What changes were proposed in this pull request?
    Currently, the Workload management move trigger kills the query being moved to a different pool if destination pool does not have enough capacity. This PR introduces a "delayed move" configuration which lets the query run in the source pool as long as possible, if the destination pool is full. It will attempt the move to destination pool only when there is claim upon the source pool. If the destination pool is not full, delayed move behaves as normal move i.e. the move will happen immediately.
   
   ### Why are the changes needed?
   For better utilization of cluster resources. 
   
   ### Does this PR introduce _any_ user-facing change? Yes
   
   ### How was this patch tested? Unit test
   


----------------------------------------------------------------
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.

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


[GitHub] [hive] guptanikhil007 commented on a change in pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
guptanikhil007 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r594076379



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -657,6 +673,30 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     }
     e.toReuse.clear();
 
+    // For pools which have queued requests, move the "delayed moves" now

Review comment:
       Comment meaning is not clear

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -794,17 +834,18 @@ private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
     final WmThreadSyncWork syncWork,
     final HashSet<String> poolsToRedistribute,
     final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+    final Map<WmTezSession, WmEvent> recordMoveEvents,
+      final boolean moveImmediately) {

Review comment:
       nit: indentation fix

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -816,18 +857,36 @@ private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
             LOG.error("Failed to move session: {}. Session is not added to destination.", moveSession);
           }
         } else {
-          WmTezSession session = moveSession.srcSession;
-          KillQueryContext killQueryContext = new KillQueryContext(session, "Destination pool " + destPoolName +
-            " is full. Killing query.");
-          resetAndQueueKill(syncWork.toKillQuery, killQueryContext, toReuse);
+        LOG.error("Failed to move session: {}. Session is not removed from its pool.", moveSession);
         }
       } else {
-        LOG.error("Failed to move session: {}. Session is not removed from its pool.", moveSession);
+        // If delayed move is set to true, don't kill the query. Let the query run in source pool
+        // add the session to the source pool's delayed move sessions so that the session can be moved
+        // later when there is a new request on the source pool.
+        if(HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE) && !moveImmediately) {
+          LOG.info("Move: {} is a delayed move.Since destination pool {} is full, running in source pool "
+              + "as long as possible.", moveSession, destPoolName);
+          String srcPoolName = moveSession.srcSession.getPoolName();
+          if (srcPoolName != null) {
+            PoolState srcPool = pools.get(srcPoolName);
+            if (srcPool != null) {
+              if (srcPool.delayedMoveSessionsSet.add(moveSession)) {
+                srcPool.delayedMoveSessionsQueue.addLast(moveSession);

Review comment:
       We can try using TreeSet instead of a queue and a HashSet

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -657,6 +673,30 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     }
     e.toReuse.clear();
 
+    // For pools which have queued requests, move the "delayed moves" now

Review comment:
       Put the code behind the config




----------------------------------------------------------------
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.

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


[GitHub] [hive] Dawn2111 commented on a change in pull request #2065: HIVE-2420: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r621834393



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java
##########
@@ -47,8 +47,10 @@ public void applyAction(final Map<WmTezSession, Trigger> queriesViolated) {
           break;
         case MOVE_TO_POOL:
           String destPoolName = entry.getValue().getAction().getPoolName();
-          Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);
-          moveFutures.put(wmTezSession, moveFuture);
+          if (!wmTezSession.isDelayedMove()) {
+            Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);

Review comment:
       Dont think we need to - any query being completed/killed in the destination pool will create a return/kill event. This  in turn will wake up the master thread which will retry the delayed move in the same iteration of the master thread loop. So the existing delayed moves will be processed earlier than any subsequent move events.




-- 
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.

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


[GitHub] [hive] Dawn2111 commented on pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on pull request #2065:
URL: https://github.com/apache/hive/pull/2065#issuecomment-800405747


   @guptanikhil007, could you please review ?


----------------------------------------------------------------
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.

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


[GitHub] [hive] sankarh commented on a change in pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
sankarh commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r597613584



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -363,6 +352,21 @@ public MoveSession(final WmTezSession srcSession, final String destPool) {
     public String toString() {
       return srcSession.getSessionId() + " moving from " + srcSession.getPoolName() + " to " + destPool;
     }
+
+    @Override
+    public boolean equals(Object o) {
+      if (o==this) return true;

Review comment:
       nit: 
   1. Need space before and after binary operators. 
   2. Even single statement block should be put under {}.
   3. Space after keywords such as "if"

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -657,7 +661,37 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     }
     e.toReuse.clear();
 
-    // 9. Resolve all the kill query requests in flight. Nothing below can affect them.
+    // 9. If delayed move is set to true, for pools which have queued requests, process the "delayed moves" now.
+    // Incoming requests have more claim upon the pool than the delayed moves
+    if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE)) {
+      for (String poolName : poolsToRedistribute) {
+        PoolState pool = pools.get(poolName);
+        if (pool == null)
+          return;
+        int queueSize = pool.queue.size();
+        int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();
+        int delayedMovesToProcess = queueSize > remainingCapacity ? queueSize - remainingCapacity : 0;

Review comment:
       nit: Use () to clearly mark the boundary. 
   int delayedMovesToProcess = (queueSize > remainingCapacity) ? (queueSize - remainingCapacity) : 0;
   
   if ((delayedMovesToProcess > 0) && (pool.delayedMoveSessions.size() > 0)) {

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -794,17 +828,17 @@ private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
     final WmThreadSyncWork syncWork,
     final HashSet<String> poolsToRedistribute,
     final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+    final Map<WmTezSession, WmEvent> recordMoveEvents, final boolean moveImmediately) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, move immediately: {}", moveSession, moveImmediately);
     if (validMove(moveSession.srcSession, destPoolName)) {
       WmEvent moveEvent = new WmEvent(WmEvent.EventType.MOVE);
-      // remove from src pool
-      RemoveSessionResult rr = checkAndRemoveSessionFromItsPool(
+      // check if there is capacity in dest pool
+      if (capacityAvailable(destPoolName)) {

Review comment:
       We shouldn't change the sequence of validation here. Modify the flow as follows.
   ```
   RemoveSessionResult rr = checkAndRemoveSessionFromItsPool(
   if (rr == RemoveSessionResult.OK) {
     if (capacityAvailable(destPoolName)) {
       // Existing code to move the session
     } else {
       if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE) && !moveImmediately) {
         // new code to move it to delayed sessions list
       } else {
         // Existing code to kill the query
       }
     }
   } else {
     // Existing code to log error msg.
   }
   ```

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -32,18 +32,7 @@
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.IdentityHashMap;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;

Review comment:
       Expand the imports instead of using *.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -657,7 +661,37 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     }
     e.toReuse.clear();
 
-    // 9. Resolve all the kill query requests in flight. Nothing below can affect them.
+    // 9. If delayed move is set to true, for pools which have queued requests, process the "delayed moves" now.
+    // Incoming requests have more claim upon the pool than the delayed moves
+    if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE)) {
+      for (String poolName : poolsToRedistribute) {
+        PoolState pool = pools.get(poolName);
+        if (pool == null)

Review comment:
       nit: Need {}

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -641,7 +645,7 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     // as possible
     Map<WmTezSession, WmEvent> recordMoveEvents = new HashMap<>();
     for (MoveSession moveSession : e.moveSessions) {
-      handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse, recordMoveEvents);
+      handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse, recordMoveEvents, false);

Review comment:
       The last argument can be set based on new config instead of passing false here and then take true flow if the config =false.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -657,7 +661,37 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     }
     e.toReuse.clear();
 
-    // 9. Resolve all the kill query requests in flight. Nothing below can affect them.
+    // 9. If delayed move is set to true, for pools which have queued requests, process the "delayed moves" now.
+    // Incoming requests have more claim upon the pool than the delayed moves
+    if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE)) {
+      for (String poolName : poolsToRedistribute) {
+        PoolState pool = pools.get(poolName);
+        if (pool == null)
+          return;
+        int queueSize = pool.queue.size();
+        int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();
+        int delayedMovesToProcess = queueSize > remainingCapacity ? queueSize - remainingCapacity : 0;
+        if (delayedMovesToProcess > 0 && pool.delayedMoveSessions.size() > 0) {
+          int i = 0;
+          Iterator<MoveSession> itr = pool.delayedMoveSessions.iterator();
+          while (i < delayedMovesToProcess && itr.hasNext()) {
+            MoveSession moveSession = itr.next();
+            itr.remove();
+            WmTezSession srcSession = moveSession.srcSession;
+            if (pool.sessions.contains(srcSession)) {
+              LOG.info("Processing delayed move {} for pool {} in wm main thread as the pool has queued requests",
+                  moveSession, poolName);
+              i++;
+              handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse, recordMoveEvents,
+                  true);
+            } else

Review comment:
       Shall remove else block. It is redundant.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -816,18 +850,34 @@ private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
             LOG.error("Failed to move session: {}. Session is not added to destination.", moveSession);
           }
         } else {
-          WmTezSession session = moveSession.srcSession;
-          KillQueryContext killQueryContext = new KillQueryContext(session, "Destination pool " + destPoolName +
-            " is full. Killing query.");
-          resetAndQueueKill(syncWork.toKillQuery, killQueryContext, toReuse);
+        LOG.error("Failed to move session: {}. Session is not removed from its pool.", moveSession);

Review comment:
       nit: Misaligned statement. Add 2 spaces.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -657,7 +661,37 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     }
     e.toReuse.clear();
 
-    // 9. Resolve all the kill query requests in flight. Nothing below can affect them.
+    // 9. If delayed move is set to true, for pools which have queued requests, process the "delayed moves" now.
+    // Incoming requests have more claim upon the pool than the delayed moves
+    if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE)) {
+      for (String poolName : poolsToRedistribute) {
+        PoolState pool = pools.get(poolName);
+        if (pool == null)
+          return;
+        int queueSize = pool.queue.size();
+        int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();
+        int delayedMovesToProcess = queueSize > remainingCapacity ? queueSize - remainingCapacity : 0;
+        if (delayedMovesToProcess > 0 && pool.delayedMoveSessions.size() > 0) {
+          int i = 0;
+          Iterator<MoveSession> itr = pool.delayedMoveSessions.iterator();
+          while (i < delayedMovesToProcess && itr.hasNext()) {

Review comment:
       i < delayedMovesToProcess can be moved inside the loop to avoid redundant checks if i is not incremented.
   
   i++;
   handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse, recordMoveEvents,
                     true);
   if (i >= delayedMovesToProcess) {
     break;
   }

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -794,17 +828,17 @@ private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
     final WmThreadSyncWork syncWork,
     final HashSet<String> poolsToRedistribute,
     final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+    final Map<WmTezSession, WmEvent> recordMoveEvents, final boolean moveImmediately) {

Review comment:
       To keep it uniform with the new config, can we rename the variable as "delayedMove" instead of "moveImmediately" and use it accordingly?

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -657,7 +661,37 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     }
     e.toReuse.clear();
 
-    // 9. Resolve all the kill query requests in flight. Nothing below can affect them.
+    // 9. If delayed move is set to true, for pools which have queued requests, process the "delayed moves" now.
+    // Incoming requests have more claim upon the pool than the delayed moves
+    if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE)) {
+      for (String poolName : poolsToRedistribute) {
+        PoolState pool = pools.get(poolName);
+        if (pool == null)
+          return;
+        int queueSize = pool.queue.size();
+        int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();
+        int delayedMovesToProcess = queueSize > remainingCapacity ? queueSize - remainingCapacity : 0;
+        if (delayedMovesToProcess > 0 && pool.delayedMoveSessions.size() > 0) {
+          int i = 0;

Review comment:
       nit: Shall use the name "movedCount" instead of "i".




-- 
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.

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


[GitHub] [hive] Dawn2111 commented on a change in pull request #2065: HIVE-2420: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r621674045



##########
File path: ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java
##########
@@ -1110,6 +1110,94 @@ public void testMoveSessionsMultiPool() throws Exception {
     assertFalse(allSessionProviders.get("A").getSessions().contains(sessionA1));
   }
 
+  @Test(timeout=10000)
+  public void testDelayedMoveSessions() throws Exception {
+    final HiveConf conf = createConfForDelayedMove();
+    MockQam qam = new MockQam();
+    WMFullResourcePlan plan = new WMFullResourcePlan(plan(), Lists.newArrayList(
+        pool("A", 2, 0.6f), pool("B", 1, 0.4f)));
+    plan.setMappings(Lists.newArrayList(mapping("A", "A"), mapping("B", "B")));
+    final WorkloadManager wm = new WorkloadManagerForTest("test", conf, qam, plan);
+    wm.start();
+
+    WmTezSession sessionA1 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+
+    // [A: 1, B: 0]
+    Map<String, SessionTriggerProvider> allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(1, allSessionProviders.get("A").getSessions().size());
+    assertEquals(0, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA1));
+    assertFalse(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.6f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA1.getPoolName());
+
+    // If dest pool has capacity, move immediately
+    // [A: 0, B: 1]
+    Future<Boolean> future = wm.applyMoveSessionAsync(sessionA1, "B");
+    assertNotNull(future.get());
+    assertTrue(future.get());
+    wm.addTestEvent().get();
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(0, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertFalse(allSessionProviders.get("A").getSessions().contains(sessionA1));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("B", sessionA1.getPoolName());
+
+    WmTezSession sessionA2 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+    // [A: 1, B: 1]
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(1, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA2));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.6f, sessionA2.getClusterFraction(), EPSILON);
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA2.getPoolName());
+    assertEquals("B", sessionA1.getPoolName());
+
+    // Dest pool is maxed out. Keep running in source pool
+    // [A: 1, B: 1]
+    future = wm.applyMoveSessionAsync(sessionA2, "B");
+    assertNotNull(future.get());
+    assertFalse(future.get());
+    wm.addTestEvent().get();
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(1, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA2));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.6f, sessionA2.getClusterFraction(), EPSILON);
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA2.getPoolName());
+    assertEquals("B", sessionA1.getPoolName());
+
+    // A has queued requests. The new requests should get accepted. The delayed move should be killed
+    WmTezSession sessionA3 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+    WmTezSession sessionA4 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+
+    while(sessionA2.isOpen()) {
+      Thread.sleep(100);
+    }
+    assertNull(sessionA2.getPoolName());
+    assertEquals("Destination pool B is full. Killing query.", sessionA2.getReasonForKill());
+
+    // [A: 2, B: 1]
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(2, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA3));
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA4));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.3f, sessionA3.getClusterFraction(), EPSILON);
+    assertEquals(0.3f, sessionA4.getClusterFraction(), EPSILON);
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA3.getPoolName());
+    assertEquals("A", sessionA4.getPoolName());
+    assertEquals("B", sessionA1.getPoolName());

Review comment:
       Added

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java
##########
@@ -47,8 +47,10 @@ public void applyAction(final Map<WmTezSession, Trigger> queriesViolated) {
           break;
         case MOVE_TO_POOL:
           String destPoolName = entry.getValue().getAction().getPoolName();
-          Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);
-          moveFutures.put(wmTezSession, moveFuture);
+          if (!wmTezSession.isDelayedMove()) {

Review comment:
       The trigger validator thread iterates through all the sessions and checks for trigger violations at specified intervals.  Even for a move trigger without delayed move config set, every 500 ms or so, the trigger would be validated against all the sessions. But, yes, if delayed move config is set, the session would be found as violated initially and then marked as no-op when the action for the trigger is invoked. 

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java
##########
@@ -47,8 +47,10 @@ public void applyAction(final Map<WmTezSession, Trigger> queriesViolated) {
           break;
         case MOVE_TO_POOL:
           String destPoolName = entry.getValue().getAction().getPoolName();
-          Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);
-          moveFutures.put(wmTezSession, moveFuture);
+          if (!wmTezSession.isDelayedMove()) {

Review comment:
       The trigger validator thread anyways iterates through all the sessions and checks for trigger violations at specified intervals.  Even for a move trigger without delayed move config set, every 500 ms or so, the trigger would be validated against all the sessions. But, yes, if delayed move config is set, the session would be found as violated initially and then marked as no-op when the action for the trigger is invoked. 

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -790,45 +842,72 @@ private void dumpPoolState(PoolState ps, List<String> set) {
     }
   }
 
-  private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
-    final WmThreadSyncWork syncWork,
-    final HashSet<String> poolsToRedistribute,
-    final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+  private static enum MoveSessionResult {
+    OK, // Normal case - the session was moved.
+    KILLED, // Killed because destination pool was full and delayed move is false.
+    CONVERTED_TO_DELAYED_MOVE, // the move session was added to the pool's delayed moves as the dest. pool was full
+    // and delayed move is true.
+    ERROR
+  }
+
+  private MoveSessionResult handleMoveSessionOnMasterThread(final MoveSession moveSession,
+      final WmThreadSyncWork syncWork,
+      final HashSet<String> poolsToRedistribute,
+      final Map<WmTezSession, GetRequest> toReuse,
+      final Map<WmTezSession, WmEvent> recordMoveEvents,
+      final boolean convertToDelayedMove) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, Convert to Delayed Move: {}", moveSession, convertToDelayedMove);
     if (validMove(moveSession.srcSession, destPoolName)) {
+      String srcPoolName = moveSession.srcSession.getPoolName();
+      PoolState srcPool = pools.get(srcPoolName);
+      boolean capacityAvailableInDest = capacityAvailable(destPoolName);
+      // If delayed move is set to true and if destination pool doesn't have enough capacity, don't kill the query.
+      // Let the query run in source pool. Add the session to the source pool's delayed move sessions.
+      if (convertToDelayedMove && !capacityAvailableInDest) {
+        srcPool.delayedMoveSessions.add(moveSession);
+        moveSession.srcSession.setDelayedMove(true);

Review comment:
       When a pool is updated or destroyed as a consequence of disabling WLM , all the sessions in the pool are removed and we remove the delayed move sessions at that time too - extractAllSessionsToKill().

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -790,45 +842,72 @@ private void dumpPoolState(PoolState ps, List<String> set) {
     }
   }
 
-  private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
-    final WmThreadSyncWork syncWork,
-    final HashSet<String> poolsToRedistribute,
-    final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+  private static enum MoveSessionResult {
+    OK, // Normal case - the session was moved.
+    KILLED, // Killed because destination pool was full and delayed move is false.
+    CONVERTED_TO_DELAYED_MOVE, // the move session was added to the pool's delayed moves as the dest. pool was full
+    // and delayed move is true.
+    ERROR
+  }
+
+  private MoveSessionResult handleMoveSessionOnMasterThread(final MoveSession moveSession,
+      final WmThreadSyncWork syncWork,
+      final HashSet<String> poolsToRedistribute,
+      final Map<WmTezSession, GetRequest> toReuse,
+      final Map<WmTezSession, WmEvent> recordMoveEvents,
+      final boolean convertToDelayedMove) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, Convert to Delayed Move: {}", moveSession, convertToDelayedMove);
     if (validMove(moveSession.srcSession, destPoolName)) {
+      String srcPoolName = moveSession.srcSession.getPoolName();
+      PoolState srcPool = pools.get(srcPoolName);
+      boolean capacityAvailableInDest = capacityAvailable(destPoolName);
+      // If delayed move is set to true and if destination pool doesn't have enough capacity, don't kill the query.
+      // Let the query run in source pool. Add the session to the source pool's delayed move sessions.
+      if (convertToDelayedMove && !capacityAvailableInDest) {
+        srcPool.delayedMoveSessions.add(moveSession);
+        moveSession.srcSession.setDelayedMove(true);

Review comment:
       When a pool is updated or destroyed as a consequence of disabling WLM , all the sessions in the pool are removed and we remove the delayed move sessions at that time too - PoolState.extractAllSessionsToKill().




-- 
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.

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


[GitHub] [hive] Dawn2111 commented on a change in pull request #2065: HIVE-2420: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r620624406



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -1248,6 +1334,42 @@ private void processPoolChangesOnMasterThread(
     }
   }
 
+  private void processDelayedMovesForPool(final String poolName, final HashSet<String> poolsToRedistribute, final Map<WmTezSession, WmEvent> recordMoveEvents,
+      WmThreadSyncWork syncWork, IdentityHashMap<WmTezSession, GetRequest> toReuse) {
+    long currentTime = System.currentTimeMillis();
+    PoolState pool = pools.get(poolName);
+    int movedCount = 0;
+    int queueSize = pool.queue.size();
+    int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();

Review comment:
       Yes. A new request will wake up the master thread.




-- 
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.

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


[GitHub] [hive] Dawn2111 commented on a change in pull request #2065: HIVE-2420: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r621834393



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java
##########
@@ -47,8 +47,10 @@ public void applyAction(final Map<WmTezSession, Trigger> queriesViolated) {
           break;
         case MOVE_TO_POOL:
           String destPoolName = entry.getValue().getAction().getPoolName();
-          Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);
-          moveFutures.put(wmTezSession, moveFuture);
+          if (!wmTezSession.isDelayedMove()) {
+            Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);

Review comment:
       I dont think we need to because any query being completed/killed in the destination pool will create a return/kill event. This  in turn will wake up the master thread which will retry the delayed move in the same iteration of the master thread loop. So the existing delayed moves will be processed earlier than any subsequent move events.




-- 
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.

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


[GitHub] [hive] Dawn2111 commented on a change in pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r599773480



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -794,17 +828,17 @@ private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
     final WmThreadSyncWork syncWork,
     final HashSet<String> poolsToRedistribute,
     final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+    final Map<WmTezSession, WmEvent> recordMoveEvents, final boolean moveImmediately) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, move immediately: {}", moveSession, moveImmediately);
     if (validMove(moveSession.srcSession, destPoolName)) {
       WmEvent moveEvent = new WmEvent(WmEvent.EventType.MOVE);
-      // remove from src pool
-      RemoveSessionResult rr = checkAndRemoveSessionFromItsPool(
+      // check if there is capacity in dest pool
+      if (capacityAvailable(destPoolName)) {

Review comment:
       Thanks for catch. The session should be removed from the source pool when the query is killed. However, cannot do exactly as suggested above as we don't want to remove the session from the source pool if delayedMove is true and dest pool is maxed out. Changed it like this :
   Left the flow as it is if delayedMove is false. Added a block for the case when delayed move is true and dest. pool is maxed out. This adds the session to the source pool's delayedMoveSessions.




-- 
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.

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


[GitHub] [hive] sankarh commented on pull request #2065: HIVE-24201: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
sankarh commented on pull request #2065:
URL: https://github.com/apache/hive/pull/2065#issuecomment-829425475


   +1, Patch looks good.


-- 
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.

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


[GitHub] [hive] Dawn2111 commented on pull request #2065: HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on pull request #2065:
URL: https://github.com/apache/hive/pull/2065#issuecomment-813531492


   @sankarh, refactored code to mark the WmTezSession as delayed move so that duplicate move session events are not created for a delayed move every time the triggers are validated. Also, added delayed move retry and expiry. Could you please review?


-- 
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.

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


[GitHub] [hive] sankarh commented on a change in pull request #2065: HIVE-24201: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
sankarh commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r622334824



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -682,11 +685,8 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     // as possible
     Map<WmTezSession, WmEvent> recordMoveEvents = new HashMap<>();
     for (MoveSession moveSession : e.moveSessions) {
-      if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE)) {
-        handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse, recordMoveEvents, true);
-      } else {
-        handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse, recordMoveEvents, false);
-      }
+      handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse,

Review comment:
       Shall store the HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE) in a boolean variable outside the "for" loop and use it here.

##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -3753,11 +3753,12 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
         "Determines behavior of the wm move trigger when destination pool is full.\n" +
         "If true, the query will run in source pool as long as possible if destination pool is full;\n" +
         "if false, the query will be killed if destination pool is full."),
-    HIVE_SERVER2_WM_DELAYED_MOVE_TIMEOUT("hive.server2.wm.delayed.move.timeout", "600",
+    HIVE_SERVER2_WM_DELAYED_MOVE_TIMEOUT("hive.server2.wm.delayed.move.timeout", "3600",
         new TimeValidator(TimeUnit.SECONDS),
         "The amount of time a delayed move is allowed to run in the source pool,\n" +
-        "when a delayed move session times out, the session is moved to the destination pool.\n"),
-    HIVE_SERVER2_WM_DELAYED_MOVE_VALIDATOR_INTERVAL("hive.server2.wm.delayed.move.validator.interval", "10",
+        "when a delayed move session times out, the session is moved to the destination pool.\n" +
+        "A value of 0 indicates no timeout"),
+    HIVE_SERVER2_WM_DELAYED_MOVE_VALIDATOR_INTERVAL("hive.server2.wm.delayed.move.validator.interval", "60",
         new TimeValidator(TimeUnit.SECONDS),
         "Interval for checking for expired delayed moves and retries. Value of 0 indicates no checks."),

Review comment:
       I think, we shouldn't allow 0 for interval. It creates confusion as we set timeout but it doesn't work.




-- 
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.

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


[GitHub] [hive] Dawn2111 commented on a change in pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r599773480



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -794,17 +828,17 @@ private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
     final WmThreadSyncWork syncWork,
     final HashSet<String> poolsToRedistribute,
     final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+    final Map<WmTezSession, WmEvent> recordMoveEvents, final boolean moveImmediately) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, move immediately: {}", moveSession, moveImmediately);
     if (validMove(moveSession.srcSession, destPoolName)) {
       WmEvent moveEvent = new WmEvent(WmEvent.EventType.MOVE);
-      // remove from src pool
-      RemoveSessionResult rr = checkAndRemoveSessionFromItsPool(
+      // check if there is capacity in dest pool
+      if (capacityAvailable(destPoolName)) {

Review comment:
       Thanks for catch! This introduced a bug as the session wasn't being removed from the source pool when the query was killed. However, cannot do exactly as suggested above as we don't want to remove the session from the source pool if delayedMove is true and dest pool is maxed out. Changed it like this :
   Left the flow as it is if delayedMove is false. Added a block for the case when delayed move is true and dest. pool is maxed out. This adds the session to the source pool's delayedMoveSessions.




-- 
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.

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


[GitHub] [hive] Dawn2111 removed a comment on pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 removed a comment on pull request #2065:
URL: https://github.com/apache/hive/pull/2065#issuecomment-800405747


   @guptanikhil007, could you please review ?


-- 
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.

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


[GitHub] [hive] Dawn2111 commented on pull request #2065: [WIP] HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on pull request #2065:
URL: https://github.com/apache/hive/pull/2065#issuecomment-802520264


   @sankarh, Could you please review?


-- 
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.

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


[GitHub] [hive] Dawn2111 commented on a change in pull request #2065: HIVE-2420: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
Dawn2111 commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r621709390



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -790,45 +842,72 @@ private void dumpPoolState(PoolState ps, List<String> set) {
     }
   }
 
-  private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
-    final WmThreadSyncWork syncWork,
-    final HashSet<String> poolsToRedistribute,
-    final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+  private static enum MoveSessionResult {
+    OK, // Normal case - the session was moved.
+    KILLED, // Killed because destination pool was full and delayed move is false.
+    CONVERTED_TO_DELAYED_MOVE, // the move session was added to the pool's delayed moves as the dest. pool was full
+    // and delayed move is true.
+    ERROR
+  }
+
+  private MoveSessionResult handleMoveSessionOnMasterThread(final MoveSession moveSession,
+      final WmThreadSyncWork syncWork,
+      final HashSet<String> poolsToRedistribute,
+      final Map<WmTezSession, GetRequest> toReuse,
+      final Map<WmTezSession, WmEvent> recordMoveEvents,
+      final boolean convertToDelayedMove) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, Convert to Delayed Move: {}", moveSession, convertToDelayedMove);
     if (validMove(moveSession.srcSession, destPoolName)) {
+      String srcPoolName = moveSession.srcSession.getPoolName();
+      PoolState srcPool = pools.get(srcPoolName);
+      boolean capacityAvailableInDest = capacityAvailable(destPoolName);
+      // If delayed move is set to true and if destination pool doesn't have enough capacity, don't kill the query.
+      // Let the query run in source pool. Add the session to the source pool's delayed move sessions.
+      if (convertToDelayedMove && !capacityAvailableInDest) {
+        srcPool.delayedMoveSessions.add(moveSession);
+        moveSession.srcSession.setDelayedMove(true);

Review comment:
       When a pool is updated or destroyed as a consequence of disabling WLM , all the sessions in the pool are removed and we remove the delayed move sessions at that time as well - PoolState.extractAllSessionsToKill().




-- 
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.

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


[GitHub] [hive] sankarh commented on a change in pull request #2065: HIVE-24201-WorkloadManager kills query being moved to different pool if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
sankarh commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r620083627



##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -3749,6 +3749,17 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
         new TimeValidator(TimeUnit.SECONDS),
         "The timeout for AM registry registration, after which (on attempting to use the\n" +
         "session), we kill it and try to get another one."),
+    HIVE_SERVER2_WM_DELAYED_MOVE("hive.server2.wm.delayed.move", false,
+        "Determines behavior of the wm move trigger when destination pool is full.\n" +
+        "If true, the query will run in source pool as long as possible if destination pool is full;\n" +
+        "if false, the query will be killed if destination pool is full."),
+    HIVE_SERVER2_WM_DELAYED_MOVE_TIMEOUT("hive.server2.wm.delayed.move.timeout", "600",
+        new TimeValidator(TimeUnit.SECONDS),
+        "The amount of time a delayed move is allowed to run in the source pool,\n" +
+        "when a delayed move session times out, the session is moved to the destination pool.\n"),
+    HIVE_SERVER2_WM_DELAYED_MOVE_VALIDATOR_INTERVAL("hive.server2.wm.delayed.move.validator.interval", "10",
+        new TimeValidator(TimeUnit.SECONDS),
+        "Interval for checking for expired delayed moves and retries. Value of 0 indicates no checks."),

Review comment:
       Does "0" means no timeout check or no support of delayed move itself? I think, in any case, this creates confusion. We shouldn't allow 0 and this config should be > 0.
   hive.server2.wm.delayed.move.timeout=0 can be used for no timeout case.

##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -3749,6 +3749,17 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
         new TimeValidator(TimeUnit.SECONDS),
         "The timeout for AM registry registration, after which (on attempting to use the\n" +
         "session), we kill it and try to get another one."),
+    HIVE_SERVER2_WM_DELAYED_MOVE("hive.server2.wm.delayed.move", false,
+        "Determines behavior of the wm move trigger when destination pool is full.\n" +
+        "If true, the query will run in source pool as long as possible if destination pool is full;\n" +
+        "if false, the query will be killed if destination pool is full."),
+    HIVE_SERVER2_WM_DELAYED_MOVE_TIMEOUT("hive.server2.wm.delayed.move.timeout", "600",
+        new TimeValidator(TimeUnit.SECONDS),
+        "The amount of time a delayed move is allowed to run in the source pool,\n" +
+        "when a delayed move session times out, the session is moved to the destination pool.\n"),

Review comment:
       If value 0 have special meaning such as "doesn't expire", then need to capture it here.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -790,45 +842,72 @@ private void dumpPoolState(PoolState ps, List<String> set) {
     }
   }
 
-  private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
-    final WmThreadSyncWork syncWork,
-    final HashSet<String> poolsToRedistribute,
-    final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+  private static enum MoveSessionResult {
+    OK, // Normal case - the session was moved.
+    KILLED, // Killed because destination pool was full and delayed move is false.
+    CONVERTED_TO_DELAYED_MOVE, // the move session was added to the pool's delayed moves as the dest. pool was full
+    // and delayed move is true.
+    ERROR
+  }
+
+  private MoveSessionResult handleMoveSessionOnMasterThread(final MoveSession moveSession,
+      final WmThreadSyncWork syncWork,
+      final HashSet<String> poolsToRedistribute,
+      final Map<WmTezSession, GetRequest> toReuse,
+      final Map<WmTezSession, WmEvent> recordMoveEvents,
+      final boolean convertToDelayedMove) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, Convert to Delayed Move: {}", moveSession, convertToDelayedMove);
     if (validMove(moveSession.srcSession, destPoolName)) {
+      String srcPoolName = moveSession.srcSession.getPoolName();
+      PoolState srcPool = pools.get(srcPoolName);
+      boolean capacityAvailableInDest = capacityAvailable(destPoolName);
+      // If delayed move is set to true and if destination pool doesn't have enough capacity, don't kill the query.
+      // Let the query run in source pool. Add the session to the source pool's delayed move sessions.
+      if (convertToDelayedMove && !capacityAvailableInDest) {
+        srcPool.delayedMoveSessions.add(moveSession);
+        moveSession.srcSession.setDelayedMove(true);

Review comment:
       What happens when we de-activate WLM resource plan and have sessions in delayed move list?

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -1248,6 +1334,42 @@ private void processPoolChangesOnMasterThread(
     }
   }
 
+  private void processDelayedMovesForPool(final String poolName, final HashSet<String> poolsToRedistribute, final Map<WmTezSession, WmEvent> recordMoveEvents,
+      WmThreadSyncWork syncWork, IdentityHashMap<WmTezSession, GetRequest> toReuse) {
+    long currentTime = System.currentTimeMillis();
+    PoolState pool = pools.get(poolName);
+    int movedCount = 0;
+    int queueSize = pool.queue.size();
+    int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();
+    int delayedMovesToProcess = (queueSize > remainingCapacity) ? (queueSize - remainingCapacity) : 0;
+    Iterator<MoveSession> iter = pool.delayedMoveSessions.iterator();
+    while (iter.hasNext()) {
+      MoveSession moveSession = iter.next();
+      MoveSessionResult result;
+      //Discard the delayed move if invalid
+      if (!validDelayedMove(moveSession, pool, poolName)) {
+        iter.remove();
+        continue;
+      }
+      // Process the delayed move if

Review comment:
       nit: Add a blank line before the comments.

##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -3749,6 +3749,17 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
         new TimeValidator(TimeUnit.SECONDS),
         "The timeout for AM registry registration, after which (on attempting to use the\n" +
         "session), we kill it and try to get another one."),
+    HIVE_SERVER2_WM_DELAYED_MOVE("hive.server2.wm.delayed.move", false,
+        "Determines behavior of the wm move trigger when destination pool is full.\n" +
+        "If true, the query will run in source pool as long as possible if destination pool is full;\n" +
+        "if false, the query will be killed if destination pool is full."),
+    HIVE_SERVER2_WM_DELAYED_MOVE_TIMEOUT("hive.server2.wm.delayed.move.timeout", "600",

Review comment:
       The default can be round figure of 1 hour. You can set it as 360.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -790,45 +842,72 @@ private void dumpPoolState(PoolState ps, List<String> set) {
     }
   }
 
-  private void handleMoveSessionOnMasterThread(final MoveSession moveSession,
-    final WmThreadSyncWork syncWork,
-    final HashSet<String> poolsToRedistribute,
-    final Map<WmTezSession, GetRequest> toReuse,
-    final Map<WmTezSession, WmEvent> recordMoveEvents) {
+  private static enum MoveSessionResult {
+    OK, // Normal case - the session was moved.
+    KILLED, // Killed because destination pool was full and delayed move is false.
+    CONVERTED_TO_DELAYED_MOVE, // the move session was added to the pool's delayed moves as the dest. pool was full
+    // and delayed move is true.
+    ERROR
+  }
+
+  private MoveSessionResult handleMoveSessionOnMasterThread(final MoveSession moveSession,
+      final WmThreadSyncWork syncWork,
+      final HashSet<String> poolsToRedistribute,
+      final Map<WmTezSession, GetRequest> toReuse,
+      final Map<WmTezSession, WmEvent> recordMoveEvents,
+      final boolean convertToDelayedMove) {
     String destPoolName = moveSession.destPool;
-    LOG.info("Handling move session event: {}", moveSession);
+    LOG.info("Handling move session event: {}, Convert to Delayed Move: {}", moveSession, convertToDelayedMove);
     if (validMove(moveSession.srcSession, destPoolName)) {
+      String srcPoolName = moveSession.srcSession.getPoolName();
+      PoolState srcPool = pools.get(srcPoolName);
+      boolean capacityAvailableInDest = capacityAvailable(destPoolName);
+      // If delayed move is set to true and if destination pool doesn't have enough capacity, don't kill the query.
+      // Let the query run in source pool. Add the session to the source pool's delayed move sessions.
+      if (convertToDelayedMove && !capacityAvailableInDest) {
+        srcPool.delayedMoveSessions.add(moveSession);
+        moveSession.srcSession.setDelayedMove(true);
+        LOG.info("Converting Move: {} to a delayed move. Since destination pool {} is full, running in source pool {}"
+            + "as long as possible.", moveSession, destPoolName, srcPoolName);

Review comment:
       Add a space before "as" or else pool name looks incorrect.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -421,6 +446,22 @@ private void runWmThread() {
     }
   }
 
+  private void runDelayedMoveThread() {
+    while (true) {

Review comment:
       Add an info log here with delayedMoveValidationIntervalMs.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -641,7 +682,11 @@ private void processCurrentEvents(EventState e, WmThreadSyncWork syncWork) throw
     // as possible
     Map<WmTezSession, WmEvent> recordMoveEvents = new HashMap<>();
     for (MoveSession moveSession : e.moveSessions) {
-      handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, e.toReuse, recordMoveEvents);
+      if (HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE)) {

Review comment:
       Instead of "if-else", we can directly pass the value of HiveConf.getBoolVar(conf, ConfVars.HIVE_SERVER2_WM_DELAYED_MOVE) as last argument.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -1248,6 +1334,42 @@ private void processPoolChangesOnMasterThread(
     }
   }
 
+  private void processDelayedMovesForPool(final String poolName, final HashSet<String> poolsToRedistribute, final Map<WmTezSession, WmEvent> recordMoveEvents,
+      WmThreadSyncWork syncWork, IdentityHashMap<WmTezSession, GetRequest> toReuse) {
+    long currentTime = System.currentTimeMillis();
+    PoolState pool = pools.get(poolName);
+    int movedCount = 0;
+    int queueSize = pool.queue.size();
+    int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();
+    int delayedMovesToProcess = (queueSize > remainingCapacity) ? (queueSize - remainingCapacity) : 0;
+    Iterator<MoveSession> iter = pool.delayedMoveSessions.iterator();
+    while (iter.hasNext()) {
+      MoveSession moveSession = iter.next();
+      MoveSessionResult result;
+      //Discard the delayed move if invalid
+      if (!validDelayedMove(moveSession, pool, poolName)) {
+        iter.remove();
+        continue;
+      }
+      // Process the delayed move if
+      // 1. The delayed move has timed out or
+      // 2. The destination pool has freed up or
+      // 3. If the source pool has incoming requests and we need to free up capacity in the source pool
+      // to accommodate these requests.
+      if (((currentTime - moveSession.startTime) >= delayedMoveTimeOutMs) || (capacityAvailable(moveSession.destPool))
+          || (movedCount < delayedMovesToProcess)) {
+        LOG.info("Processing delayed move {} for pool {}", moveSession, poolName);
+        result = handleMoveSessionOnMasterThread(moveSession, syncWork, poolsToRedistribute, toReuse, recordMoveEvents,
+            false);
+        iter.remove();
+        if (result == MoveSessionResult.OK) {

Review comment:
       I think movedCount should be incremented for both moved or killed sessions. 

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -1248,6 +1334,42 @@ private void processPoolChangesOnMasterThread(
     }
   }
 
+  private void processDelayedMovesForPool(final String poolName, final HashSet<String> poolsToRedistribute, final Map<WmTezSession, WmEvent> recordMoveEvents,
+      WmThreadSyncWork syncWork, IdentityHashMap<WmTezSession, GetRequest> toReuse) {
+    long currentTime = System.currentTimeMillis();
+    PoolState pool = pools.get(poolName);
+    int movedCount = 0;
+    int queueSize = pool.queue.size();
+    int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();

Review comment:
       Will queuing a query to given pool wake up master thread? If not, then we have issues where a query might wait in worst case 10 secs to process delayed move.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
##########
@@ -1248,6 +1334,42 @@ private void processPoolChangesOnMasterThread(
     }
   }
 
+  private void processDelayedMovesForPool(final String poolName, final HashSet<String> poolsToRedistribute, final Map<WmTezSession, WmEvent> recordMoveEvents,
+      WmThreadSyncWork syncWork, IdentityHashMap<WmTezSession, GetRequest> toReuse) {
+    long currentTime = System.currentTimeMillis();
+    PoolState pool = pools.get(poolName);
+    int movedCount = 0;
+    int queueSize = pool.queue.size();
+    int remainingCapacity = pool.queryParallelism - pool.getTotalActiveSessions();
+    int delayedMovesToProcess = (queueSize > remainingCapacity) ? (queueSize - remainingCapacity) : 0;
+    Iterator<MoveSession> iter = pool.delayedMoveSessions.iterator();
+    while (iter.hasNext()) {
+      MoveSession moveSession = iter.next();
+      MoveSessionResult result;
+      //Discard the delayed move if invalid
+      if (!validDelayedMove(moveSession, pool, poolName)) {
+        iter.remove();
+        continue;
+      }
+      // Process the delayed move if
+      // 1. The delayed move has timed out or
+      // 2. The destination pool has freed up or
+      // 3. If the source pool has incoming requests and we need to free up capacity in the source pool
+      // to accommodate these requests.
+      if (((currentTime - moveSession.startTime) >= delayedMoveTimeOutMs) || (capacityAvailable(moveSession.destPool))

Review comment:
       We can check (movedCount < delayedMovesToProcess) first, next capacityAvailable and last timeout as it is likely case and can save few cpu cycles.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java
##########
@@ -47,8 +47,10 @@ public void applyAction(final Map<WmTezSession, Trigger> queriesViolated) {
           break;
         case MOVE_TO_POOL:
           String destPoolName = entry.getValue().getAction().getPoolName();
-          Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);
-          moveFutures.put(wmTezSession, moveFuture);
+          if (!wmTezSession.isDelayedMove()) {
+            Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);

Review comment:
       How do we ensure, when we move current session to dest pool, we first check any delayed moves queued up for given dest pool before we process this one? 
   I think, we should perform Step-9 before Step-8 to give higher priority to delayed moves.

##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -3749,6 +3749,17 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
         new TimeValidator(TimeUnit.SECONDS),
         "The timeout for AM registry registration, after which (on attempting to use the\n" +
         "session), we kill it and try to get another one."),
+    HIVE_SERVER2_WM_DELAYED_MOVE("hive.server2.wm.delayed.move", false,
+        "Determines behavior of the wm move trigger when destination pool is full.\n" +
+        "If true, the query will run in source pool as long as possible if destination pool is full;\n" +
+        "if false, the query will be killed if destination pool is full."),
+    HIVE_SERVER2_WM_DELAYED_MOVE_TIMEOUT("hive.server2.wm.delayed.move.timeout", "600",
+        new TimeValidator(TimeUnit.SECONDS),
+        "The amount of time a delayed move is allowed to run in the source pool,\n" +
+        "when a delayed move session times out, the session is moved to the destination pool.\n"),
+    HIVE_SERVER2_WM_DELAYED_MOVE_VALIDATOR_INTERVAL("hive.server2.wm.delayed.move.validator.interval", "10",
+        new TimeValidator(TimeUnit.SECONDS),
+        "Interval for checking for expired delayed moves and retries. Value of 0 indicates no checks."),

Review comment:
       I think, retry should happen always at regular interval but timeout can be enabled only if hive.server2.wm.delayed.move.timeout>0. 

##########
File path: ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java
##########
@@ -1110,6 +1110,94 @@ public void testMoveSessionsMultiPool() throws Exception {
     assertFalse(allSessionProviders.get("A").getSessions().contains(sessionA1));
   }
 
+  @Test(timeout=10000)
+  public void testDelayedMoveSessions() throws Exception {
+    final HiveConf conf = createConfForDelayedMove();
+    MockQam qam = new MockQam();
+    WMFullResourcePlan plan = new WMFullResourcePlan(plan(), Lists.newArrayList(
+        pool("A", 2, 0.6f), pool("B", 1, 0.4f)));
+    plan.setMappings(Lists.newArrayList(mapping("A", "A"), mapping("B", "B")));
+    final WorkloadManager wm = new WorkloadManagerForTest("test", conf, qam, plan);
+    wm.start();
+
+    WmTezSession sessionA1 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+
+    // [A: 1, B: 0]
+    Map<String, SessionTriggerProvider> allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(1, allSessionProviders.get("A").getSessions().size());
+    assertEquals(0, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA1));
+    assertFalse(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.6f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA1.getPoolName());
+
+    // If dest pool has capacity, move immediately
+    // [A: 0, B: 1]
+    Future<Boolean> future = wm.applyMoveSessionAsync(sessionA1, "B");
+    assertNotNull(future.get());
+    assertTrue(future.get());
+    wm.addTestEvent().get();
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(0, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertFalse(allSessionProviders.get("A").getSessions().contains(sessionA1));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("B", sessionA1.getPoolName());
+
+    WmTezSession sessionA2 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+    // [A: 1, B: 1]
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(1, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA2));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.6f, sessionA2.getClusterFraction(), EPSILON);
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA2.getPoolName());
+    assertEquals("B", sessionA1.getPoolName());
+
+    // Dest pool is maxed out. Keep running in source pool
+    // [A: 1, B: 1]
+    future = wm.applyMoveSessionAsync(sessionA2, "B");
+    assertNotNull(future.get());
+    assertFalse(future.get());
+    wm.addTestEvent().get();
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(1, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA2));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.6f, sessionA2.getClusterFraction(), EPSILON);
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA2.getPoolName());
+    assertEquals("B", sessionA1.getPoolName());
+
+    // A has queued requests. The new requests should get accepted. The delayed move should be killed
+    WmTezSession sessionA3 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+    WmTezSession sessionA4 = (WmTezSession) wm.getSession(null, mappingInput("A"), conf);
+
+    while(sessionA2.isOpen()) {
+      Thread.sleep(100);
+    }
+    assertNull(sessionA2.getPoolName());
+    assertEquals("Destination pool B is full. Killing query.", sessionA2.getReasonForKill());
+
+    // [A: 2, B: 1]
+    allSessionProviders = wm.getAllSessionTriggerProviders();
+    assertEquals(2, allSessionProviders.get("A").getSessions().size());
+    assertEquals(1, allSessionProviders.get("B").getSessions().size());
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA3));
+    assertTrue(allSessionProviders.get("A").getSessions().contains(sessionA4));
+    assertTrue(allSessionProviders.get("B").getSessions().contains(sessionA1));
+    assertEquals(0.3f, sessionA3.getClusterFraction(), EPSILON);
+    assertEquals(0.3f, sessionA4.getClusterFraction(), EPSILON);
+    assertEquals(0.4f, sessionA1.getClusterFraction(), EPSILON);
+    assertEquals("A", sessionA3.getPoolName());
+    assertEquals("A", sessionA4.getPoolName());
+    assertEquals("B", sessionA1.getPoolName());

Review comment:
       Can we add test to verify the timeout/retry for capacity_available cases?

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java
##########
@@ -47,8 +47,10 @@ public void applyAction(final Map<WmTezSession, Trigger> queriesViolated) {
           break;
         case MOVE_TO_POOL:
           String destPoolName = entry.getValue().getAction().getPoolName();
-          Future<Boolean> moveFuture = wm.applyMoveSessionAsync(wmTezSession, destPoolName);
-          moveFutures.put(wmTezSession, moveFuture);
+          if (!wmTezSession.isDelayedMove()) {

Review comment:
       Will it add overhead in trigger validator thread if bunch of queries scheduled for delayed move? I mean, it generates trigger events continuously even though we no-op it here.




-- 
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.

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


[GitHub] [hive] sankarh commented on a change in pull request #2065: HIVE-24201: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
sankarh commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r622782514



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TriggerValidatorRunnable.java
##########
@@ -95,7 +95,7 @@ public void run() {
 
           Trigger chosenTrigger = violatedSessions.get(sessionState);
           if (chosenTrigger != null) {
-            LOG.info("Query: {}. {}. Applying action.", sessionState.getWmContext().getQueryId(),
+            LOG.debug("Query: {}. {}. Applying action.", sessionState.getWmContext().getQueryId(),

Review comment:
       Do we have some other log that signifies the trigger event is generated? Info logs are helping us to trace such events.




-- 
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.

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


[GitHub] [hive] sankarh merged pull request #2065: HIVE-24201: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
sankarh merged pull request #2065:
URL: https://github.com/apache/hive/pull/2065


   


-- 
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.

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


[GitHub] [hive] sankarh commented on a change in pull request #2065: HIVE-24201: WorkloadManager can support delayed move if destination pool does not have enough sessions

Posted by GitBox <gi...@apache.org>.
sankarh commented on a change in pull request #2065:
URL: https://github.com/apache/hive/pull/2065#discussion_r622782514



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TriggerValidatorRunnable.java
##########
@@ -95,7 +95,7 @@ public void run() {
 
           Trigger chosenTrigger = violatedSessions.get(sessionState);
           if (chosenTrigger != null) {
-            LOG.info("Query: {}. {}. Applying action.", sessionState.getWmContext().getQueryId(),
+            LOG.debug("Query: {}. {}. Applying action.", sessionState.getWmContext().getQueryId(),

Review comment:
       Do we have some other log that signifies the trigger event is generated?




-- 
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.

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