You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tez.apache.org by ss...@apache.org on 2015/07/15 02:25:42 UTC
[09/50] [abbrv] tez git commit: TEZ-2542. TezDAGID fromString array
length check. (Vasanth kumar RJ via hitesh)
TEZ-2542. TezDAGID fromString array length check. (Vasanth kumar RJ via hitesh)
Project: http://git-wip-us.apache.org/repos/asf/tez/repo
Commit: http://git-wip-us.apache.org/repos/asf/tez/commit/2073351c
Tree: http://git-wip-us.apache.org/repos/asf/tez/tree/2073351c
Diff: http://git-wip-us.apache.org/repos/asf/tez/diff/2073351c
Branch: refs/heads/TEZ-2003
Commit: 2073351ca25bb715d910915b333648165d7c9ab6
Parents: 4a88ee1
Author: Hitesh Shah <hi...@apache.org>
Authored: Tue Jun 30 20:12:04 2015 -0700
Committer: Hitesh Shah <hi...@apache.org>
Committed: Tue Jun 30 20:12:04 2015 -0700
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../java/org/apache/tez/dag/api/TestDAG.java | 1 +
.../org/apache/tez/dag/records/TezDAGID.java | 27 +++++++++-----
.../org/apache/tez/dag/records/TestTezIds.java | 39 ++++++++++++++++++++
.../tez/dag/api/client/DAGClientHandler.java | 8 ++--
.../org/apache/tez/dag/app/RecoveryParser.java | 8 ++--
.../org/apache/tez/history/ATSImportTool.java | 5 ++-
7 files changed, 72 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tez/blob/2073351c/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 73e1497..85ff94a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -11,6 +11,7 @@ INCOMPATIBLE CHANGES
TEZ-2468. Change the minimum Java version to Java 7.
ALL CHANGES:
+ TEZ-2542. TezDAGID fromString array length check.
TEZ-2296. Add option to print counters for tez-examples.
TEZ-2570. Fix license header issue for eps image files.
TEZ-2378. In case Fetcher (unordered) fails to do local fetch, log in debug mode to reduce log size.
http://git-wip-us.apache.org/repos/asf/tez/blob/2073351c/tez-api/src/test/java/org/apache/tez/dag/api/TestDAG.java
----------------------------------------------------------------------
diff --git a/tez-api/src/test/java/org/apache/tez/dag/api/TestDAG.java b/tez-api/src/test/java/org/apache/tez/dag/api/TestDAG.java
index 0a66405..3fe17df 100644
--- a/tez-api/src/test/java/org/apache/tez/dag/api/TestDAG.java
+++ b/tez-api/src/test/java/org/apache/tez/dag/api/TestDAG.java
@@ -307,4 +307,5 @@ public class TestDAG {
Assert.assertEquals("Duplicated output:output_1, vertexName=v1", e.getMessage());
}
}
+
}
http://git-wip-us.apache.org/repos/asf/tez/blob/2073351c/tez-common/src/main/java/org/apache/tez/dag/records/TezDAGID.java
----------------------------------------------------------------------
diff --git a/tez-common/src/main/java/org/apache/tez/dag/records/TezDAGID.java b/tez-common/src/main/java/org/apache/tez/dag/records/TezDAGID.java
index 27cc751..3828890 100644
--- a/tez-common/src/main/java/org/apache/tez/dag/records/TezDAGID.java
+++ b/tez-common/src/main/java/org/apache/tez/dag/records/TezDAGID.java
@@ -177,17 +177,26 @@ public class TezDAGID extends TezID {
}
public static TezDAGID fromString(String dagId) {
+ int id = 0;
+ int appId = 0;
+ String[] split = dagId.split("_");
+ if (split.length != 4 || !dagId.startsWith(DAG + "_")) {
+ throw new IllegalArgumentException("Invalid DAG Id format : " + dagId);
+ }
+ String rmId = split[1];
+ try {
+ appId = Integer.parseInt(split[2]);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("Error while parsing App Id '"
+ + split[2] + "' of DAG Id : " + dagId);
+ }
try {
- String[] split = dagId.split("_");
- String rmId = split[1];
- int appId = tezAppIdFormat.get().parse(split[2]).intValue();
- int id;
- id = tezDagIdFormat.get().parse(split[3]).intValue();
- return TezDAGID.getInstance(rmId, appId, id);
- } catch (Exception e) {
- e.printStackTrace();
+ id = Integer.parseInt(split[3]);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("Error while parsing Id '" + split[3]
+ + "' of DAG Id : " + dagId);
}
- return null;
+ return TezDAGID.getInstance(rmId, appId, id);
}
/**
http://git-wip-us.apache.org/repos/asf/tez/blob/2073351c/tez-common/src/test/java/org/apache/tez/dag/records/TestTezIds.java
----------------------------------------------------------------------
diff --git a/tez-common/src/test/java/org/apache/tez/dag/records/TestTezIds.java b/tez-common/src/test/java/org/apache/tez/dag/records/TestTezIds.java
index 41d6d0f..04a2b35 100644
--- a/tez-common/src/test/java/org/apache/tez/dag/records/TestTezIds.java
+++ b/tez-common/src/test/java/org/apache/tez/dag/records/TestTezIds.java
@@ -108,4 +108,43 @@ public class TestTezIds {
verifyAttemptId(taIdStr, taId);
}
+ @Test(timeout=5000)
+ public void testInvalidDagIds() {
+ String dagIdStr = "aaa_111_1_1";
+ TezDAGID dagId;
+ try {
+ dagId = TezDAGID.fromString(dagIdStr);
+ Assert.fail("Expected failure for invalid dagId=" + dagIdStr);
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Invalid DAG Id format"));
+ }
+
+ dagIdStr = "dag_111_11";
+ try {
+ dagId = TezDAGID.fromString(dagIdStr);
+ Assert.fail("Expected failure for invalid dagId=" + dagIdStr);
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Invalid DAG Id format"));
+ }
+
+ dagIdStr = "dag_111_11_aa";
+ try {
+ dagId = TezDAGID.fromString(dagIdStr);
+ Assert.fail("Expected failure for invalid dagId=" + dagIdStr);
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Error while parsing"));
+ }
+
+ dagIdStr = "dag_111_aa_1";
+ try {
+ dagId = TezDAGID.fromString(dagIdStr);
+ Assert.fail("Expected failure for invalid dagId=" + dagIdStr);
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Error while parsing"));
+ }
+
+ }
+
+
+
}
http://git-wip-us.apache.org/repos/asf/tez/blob/2073351c/tez-dag/src/main/java/org/apache/tez/dag/api/client/DAGClientHandler.java
----------------------------------------------------------------------
diff --git a/tez-dag/src/main/java/org/apache/tez/dag/api/client/DAGClientHandler.java b/tez-dag/src/main/java/org/apache/tez/dag/api/client/DAGClientHandler.java
index e43f06f..13cd8f4 100644
--- a/tez-dag/src/main/java/org/apache/tez/dag/api/client/DAGClientHandler.java
+++ b/tez-dag/src/main/java/org/apache/tez/dag/api/client/DAGClientHandler.java
@@ -79,9 +79,11 @@ public class DAGClientHandler {
}
DAG getDAG(String dagIdStr) throws TezException {
- TezDAGID dagId = TezDAGID.fromString(dagIdStr);
- if (dagId == null) {
- throw new TezException("Bad dagId: " + dagIdStr);
+ TezDAGID dagId;
+ try {
+ dagId = TezDAGID.fromString(dagIdStr);
+ } catch (IllegalArgumentException e) {
+ throw new TezException("Bad dagId: " + dagIdStr, e);
}
DAG currentDAG = getCurrentDAG();
http://git-wip-us.apache.org/repos/asf/tez/blob/2073351c/tez-dag/src/main/java/org/apache/tez/dag/app/RecoveryParser.java
----------------------------------------------------------------------
diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/RecoveryParser.java b/tez-dag/src/main/java/org/apache/tez/dag/app/RecoveryParser.java
index 8d1005f..8922d6a 100644
--- a/tez-dag/src/main/java/org/apache/tez/dag/app/RecoveryParser.java
+++ b/tez-dag/src/main/java/org/apache/tez/dag/app/RecoveryParser.java
@@ -508,9 +508,11 @@ public class RecoveryParser {
+ ", timestamp=" + proto.getTimestamp()
+ ", event=" + eventType);
}
- TezDAGID dagId = TezDAGID.fromString(proto.getDagId());
- if (dagId == null) {
- throw new IOException("null dagId, summary records may be corrupted");
+ TezDAGID dagId;
+ try {
+ dagId = TezDAGID.fromString(proto.getDagId());
+ } catch (IllegalArgumentException e) {
+ throw new IOException("Invalid dagId, summary records may be corrupted", e);
}
if (dagCounter < dagId.getId()) {
dagCounter = dagId.getId();
http://git-wip-us.apache.org/repos/asf/tez/blob/2073351c/tez-plugins/tez-history-parser/src/main/java/org/apache/tez/history/ATSImportTool.java
----------------------------------------------------------------------
diff --git a/tez-plugins/tez-history-parser/src/main/java/org/apache/tez/history/ATSImportTool.java b/tez-plugins/tez-history-parser/src/main/java/org/apache/tez/history/ATSImportTool.java
index 87da000..0e53d27 100644
--- a/tez-plugins/tez-history-parser/src/main/java/org/apache/tez/history/ATSImportTool.java
+++ b/tez-plugins/tez-history-parser/src/main/java/org/apache/tez/history/ATSImportTool.java
@@ -124,12 +124,13 @@ public class ATSImportTool extends Configured implements Tool {
private final File downloadDir;
private final File zipFile;
private final Client httpClient;
+ private final TezDAGID tezDAGID;
public ATSImportTool(String baseUri, String dagId, File baseDownloadDir, int batchSize)
throws TezException {
Preconditions.checkArgument(!Strings.isNullOrEmpty(dagId), "dagId can not be null or empty");
Preconditions.checkArgument(baseDownloadDir != null, "downloadDir can not be null");
- Preconditions.checkArgument(TezDAGID.fromString(dagId) != null, "Not a valid DAG ID " + dagId);
+ tezDAGID = TezDAGID.fromString(dagId);
this.baseUri = baseUri;
this.batchSize = batchSize;
@@ -185,7 +186,7 @@ public class ATSImportTool extends Configured implements Tool {
JSONObject finalJson = new JSONObject();
//Download application details (TEZ_VERSION etc)
- String tezAppId = "tez_" + (TezDAGID.fromString(dagId)).getApplicationId().toString();
+ String tezAppId = "tez_" + tezDAGID.getApplicationId().toString();
String tezAppUrl = String.format("%s/%s/%s", baseUri, Constants.TEZ_APPLICATION, tezAppId);
JSONObject tezAppJson = getJsonRootEntity(tezAppUrl);
finalJson.put(Constants.APPLICATION, tezAppJson);