You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tez.apache.org by GitBox <gi...@apache.org> on 2021/11/13 15:30:20 UTC

[GitHub] [tez] amahussein opened a new pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

amahussein opened a new pull request #161:
URL: https://github.com/apache/tez/pull/161


   TEZ-4349 DAGClient gets stuck with invalid cached DAGStatu
   
   The `cachedDagStatus` should be valid for a certain amount of time, or certain number of retires.
   
   When the `cachedDAGStatus` expires, the DAGClient tries to pull from AM or the RM.
   An error in fetching the status from both AM and RM, would return null to the caller.
   
   - The expiration time can be configured using `TezConfiguration.TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES` `tez.client.dag.status.cache.timeout-minutes`, and the default is 5. The `timeUnit` of the expiration is minutes.
   - Added a new UT `TestDAGClient.testGetDagStatusWithCachedStatusExpiration`
   - ran the following unit tests: `mvn test -Dtest=TestDAGClient,TestTezClient,TestMockDAGAppMaster`


-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] amahussein commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
amahussein commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r750410092



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -1975,6 +1975,18 @@ public static void validateProperty(String property, Scope usedScope) {
       TEZ_PREFIX + "test.minicluster.app.wait.on.shutdown.secs";
   public static final long TEZ_TEST_MINI_CLUSTER_APP_WAIT_ON_SHUTDOWN_SECS_DEFAULT = 30;
 
+  /**
+   * Long value
+   * Status Cache timeout window in minutes for the DAGClient.
+   */
+  @Private
+  @ConfigurationScope(Scope.CLIENT)
+  @ConfigurationProperty(type="long")
+  public static final String TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES = TEZ_PREFIX
+      + "client.dag.status.cache.timeout-minutes";
+  // Default timeout is 5 minutes.
+  public static final long TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES_DEFAULT = 5;

Review comment:
       I am fine with seconds. However, I am not so sure about the impact of setting the expiration to a small value.
   For example, it is possible to have some delay in the AM executing the RPC call (or even a timeout at one time). If the expiration of the cache is smaller than the total of ("AM RPC with failure" + "wait for the client to retry" + "AM RPC"), then the cache won't serve its purpose.
   If I understand correctly, `cachedDAGStatus` purpose is to protect the client from falling too soon to the RM. Correct me if I misunderstand the purpose of the `cachedDAGStatus`.




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] abstractdog commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r748857804



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/client/DAGClientImpl.java
##########
@@ -221,13 +245,14 @@ protected DAGStatus getDAGStatusInternal(@Nullable Set<StatusGetOpts> statusOpti
       final DAGStatus dagStatus = getDAGStatusViaAM(statusOptions, timeout);
 
       if (!dagCompleted) {
-        if (dagStatus != null) {
-          cachedDagStatus = dagStatus;
+        if (dagStatus != null) { // update the cached DAGStatus
+          cachedDAGStatusRef.setValue(dagStatus);
           return dagStatus;
         }
-        if (cachedDagStatus != null) {
+        DAGStatus cachedDAG = cachedDAGStatusRef.getValue();
+        if (cachedDAG != null) {
           // could not get from AM (not reachable/ was killed). return cached status.
-          return cachedDagStatus;
+          return cachedDAG;

Review comment:
       am I right to assume this is the codepath where the original issue happened? could you please clarify how can we indefinitely stuck here?
   I mean, we can only hit this part if getDAGStatusViaAM returns null but dagCompleted is not true, so when we hit this again in getDAGStatusViaAM and again:
   ```
       } catch (TezException e) {
         // can be either due to a n/w issue of due to AM completed.
       } catch (IOException e) {
         // can be either due to a n/w issue of due to AM completed.
       }
   ```
   also getApplicationReportInternal keeps returning null in checkAndSetDagCompletionStatus
   
   was it the case for you? 
   if so, does it make sense to put at least debug level log messages to the silent catch branches?




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] amahussein commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
amahussein commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r749556005



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/client/DAGClientImpl.java
##########
@@ -221,13 +245,14 @@ protected DAGStatus getDAGStatusInternal(@Nullable Set<StatusGetOpts> statusOpti
       final DAGStatus dagStatus = getDAGStatusViaAM(statusOptions, timeout);
 
       if (!dagCompleted) {
-        if (dagStatus != null) {
-          cachedDagStatus = dagStatus;
+        if (dagStatus != null) { // update the cached DAGStatus
+          cachedDAGStatusRef.setValue(dagStatus);
           return dagStatus;
         }
-        if (cachedDagStatus != null) {
+        DAGStatus cachedDAG = cachedDAGStatusRef.getValue();
+        if (cachedDAG != null) {
           // could not get from AM (not reachable/ was killed). return cached status.
-          return cachedDagStatus;
+          return cachedDAG;

Review comment:
       Thanks for the feedback.
   
   Yes, considering the implementation of [TezJob.run()-Line222](https://github.com/apache/pig/blob/trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezJob.java#L222) in Pig:
   
   Pig polls on the DAGStatus inside an infinite loop:
   ```java
           while (true) {
               try {
                   dagStatus = dagClient.getDAGStatus(null);
               } catch (Exception e) {
                   log.info("Cannot retrieve DAG status", e);
                   break;
               }
              if (dagStatus.isCompleted()) {
                  // do something
                  // break;
              }
              sleep(1000);
          }  
   ```
   
   
   Let's assume the following scenario on Tez Side:
   - Pig first iteration calls `getDAGStatusViaAM()` which successfully pulls the DAGStatus and updates the cachedDAGStatus to running.
   - Pig sleeps 1000
   - second call from Pig calls `getDAGStatusViaAM()` which encounters `TezException` or `IOException`. The call would return the last cachedDAGStatus (which is running), instead of null.
   - Since the status is `running`, the Pig-thread sleeps
   - This will keep going as long as the `getDAGStatusViaAM()` fails, and the last valid DAGStatus is still cached.
   
   The problem in this corner case is that the Pig client will keep looping indefinitely as long as it does not receive a null or dagClient.getDAGStatus(null) does not throw an exception.
   From a client perspective, it is better to fail early in order to recover faster.




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] amahussein commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
amahussein commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r750618656



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -1975,6 +1975,18 @@ public static void validateProperty(String property, Scope usedScope) {
       TEZ_PREFIX + "test.minicluster.app.wait.on.shutdown.secs";
   public static final long TEZ_TEST_MINI_CLUSTER_APP_WAIT_ON_SHUTDOWN_SECS_DEFAULT = 30;
 
+  /**
+   * Long value
+   * Status Cache timeout window in minutes for the DAGClient.
+   */
+  @Private
+  @ConfigurationScope(Scope.CLIENT)
+  @ConfigurationProperty(type="long")
+  public static final String TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES = TEZ_PREFIX
+      + "client.dag.status.cache.timeout-minutes";
+  // Default timeout is 5 minutes.
+  public static final long TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES_DEFAULT = 5;

Review comment:
       The latest commit changes the TimeUnit to seconds and the default to 60.
   




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] abstractdog commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r749677672



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -1975,6 +1975,18 @@ public static void validateProperty(String property, Scope usedScope) {
       TEZ_PREFIX + "test.minicluster.app.wait.on.shutdown.secs";
   public static final long TEZ_TEST_MINI_CLUSTER_APP_WAIT_ON_SHUTDOWN_SECS_DEFAULT = 30;
 
+  /**
+   * Long value
+   * Status Cache timeout window in minutes for the DAGClient.
+   */
+  @Private
+  @ConfigurationScope(Scope.CLIENT)
+  @ConfigurationProperty(type="long")
+  public static final String TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES = TEZ_PREFIX
+      + "client.dag.status.cache.timeout-minutes";
+  // Default timeout is 5 minutes.
+  public static final long TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES_DEFAULT = 5;

Review comment:
       I think we can lower this to 1 minute, especially because in case of query status stuck (that the patch is about to address) this default value will be the minimum time in which the client has the chance to realize the stuck cached progress




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] abstractdog commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r775142329



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -1975,6 +1975,18 @@ public static void validateProperty(String property, Scope usedScope) {
       TEZ_PREFIX + "test.minicluster.app.wait.on.shutdown.secs";
   public static final long TEZ_TEST_MINI_CLUSTER_APP_WAIT_ON_SHUTDOWN_SECS_DEFAULT = 30;
 
+  /**
+   * Long value
+   * Status Cache timeout window in minutes for the DAGClient.
+   */
+  @Private
+  @ConfigurationScope(Scope.CLIENT)
+  @ConfigurationProperty(type="long")
+  public static final String TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES = TEZ_PREFIX
+      + "client.dag.status.cache.timeout-minutes";
+  // Default timeout is 5 minutes.
+  public static final long TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES_DEFAULT = 5;

Review comment:
       thanks @amahussein, I'm assuming the default value 60s would work properly




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] abstractdog commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r748856603



##########
File path: tez-api/src/test/java/org/apache/tez/dag/api/client/rpc/TestDAGClient.java
##########
@@ -578,4 +604,110 @@ public void testTimelineClientCleanup() throws Exception {
     }
     Assert.assertFalse("Reloader is still alive", reloaderStillAlive);
   }
+
+  @Test(timeout = 50000)
+  public void testGetDagStatusWithCachedStatusExpiration() throws Exception {
+    long startTime;
+    long endTime;
+    long diff;
+
+    TezConfiguration tezConf = new TezConfiguration();
+    tezConf.setLong(TezConfiguration.TEZ_DAG_STATUS_POLLINTERVAL_MS, 800l);
+    tezConf.setLong(TezConfiguration.TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES, 100000l);
+
+    DAGClientImplForTest dagClient =
+        new DAGClientImplForTest(mockAppId, dagIdStr, tezConf, null);
+    DAGClientRPCImplForTest dagClientRpc =
+        new DAGClientRPCImplForTest(mockAppId, dagIdStr, tezConf, null);
+    dagClient.setRealClient(dagClientRpc);
+
+    DAGStatus dagStatus;
+    DAGStatus rmDagStatus;
+
+    // Fetch from AM. RUNNING
+    rmDagStatus =
+        new DAGStatus(constructDagStatusProto(DAGStatusStateProto.DAG_RUNNING), DagStatusSource.RM);
+    dagClient.setRmDagStatus(rmDagStatus);
+    dagClientRpc.setAMProxy(createMockProxy(DAGStatusStateProto.DAG_RUNNING, -1));
+
+    startTime = System.currentTimeMillis();
+    dagStatus = dagClient.getDAGStatus(EnumSet.noneOf(StatusGetOpts.class), 2000l);
+    endTime = System.currentTimeMillis();
+    diff = endTime - startTime;
+    assertTrue(diff > 1500l && diff < 2500l);
+    // Directly from AM
+    assertEquals(0, dagClient.numGetStatusViaRmInvocations);
+    // Directly from AM - one refresh. One with timeout.
+    assertEquals(2, dagClientRpc.numGetStatusViaAmInvocations);
+    assertEquals(DAGStatus.State.RUNNING, dagStatus.getState());
+
+    // Fetch from AM. Success.
+    dagClient.resetCounters();
+    dagClientRpc.resetCountesr();
+    rmDagStatus =
+        new DAGStatus(constructDagStatusProto(DAGStatusStateProto.DAG_RUNNING), DagStatusSource.RM);
+    dagClient.setRmDagStatus(rmDagStatus);
+    dagClientRpc.setAMProxy(createMockProxy(DAGStatusStateProto.DAG_SUCCEEDED, 1000l));
+
+    startTime = System.currentTimeMillis();
+    dagStatus = dagClient.getDAGStatus(EnumSet.noneOf(StatusGetOpts.class), 2000l);
+    endTime = System.currentTimeMillis();
+    diff = endTime - startTime;
+    assertTrue("diff is " + diff, diff > 500l && diff < 1500l);
+    // Directly from AM
+    assertEquals(0, dagClient.numGetStatusViaRmInvocations);
+    // Directly from AM - previous request cached, so single invocation only.
+    assertEquals(1, dagClientRpc.numGetStatusViaAmInvocations);
+    assertEquals(DAGStatus.State.SUCCEEDED, dagStatus.getState());
+
+    // verify that the cachedDAGStatus is correct
+    DAGStatus cachedDagStatus = dagClient.getCachedDAGStatus();
+    Assert.assertNotNull(cachedDagStatus);
+    Assert.assertTrue(dagStatus == cachedDagStatus);
+
+    // When AM proxy throws an exception, the cachedDAGStatus should be returned
+    dagClient.resetCounters();
+    dagClientRpc.resetCountesr();
+    dagClientRpc.injectAMFault(new IOException("injected Fault"));
+    dagStatus = dagClient.getDAGStatus(EnumSet.noneOf(StatusGetOpts.class));
+    // get the Status from the cache
+    assertEquals(0, dagClient.numGetStatusViaRmInvocations);
+    // Directly from AM - previous request cached, so single invocation only.
+    assertEquals(1, dagClientRpc.numGetStatusViaAmInvocations);
+    assertEquals(DAGStatus.State.SUCCEEDED, dagStatus.getState());
+    Assert.assertTrue(dagStatus == cachedDagStatus);
+
+    // test that RM is invoked when the cacheExpires and the AM fails.
+    dagClientRpc.setAMProxy(createMockProxy(DAGStatusStateProto.DAG_SUCCEEDED, 1000l));
+    dagClientRpc.injectAMFault(new IOException("injected AM Fault"));
+    dagClient.resetCounters();
+    dagClientRpc.resetCountesr();

Review comment:
       could you please fix this typo?
   (I know it's not introduced with this patch, but it's worth fixing as you already working on this area)




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] abstractdog commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
abstractdog commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r748857804



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/client/DAGClientImpl.java
##########
@@ -221,13 +245,14 @@ protected DAGStatus getDAGStatusInternal(@Nullable Set<StatusGetOpts> statusOpti
       final DAGStatus dagStatus = getDAGStatusViaAM(statusOptions, timeout);
 
       if (!dagCompleted) {
-        if (dagStatus != null) {
-          cachedDagStatus = dagStatus;
+        if (dagStatus != null) { // update the cached DAGStatus
+          cachedDAGStatusRef.setValue(dagStatus);
           return dagStatus;
         }
-        if (cachedDagStatus != null) {
+        DAGStatus cachedDAG = cachedDAGStatusRef.getValue();
+        if (cachedDAG != null) {
           // could not get from AM (not reachable/ was killed). return cached status.
-          return cachedDagStatus;
+          return cachedDAG;

Review comment:
       am I right to assume this is the codepath where the original issue happened? could you please clarify how can we indefinitely stuck here?
   I mean, we can only hit this part if getDAGStatusViaAM returns null but dagCompleted is not true, so when we hit this again and again in getDAGStatusViaAM :
   ```
       } catch (TezException e) {
         // can be either due to a n/w issue of due to AM completed.
       } catch (IOException e) {
         // can be either due to a n/w issue of due to AM completed.
       }
   ```
   also getApplicationReportInternal keeps returning null in checkAndSetDagCompletionStatus
   
   was it the case for you? 
   if so, does it make sense to put at least debug level log messages to the silent catch branches?




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] abstractdog merged pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
abstractdog merged pull request #161:
URL: https://github.com/apache/tez/pull/161


   


-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] tez-yetus commented on pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
tez-yetus commented on pull request #161:
URL: https://github.com/apache/tez/pull/161#issuecomment-968090170


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  13m 13s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  master passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  compile  |   0m 34s |  master passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  checkstyle  |   1m  4s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  master passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +0 :ok: |  spotbugs  |   1m 34s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m 31s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 24s |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javac  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  javac  |   0m 20s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 15s |  tez-api: The patch generated 16 new + 103 unchanged - 0 fixed = 119 total (was 103)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  findbugs  |   1m  3s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m  7s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  25m 25s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/tez/pull/161 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 8779a8e83e2d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / f39a51e58 |
   | Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/1/artifact/out/diff-checkstyle-tez-api.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/1/testReport/ |
   | Max. process+thread count | 263 (vs. ulimit of 5500) |
   | modules | C: tez-api U: tez-api |
   | Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/1/console |
   | versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically 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.

To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] tez-yetus commented on pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
tez-yetus commented on pull request #161:
URL: https://github.com/apache/tez/pull/161#issuecomment-969207336


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  13m 14s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 39s |  master passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  compile  |   0m 37s |  master passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  checkstyle  |   1m  6s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  master passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +0 :ok: |  spotbugs  |   1m 35s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m 32s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 24s |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javac  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  javac  |   0m 20s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 14s |  tez-api: The patch generated 2 new + 103 unchanged - 0 fixed = 105 total (was 103)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  findbugs  |   1m  3s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m  4s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  25m 31s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/tez/pull/161 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux d275826a7234 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / f39a51e58 |
   | Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/2/artifact/out/diff-checkstyle-tez-api.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/2/testReport/ |
   | Max. process+thread count | 262 (vs. ulimit of 5500) |
   | modules | C: tez-api U: tez-api |
   | Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/2/console |
   | versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically 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.

To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] jteagles commented on a change in pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
jteagles commented on a change in pull request #161:
URL: https://github.com/apache/tez/pull/161#discussion_r749686283



##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -1975,6 +1975,18 @@ public static void validateProperty(String property, Scope usedScope) {
       TEZ_PREFIX + "test.minicluster.app.wait.on.shutdown.secs";
   public static final long TEZ_TEST_MINI_CLUSTER_APP_WAIT_ON_SHUTDOWN_SECS_DEFAULT = 30;
 
+  /**
+   * Long value
+   * Status Cache timeout window in minutes for the DAGClient.
+   */
+  @Private
+  @ConfigurationScope(Scope.CLIENT)
+  @ConfigurationProperty(type="long")
+  public static final String TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES = TEZ_PREFIX
+      + "client.dag.status.cache.timeout-minutes";
+  // Default timeout is 5 minutes.
+  public static final long TEZ_CLIENT_DAG_STATUS_CACHE_TIMEOUT_MINUTES_DEFAULT = 5;

Review comment:
       I'm wondering if we actually would prefer seconds. I can see use cases that would benefit from a lower time than 1 minute.




-- 
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: issues-unsubscribe@tez.apache.org

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



[GitHub] [tez] tez-yetus commented on pull request #161: TEZ-4349. DAGClient gets stuck with invalid cached DAGStatus

Posted by GitBox <gi...@apache.org>.
tez-yetus commented on pull request #161:
URL: https://github.com/apache/tez/pull/161#issuecomment-970565221


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 39s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  14m 44s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 41s |  master passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  compile  |   0m 37s |  master passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  checkstyle  |   1m  5s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  master passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  master passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +0 :ok: |  spotbugs  |   1m 58s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m 56s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 37s |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javac  |   0m 38s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  javac  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 19s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   0m 32s |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  findbugs  |   1m 28s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 21s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  29m 47s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/tez/pull/161 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 7dbdbb5f65d4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / f39a51e58 |
   | Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   |  Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/3/testReport/ |
   | Max. process+thread count | 264 (vs. ulimit of 5500) |
   | modules | C: tez-api U: tez-api |
   | Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-161/3/console |
   | versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically 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.

To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org

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