You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tez.apache.org by "mudit-97 (via GitHub)" <gi...@apache.org> on 2023/02/13 04:12:01 UTC
[GitHub] [tez] mudit-97 opened a new pull request, #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
mudit-97 opened a new pull request, #266:
URL: https://github.com/apache/tez/pull/266
Raised JIRA for the same: https://issues.apache.org/jira/browse/TEZ-4474
--
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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1109317629
##########
tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java:
##########
@@ -1828,6 +1828,18 @@ public TezConfiguration(boolean loadDefaults) {
TEZ_PREFIX + "dag.recovery.enabled";
public static final boolean DAG_RECOVERY_ENABLED_DEFAULT = true;
+
+ /**
+ * Boolean value. When set, this enables AM to fail when DAG recovery is enabled and
+ * restarted app master did not find anything to recover
+ * Expert level setting.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty(type="boolean")
+ public static final String TEZ_AM_FAILURE_ON_MISSING_RECOVERY =
Review Comment:
ack, done
--
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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1427369262
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 9m 38s | 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 _ |
| +0 :ok: | mvndep | 5m 56s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 11m 7s | master passed |
| +1 :green_heart: | compile | 1m 33s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | compile | 1m 18s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 1m 13s | master passed |
| +1 :green_heart: | javadoc | 1m 30s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 1m 15s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 1m 38s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 21s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 0s | the patch passed |
| +1 :green_heart: | compile | 1m 10s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javac | 1m 10s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| -0 :warning: | checkstyle | 0m 13s | tez-api: The patch generated 1 new + 35 unchanged - 0 fixed = 36 total (was 35) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | findbugs | 2m 54s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 17s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 16s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 54m 26s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 5827c5a397e1 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 39e5a8e71 |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/2/artifact/out/diff-checkstyle-tez-api.txt |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/2/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/2/testReport/ |
| Max. process+thread count | 395 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/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] tez-yetus commented on pull request #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1427393043
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 29m 13s | 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 _ |
| +0 :ok: | mvndep | 5m 45s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 42s | master passed |
| +1 :green_heart: | compile | 1m 14s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | compile | 1m 6s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 1m 10s | master passed |
| +1 :green_heart: | javadoc | 1m 16s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 1m 6s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 1m 17s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 39s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 49s | the patch passed |
| +1 :green_heart: | compile | 0m 54s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javac | 0m 54s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| -0 :warning: | checkstyle | 0m 11s | tez-api: The patch generated 1 new + 35 unchanged - 0 fixed = 36 total (was 35) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | findbugs | 2m 19s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 12s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 2s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 19s | The patch does not generate ASF License warnings. |
| | | 69m 50s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux a422366dd230 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 39e5a8e71 |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/3/artifact/out/diff-checkstyle-tez-api.txt |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/3/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/3/testReport/ |
| Max. process+thread count | 405 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/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
[GitHub] [tez] tez-yetus commented on pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1441557119
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 _ |
| +0 :ok: | mvndep | 5m 47s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 35s | master passed |
| +1 :green_heart: | compile | 1m 14s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 4s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 5s | master passed |
| +1 :green_heart: | javadoc | 1m 15s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 7s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 18s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 38s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 53s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 0m 53s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 30s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 8s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 4s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 40m 56s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/16/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux b1b691cfb22f 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/16/testReport/ |
| Max. process+thread count | 389 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/16/console |
| versions | git=2.34.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] abstractdog commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1113971444
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -1919,6 +1919,13 @@ private DAGRecoveryData recoverDAG() throws IOException, TezException {
RecoveryParser recoveryParser = new RecoveryParser(
this, recoveryFS, recoveryDataDir, appAttemptID.getAttemptId());
DAGRecoveryData recoveredDAGData = recoveryParser.parseRecoveryData();
+
+ if(Objects.isNull(recoveredDAGData) && amConf.getBoolean(
+ TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA,
+ TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA_DEFAULT)) {
+ throw new IOException("Found nothing to recover in currentAttemptId= "
Review Comment:
a couple of comments here:
1. I think it would make sense to extract the whole parse logic to another method (from the instantiation of the recoveryParser) to prevent recoverDAG grow beyond
2. regarding the actual fix: Objects.isNull tells almost nothing about the scenario that has been discussed in jira, so you should describe it in a code comment here, touching the most important points (AM shutdown prematurely, so as the recovery service, recovery stream is not closed, there is no valid recovery data, recovery parser returns null instead of failing, etc.), also, describe the assumption here: callers of this method should catch this exception and make the AM finish with state of DAGAppMasterState.ERROR (this is currently true, but after a refactor, it can simply collide, as we don't do the actual AM failure logic here)
3. what about having the recoveryDataDir path in the exception message to help the log reader investigate further?
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
Review Comment:
this looks like an IDE generated method, please use the correct variable names (even if they are not used), e.g. here:
```
public FSDataOutputStream create(Path f, FsPermission permission, boolean overwrite, int bufferSize,
short replication, long blockSize, Progressable progress) throws IOException {
```
please check other methods too
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
+ int i, short i1, long l, Progressable progressable)
+ throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream append(Path path, int i, Progressable progressable) throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean rename(Path path, Path path1) throws IOException {
+ return false;
+ }
+
+ @Override
+ public boolean delete(Path path, boolean b) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path path) throws FileNotFoundException, IOException {
+ return new FileStatus[0];
+ }
+
+ @Override
+ public void setWorkingDirectory(Path path) {
+
+ }
+
+ @Override
+ public Path getWorkingDirectory() {
+ return null;
+ }
+
+ @Override
+ public boolean mkdirs(Path path, FsPermission fsPermission) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path path) throws IOException {
+ return null;
+ }
+ });
+ when(spyRecoveryFs.exists(any())).thenReturn(false);
+
+ DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true);
+ dam.init(conf);
+ dam.start();
+
+ Field field = DAGAppMasterForTest.class.getSuperclass().getDeclaredField("recoveryFS");
+ field.setAccessible(true);
+ field.set(dam, spyRecoveryFs);
+
+ verify(dam.mockScheduler).setShouldUnregisterFlag();
+ verify(dam.mockShutdown).shutdown();
Review Comment:
we're testing the new feature here, I'm missing something here:
1. it's not clear for the first sight which part of the spy fs caused this expected behavior? (returning with ERROR)
2. is there a chance to extend this method to reflect what happens in case of TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA=false?
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
Review Comment:
is this spy reusable in other testing methods in this class? if so, refactor it to class field, if it's not, make it obvious why is this special (with variable name and/or comment on implemented methods)
--
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 #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1441245715
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | 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 _ |
| +0 :ok: | mvndep | 5m 53s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 52s | master passed |
| +1 :green_heart: | compile | 1m 24s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 14s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | javadoc | 1m 24s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 12s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 31s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 7s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 59s | the patch passed |
| +1 :green_heart: | compile | 1m 8s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 1m 8s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 36s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 56s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 52s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 28s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 12s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 6s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 43m 47s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/13/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux c7759cbe4fdf 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/13/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/13/testReport/ |
| Max. process+thread count | 382 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/13/console |
| versions | git=2.34.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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1115239955
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
Review Comment:
made it common for the class
--
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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1115240303
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -1919,6 +1919,13 @@ private DAGRecoveryData recoverDAG() throws IOException, TezException {
RecoveryParser recoveryParser = new RecoveryParser(
this, recoveryFS, recoveryDataDir, appAttemptID.getAttemptId());
DAGRecoveryData recoveredDAGData = recoveryParser.parseRecoveryData();
+
+ if(Objects.isNull(recoveredDAGData) && amConf.getBoolean(
+ TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA,
+ TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA_DEFAULT)) {
+ throw new IOException("Found nothing to recover in currentAttemptId= "
Review Comment:
Did all 3 changes here:
1. Carved out a new function
2. Added a comment
3. Added recoveryDataDir in message
--
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] mudit-97 commented on pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1443384505
@abstractdog , the tests are completed, can you please approve and merge
--
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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1433723234
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | 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 _ |
| +0 :ok: | mvndep | 5m 49s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 43s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 6s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | javadoc | 1m 14s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 6s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 16s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 37s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | compile | 0m 45s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 45s | the patch passed |
| -0 :warning: | checkstyle | 0m 19s | tez-dag: The patch generated 5 new + 54 unchanged - 0 fixed = 59 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 8s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 2s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 41m 2s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/8/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 4887e80978c0 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/8/artifact/out/diff-checkstyle-tez-dag.txt |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/8/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/8/testReport/ |
| Max. process+thread count | 359 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/8/console |
| versions | git=2.34.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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1427512621
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | 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 _ |
| +0 :ok: | mvndep | 5m 45s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 48s | master passed |
| +1 :green_heart: | compile | 1m 15s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | compile | 1m 5s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 1m 5s | master passed |
| +1 :green_heart: | javadoc | 1m 15s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 1m 7s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 1m 18s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 45s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| -0 :warning: | checkstyle | 0m 12s | tez-api: The patch generated 1 new + 35 unchanged - 0 fixed = 36 total (was 35) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 47s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | findbugs | 2m 19s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 9s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 3s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 41m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 477913eea6f1 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 39e5a8e71 |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/4/artifact/out/diff-checkstyle-tez-api.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/4/testReport/ |
| Max. process+thread count | 391 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/4/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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1434060701
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | 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 _ |
| +0 :ok: | mvndep | 5m 55s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 11m 2s | master passed |
| +1 :green_heart: | compile | 1m 14s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 3s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 6s | master passed |
| +1 :green_heart: | javadoc | 1m 16s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 5s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 16s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 37s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 47s | the patch passed |
| +1 :green_heart: | compile | 0m 54s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 0m 54s | the patch passed |
| +1 :green_heart: | compile | 0m 45s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 45s | the patch passed |
| -0 :warning: | checkstyle | 0m 19s | tez-dag: The patch generated 1 new + 54 unchanged - 0 fixed = 55 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 47s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 7s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 2s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 41m 23s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/10/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 288bea0d4d3b 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/10/artifact/out/diff-checkstyle-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/10/testReport/ |
| Max. process+thread count | 491 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/10/console |
| versions | git=2.34.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] abstractdog commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116686434
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
+ int i, short i1, long l, Progressable progressable)
+ throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream append(Path path, int i, Progressable progressable) throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean rename(Path path, Path path1) throws IOException {
+ return false;
+ }
+
+ @Override
+ public boolean delete(Path path, boolean b) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path path) throws FileNotFoundException, IOException {
+ return new FileStatus[0];
+ }
+
+ @Override
+ public void setWorkingDirectory(Path path) {
+
+ }
+
+ @Override
+ public Path getWorkingDirectory() {
+ return null;
+ }
+
+ @Override
+ public boolean mkdirs(Path path, FsPermission fsPermission) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path path) throws IOException {
+ return null;
+ }
+ });
+ when(spyRecoveryFs.exists(any())).thenReturn(false);
+
+ DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true);
+ dam.init(conf);
+ dam.start();
+
+ Field field = DAGAppMasterForTest.class.getSuperclass().getDeclaredField("recoveryFS");
+ field.setAccessible(true);
+ field.set(dam, spyRecoveryFs);
+
+ verify(dam.mockScheduler).setShouldUnregisterFlag();
+ verify(dam.mockShutdown).shutdown();
Review Comment:
thanks @mudit-97 , just a few questions:
1. I'm not 100% sure, but do we really need spy here? as far as I know we use spies when really need an instance created by us instead of a mocked one, would you consider trying to use a mock here, which might be much simpler?
2. if we end up using a spy here, please fix the method arguments' name...I know, this might look nitpicking, but even if we don't use them now, it's always a code smell to have args like "boolean b, int i, short i1, long l"
--
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 diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116700504
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -96,6 +105,7 @@ public class TestDAGAppMaster {
private static final String CLASS_SUFFIX = "_CLASS";
private static final File TEST_DIR = new File(System.getProperty("test.build.data"),
TestDAGAppMaster.class.getName()).getAbsoluteFile();
+ private final FileSystem mockFs = mock(FileSystem.class);
Review Comment:
as this is just a single line now, it doesn't have to be a field, you can have it in the methods
--
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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1434034937
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | 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 _ |
| +0 :ok: | mvndep | 5m 54s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 47s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 5s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 5s | master passed |
| +1 :green_heart: | javadoc | 1m 19s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 6s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 20s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 41s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| -0 :warning: | checkstyle | 0m 19s | tez-dag: The patch generated 1 new + 54 unchanged - 0 fixed = 55 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 45s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 47s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 9s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 1s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 19s | The patch does not generate ASF License warnings. |
| | | 41m 20s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 42f1ada93f10 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/9/artifact/out/diff-checkstyle-tez-dag.txt |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/9/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/9/testReport/ |
| Max. process+thread count | 483 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/9/console |
| versions | git=2.34.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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1433590003
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | 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 _ |
| +0 :ok: | mvndep | 6m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 39s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 4s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | javadoc | 1m 14s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 6s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 17s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 37s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 49s | the patch passed |
| +1 :green_heart: | compile | 0m 53s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 0m 53s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| -0 :warning: | checkstyle | 0m 19s | tez-dag: The patch generated 2 new + 54 unchanged - 0 fixed = 56 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 9s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 2s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 41m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 7f65429b117e 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/6/artifact/out/diff-checkstyle-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/6/testReport/ |
| Max. process+thread count | 393 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/6/console |
| versions | git=2.34.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 #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1441244205
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 _ |
| +0 :ok: | mvndep | 6m 10s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 47s | master passed |
| +1 :green_heart: | compile | 1m 18s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 7s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 4s | master passed |
| +1 :green_heart: | javadoc | 1m 28s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 16s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 29s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 58s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 58s | the patch passed |
| +1 :green_heart: | compile | 1m 6s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 1m 6s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 34s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 56s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 47s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 17s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 19s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 44m 16s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/12/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 6cc2e5aced21 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/12/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/12/testReport/ |
| Max. process+thread count | 488 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/12/console |
| versions | git=2.34.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 #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1443380040
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | 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 _ |
| +0 :ok: | mvndep | 7m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 38s | master passed |
| +1 :green_heart: | compile | 1m 15s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 7s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | javadoc | 1m 23s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 12s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 29s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 1s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 57s | the patch passed |
| +1 :green_heart: | compile | 1m 7s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 1m 7s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| -0 :warning: | checkstyle | 0m 20s | tez-dag: The patch generated 6 new + 54 unchanged - 0 fixed = 60 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 57s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 57s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 47s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 19s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 19s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 45m 24s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/17/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 95c06f27dbd3 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / db9eb1efc |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/17/artifact/out/diff-checkstyle-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/17/testReport/ |
| Max. process+thread count | 492 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/17/console |
| versions | git=2.34.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 #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1441500370
: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 _ |
| +0 :ok: | mvndep | 5m 49s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 44s | master passed |
| +1 :green_heart: | compile | 1m 22s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 10s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | javadoc | 1m 23s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 17s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 33s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 8s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 58s | the patch passed |
| +1 :green_heart: | compile | 1m 7s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 1m 7s | the patch passed |
| +1 :green_heart: | compile | 0m 54s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 54s | the patch passed |
| -0 :warning: | checkstyle | 0m 21s | tez-dag: The patch generated 1 new + 54 unchanged - 0 fixed = 55 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 57s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 50s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 12s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 13s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 19s | The patch does not generate ASF License warnings. |
| | | 44m 3s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/15/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 9edd41df4c52 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/15/artifact/out/diff-checkstyle-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/15/testReport/ |
| Max. process+thread count | 389 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/15/console |
| versions | git=2.34.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] abstractdog commented on pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1436036960
@mudit-97, @shameersss1: thanks for working on this so far, let me find some time next week to discover and review this scenario and patch
--
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] shameersss1 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "shameersss1 (via GitHub)" <gi...@apache.org>.
shameersss1 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1109306745
##########
tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java:
##########
@@ -1828,6 +1828,18 @@ public TezConfiguration(boolean loadDefaults) {
TEZ_PREFIX + "dag.recovery.enabled";
public static final boolean DAG_RECOVERY_ENABLED_DEFAULT = true;
+
+ /**
+ * Boolean value. When set, this enables AM to fail when DAG recovery is enabled and
+ * restarted app master did not find anything to recover
+ * Expert level setting.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty(type="boolean")
+ public static final String TEZ_AM_FAILURE_ON_MISSING_RECOVERY =
Review Comment:
nit: can we have a better config name? TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA
--
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 diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116705570
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -96,6 +105,7 @@ public class TestDAGAppMaster {
private static final String CLASS_SUFFIX = "_CLASS";
private static final File TEST_DIR = new File(System.getProperty("test.build.data"),
TestDAGAppMaster.class.getName()).getAbsoluteFile();
+ private final FileSystem mockFs = mock(FileSystem.class);
Review Comment:
thanks, approved
--
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 #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1443383585
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 40s | 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 _ |
| +0 :ok: | mvndep | 6m 6s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 41s | master passed |
| +1 :green_heart: | compile | 1m 20s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 13s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | javadoc | 1m 22s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 13s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 34s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 11s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 56s | the patch passed |
| +1 :green_heart: | compile | 1m 5s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 1m 5s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| -0 :warning: | checkstyle | 0m 22s | tez-dag: The patch generated 6 new + 54 unchanged - 0 fixed = 60 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 56s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 56s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 28s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 9s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 12s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 43m 48s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/18/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 6fc6576785ed 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / db9eb1efc |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/18/artifact/out/diff-checkstyle-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/18/testReport/ |
| Max. process+thread count | 391 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/18/console |
| versions | git=2.34.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] abstractdog commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1113971444
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -1919,6 +1919,13 @@ private DAGRecoveryData recoverDAG() throws IOException, TezException {
RecoveryParser recoveryParser = new RecoveryParser(
this, recoveryFS, recoveryDataDir, appAttemptID.getAttemptId());
DAGRecoveryData recoveredDAGData = recoveryParser.parseRecoveryData();
+
+ if(Objects.isNull(recoveredDAGData) && amConf.getBoolean(
+ TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA,
+ TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA_DEFAULT)) {
+ throw new IOException("Found nothing to recover in currentAttemptId= "
Review Comment:
a couple of comments here:
1. I think it would make sense to extract the whole parse logic to another method (from the instantiation of the recoveryParser) to prevent recoverDAG grow further
2. regarding the actual fix: Objects.isNull tells almost nothing about the scenario that has been discussed in jira, so you should describe it in a code comment here, touching the most important points (AM shutdown prematurely, so as the recovery service, recovery stream is not closed, there is no valid recovery data, recovery parser returns null instead of failing, etc.), also, describe the assumption here: callers of this method should catch this exception and make the AM finish with state of DAGAppMasterState.ERROR (this is currently true, but after a refactor, it can simply collide, as we don't do the actual AM failure logic here)
3. what about having the recoveryDataDir path in the exception message to help the log reader investigate further?
--
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] shameersss1 commented on pull request #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "shameersss1 (via GitHub)" <gi...@apache.org>.
shameersss1 commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1434117721
@abstractdog Could you please review the PR?
--
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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1434147389
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 _ |
| +0 :ok: | mvndep | 5m 44s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 32s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 5s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 10s | master passed |
| +1 :green_heart: | javadoc | 1m 17s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 7s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 20s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 44s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 53s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 0m 53s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 29s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 47s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 9s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 2s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 22s | The patch does not generate ASF License warnings. |
| | | 41m 5s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/11/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 4adcc760ff49 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/11/testReport/ |
| Max. process+thread count | 491 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/11/console |
| versions | git=2.34.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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1433661686
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 10s | 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 _ |
| +0 :ok: | mvndep | 6m 4s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 11m 20s | master passed |
| +1 :green_heart: | compile | 1m 15s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 6s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | javadoc | 1m 15s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 5s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 14s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 37s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 47s | the patch passed |
| +1 :green_heart: | compile | 0m 53s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 0m 53s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| -0 :warning: | checkstyle | 0m 19s | tez-dag: The patch generated 5 new + 54 unchanged - 0 fixed = 59 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 47s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 8s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 0s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 19s | The patch does not generate ASF License warnings. |
| | | 42m 30s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 9677ba84eb36 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/7/artifact/out/diff-checkstyle-tez-dag.txt |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/7/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/7/testReport/ |
| Max. process+thread count | 355 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/7/console |
| versions | git=2.34.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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116703351
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
+ int i, short i1, long l, Progressable progressable)
+ throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream append(Path path, int i, Progressable progressable) throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean rename(Path path, Path path1) throws IOException {
+ return false;
+ }
+
+ @Override
+ public boolean delete(Path path, boolean b) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path path) throws FileNotFoundException, IOException {
+ return new FileStatus[0];
+ }
+
+ @Override
+ public void setWorkingDirectory(Path path) {
+
+ }
+
+ @Override
+ public Path getWorkingDirectory() {
+ return null;
+ }
+
+ @Override
+ public boolean mkdirs(Path path, FsPermission fsPermission) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path path) throws IOException {
+ return null;
+ }
+ });
+ when(spyRecoveryFs.exists(any())).thenReturn(false);
+
+ DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true);
+ dam.init(conf);
+ dam.start();
+
+ Field field = DAGAppMasterForTest.class.getSuperclass().getDeclaredField("recoveryFS");
+ field.setAccessible(true);
+ field.set(dam, spyRecoveryFs);
+
+ verify(dam.mockScheduler).setShouldUnregisterFlag();
+ verify(dam.mockShutdown).shutdown();
Review Comment:
@abstractdog done that change also, please check
--
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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116703802
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -96,6 +105,7 @@ public class TestDAGAppMaster {
private static final String CLASS_SUFFIX = "_CLASS";
private static final File TEST_DIR = new File(System.getProperty("test.build.data"),
TestDAGAppMaster.class.getName()).getAbsoluteFile();
+ private final FileSystem mockFs = mock(FileSystem.class);
Review Comment:
done
--
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 #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1441498588
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | 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 _ |
| +0 :ok: | mvndep | 5m 59s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 37s | master passed |
| +1 :green_heart: | compile | 1m 16s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | compile | 1m 10s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | javadoc | 1m 16s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 1m 14s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +0 :ok: | spotbugs | 1m 34s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 6s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 58s | the patch passed |
| +1 :green_heart: | compile | 1m 8s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javac | 1m 8s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| -0 :warning: | checkstyle | 0m 21s | tez-dag: The patch generated 1 new + 54 unchanged - 0 fixed = 55 total (was 54) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| +1 :green_heart: | findbugs | 2m 54s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 21s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 12s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 43m 58s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/14/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux e8e211e99d5f 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / be994890d |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu222.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~22.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/14/artifact/out/diff-checkstyle-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/14/testReport/ |
| Max. process+thread count | 360 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/14/console |
| versions | git=2.34.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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1427369575
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 29m 26s | 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 _ |
| +0 :ok: | mvndep | 5m 56s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 11m 6s | master passed |
| +1 :green_heart: | compile | 1m 32s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | compile | 1m 18s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 1m 14s | master passed |
| +1 :green_heart: | javadoc | 1m 28s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 1m 17s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 1m 35s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 16s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 59s | the patch passed |
| +1 :green_heart: | compile | 1m 10s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javac | 1m 10s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| -0 :warning: | checkstyle | 0m 12s | tez-api: The patch generated 1 new + 35 unchanged - 0 fixed = 36 total (was 35) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 56s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | findbugs | 2m 56s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 17s | tez-api in the patch passed. |
| -1 :x: | unit | 5m 44s | tez-dag in the patch failed. |
| +1 :green_heart: | asflicense | 0m 22s | The patch does not generate ASF License warnings. |
| | | 74m 38s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | tez.dag.app.TestDAGAppMaster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 1051e141b65d 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 39e5a8e71 |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/1/artifact/out/diff-checkstyle-tez-api.txt |
| unit | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/1/artifact/out/patch-unit-tez-dag.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/1/testReport/ |
| Max. process+thread count | 460 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/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 #266: TEZ-4474: Added config to fail the DAG status when shutdown called with no current running DAGs
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #266:
URL: https://github.com/apache/tez/pull/266#issuecomment-1427571486
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | 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 _ |
| +0 :ok: | mvndep | 5m 51s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 10m 35s | master passed |
| +1 :green_heart: | compile | 1m 19s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | compile | 1m 8s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | javadoc | 1m 19s | master passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 1m 9s | master passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 1m 23s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 50s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 53s | the patch passed |
| +1 :green_heart: | compile | 1m 0s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javac | 1m 0s | the patch passed |
| +1 :green_heart: | compile | 0m 50s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 0m 50s | the patch passed |
| -0 :warning: | checkstyle | 0m 11s | tez-api: The patch generated 1 new + 35 unchanged - 0 fixed = 36 total (was 35) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 50s | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javadoc | 0m 49s | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | findbugs | 2m 30s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 13s | tez-api in the patch passed. |
| +1 :green_heart: | unit | 5m 10s | tez-dag in the patch passed. |
| +1 :green_heart: | asflicense | 0m 22s | The patch does not generate ASF License warnings. |
| | | 42m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/266 |
| JIRA Issue | TEZ-4474 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 48c3047c4574 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 39e5a8e71 |
| Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/5/artifact/out/diff-checkstyle-tez-api.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/5/testReport/ |
| Max. process+thread count | 490 (vs. ulimit of 5500) |
| modules | C: tez-api tez-dag U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-266/5/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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1115423927
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
+ int i, short i1, long l, Progressable progressable)
+ throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream append(Path path, int i, Progressable progressable) throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean rename(Path path, Path path1) throws IOException {
+ return false;
+ }
+
+ @Override
+ public boolean delete(Path path, boolean b) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path path) throws FileNotFoundException, IOException {
+ return new FileStatus[0];
+ }
+
+ @Override
+ public void setWorkingDirectory(Path path) {
+
+ }
+
+ @Override
+ public Path getWorkingDirectory() {
+ return null;
+ }
+
+ @Override
+ public boolean mkdirs(Path path, FsPermission fsPermission) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path path) throws IOException {
+ return null;
+ }
+ });
+ when(spyRecoveryFs.exists(any())).thenReturn(false);
+
+ DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true);
+ dam.init(conf);
+ dam.start();
+
+ Field field = DAGAppMasterForTest.class.getSuperclass().getDeclaredField("recoveryFS");
+ field.setAccessible(true);
+ field.set(dam, spyRecoveryFs);
+
+ verify(dam.mockScheduler).setShouldUnregisterFlag();
+ verify(dam.mockShutdown).shutdown();
Review Comment:
@abstractdog , enhanced the test case
1. For the spy, now I am checking the exact number of invocations + I am capturing the values in each invocation to confirm this happened during recovery flow only and that too during summary file fetch
2. I created a separate test case to capture that scenario also when TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA is false
--
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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1115428110
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
Review Comment:
@abstractdog , I tried to create a spy filesystem object, FileSystem was an abstract hadoop class and it had the similar variable names so I kept them same, these I just created for placeholder because I needed to create an instance of Filesystem and kept in parity with main class, if needed I will replace these with some other names
--
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 #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog merged PR #266:
URL: https://github.com/apache/tez/pull/266
--
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] mudit-97 commented on a diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "mudit-97 (via GitHub)" <gi...@apache.org>.
mudit-97 commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116698666
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
+ int i, short i1, long l, Progressable progressable)
+ throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream append(Path path, int i, Progressable progressable) throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean rename(Path path, Path path1) throws IOException {
+ return false;
+ }
+
+ @Override
+ public boolean delete(Path path, boolean b) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path path) throws FileNotFoundException, IOException {
+ return new FileStatus[0];
+ }
+
+ @Override
+ public void setWorkingDirectory(Path path) {
+
+ }
+
+ @Override
+ public Path getWorkingDirectory() {
+ return null;
+ }
+
+ @Override
+ public boolean mkdirs(Path path, FsPermission fsPermission) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path path) throws IOException {
+ return null;
+ }
+ });
+ when(spyRecoveryFs.exists(any())).thenReturn(false);
+
+ DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true);
+ dam.init(conf);
+ dam.start();
+
+ Field field = DAGAppMasterForTest.class.getSuperclass().getDeclaredField("recoveryFS");
+ field.setAccessible(true);
+ field.set(dam, spyRecoveryFs);
+
+ verify(dam.mockScheduler).setShouldUnregisterFlag();
+ verify(dam.mockShutdown).shutdown();
Review Comment:
@abstractdog , converted it to mock and removed spy, please check
--
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 diff in pull request #266: TEZ-4474: Added config to fail the DAG status when recovery data is missing
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116700738
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified() throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA, true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission, boolean b,
+ int i, short i1, long l, Progressable progressable)
+ throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream append(Path path, int i, Progressable progressable) throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean rename(Path path, Path path1) throws IOException {
+ return false;
+ }
+
+ @Override
+ public boolean delete(Path path, boolean b) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path path) throws FileNotFoundException, IOException {
+ return new FileStatus[0];
+ }
+
+ @Override
+ public void setWorkingDirectory(Path path) {
+
+ }
+
+ @Override
+ public Path getWorkingDirectory() {
+ return null;
+ }
+
+ @Override
+ public boolean mkdirs(Path path, FsPermission fsPermission) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path path) throws IOException {
+ return null;
+ }
+ });
+ when(spyRecoveryFs.exists(any())).thenReturn(false);
+
+ DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true);
+ dam.init(conf);
+ dam.start();
+
+ Field field = DAGAppMasterForTest.class.getSuperclass().getDeclaredField("recoveryFS");
+ field.setAccessible(true);
+ field.set(dam, spyRecoveryFs);
+
+ verify(dam.mockScheduler).setShouldUnregisterFlag();
+ verify(dam.mockShutdown).shutdown();
Review Comment:
very cool, thanks, one more comment :)
--
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