Return-Path: X-Original-To: apmail-tez-commits-archive@minotaur.apache.org Delivered-To: apmail-tez-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id DE58C10907 for ; Mon, 24 Nov 2014 19:43:19 +0000 (UTC) Received: (qmail 85077 invoked by uid 500); 24 Nov 2014 19:43:19 -0000 Delivered-To: apmail-tez-commits-archive@tez.apache.org Received: (qmail 85047 invoked by uid 500); 24 Nov 2014 19:43:19 -0000 Mailing-List: contact commits-help@tez.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@tez.apache.org Delivered-To: mailing list commits@tez.apache.org Received: (qmail 85038 invoked by uid 99); 24 Nov 2014 19:43:19 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 24 Nov 2014 19:43:19 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 5D8E9A1752D; Mon, 24 Nov 2014 19:43:19 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: sseth@apache.org To: commits@tez.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: tez git commit: TEZ-1697. DAG submission fails if a local resource added is already part of tez.lib.uris. (sseth) Date: Mon, 24 Nov 2014 19:43:19 +0000 (UTC) Repository: tez Updated Branches: refs/heads/master 81eef37d9 -> 754124375 TEZ-1697. DAG submission fails if a local resource added is already part of tez.lib.uris. (sseth) Project: http://git-wip-us.apache.org/repos/asf/tez/repo Commit: http://git-wip-us.apache.org/repos/asf/tez/commit/75412437 Tree: http://git-wip-us.apache.org/repos/asf/tez/tree/75412437 Diff: http://git-wip-us.apache.org/repos/asf/tez/diff/75412437 Branch: refs/heads/master Commit: 75412437543a8d26c48dd2d1d64cffca97cd6b98 Parents: 81eef37 Author: Siddharth Seth Authored: Mon Nov 24 11:43:04 2014 -0800 Committer: Siddharth Seth Committed: Mon Nov 24 11:43:04 2014 -0800 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/tez/common/TezCommonUtils.java | 34 +++++++-- .../main/java/org/apache/tez/dag/api/DAG.java | 9 ++- .../java/org/apache/tez/dag/api/Vertex.java | 2 +- .../apache/tez/common/TestTezCommonUtils.java | 72 ++++++++++++++++++++ .../org/apache/tez/dag/api/TestDAGVerify.java | 19 ++++-- 6 files changed, 121 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tez/blob/75412437/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 011bbaf..0ac7605 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -24,6 +24,7 @@ ALL CHANGES: TEZ-1685. Remove YARNMaster which is never used. TEZ-1797. Create necessary content for Tez DOAP file. TEZ-1650. Please create a DOAP file for your TLP. + TEZ-1697. DAG submission fails if a local resource added is already part of tez.lib.uris Release 0.5.3: Unreleased http://git-wip-us.apache.org/repos/asf/tez/blob/75412437/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java b/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java index 345177a..c5d5ebc 100644 --- a/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java +++ b/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java @@ -307,14 +307,36 @@ public class TezCommonUtils { } public static void addAdditionalLocalResources(Map additionalLrs, - Map originalLRs) { + Map originalLRs, String logContext) { + // TODO TEZ-1798. Handle contents of Tez archives for duplicate LocalResource checks if (additionalLrs != null && !additionalLrs.isEmpty()) { - for (Map.Entry lr : additionalLrs.entrySet()) { - if (originalLRs.containsKey(lr.getKey())) { - throw new TezUncheckedException("Attempting to add duplicate resource: " + lr.getKey()); - } else { - originalLRs.put(lr.getKey(), lr.getValue()); + StringBuilder sb = new StringBuilder(); + for (Map.Entry lrEntry : additionalLrs.entrySet()) { + LocalResource originalLr = originalLRs.get(lrEntry.getKey()); + if (originalLr != null) { + LocalResource additionalLr = lrEntry.getValue(); + if (originalLr.getSize() != additionalLr.getSize()) { + throw new TezUncheckedException( + "Duplicate Resources found with different size for [" + logContext + "]: " + lrEntry.getKey() + + " : " + "[" + additionalLr.getResource() + "=" + additionalLr.getSize() + + "],[" + originalLr.getResource() + "=" + originalLr.getSize()); + } else { + if (originalLr.getResource().equals(additionalLr.getResource())) { + sb.append("[").append(lrEntry.getKey()).append(" : Duplicate]"); + } else { + sb.append("[").append(lrEntry.getKey()).append(" : DuplicateDifferentPath]"); + } + } } + // The LR either does not exist, or is an 'equivalent' dupe. + // Prefer the tez specified LR instead of the equivalent user specified LR for container reuse matching + originalLRs.put(lrEntry.getKey(), lrEntry.getValue()); + } + String logString = sb.toString(); + if (!logString.isEmpty()) { + LOG.warn("Found Resources Duplication in " + logContext + " after including resources from " + + TezConfiguration.TEZ_LIB_URIS + " and " + TezConfiguration.TEZ_AUX_URIS + ": " + + logString); } } } http://git-wip-us.apache.org/repos/asf/tez/blob/75412437/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java b/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java index acef4da..edaf800 100644 --- a/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java +++ b/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java @@ -113,7 +113,7 @@ public class DAG { */ public synchronized DAG addTaskLocalFiles(Map localFiles) { Preconditions.checkNotNull(localFiles); - TezCommonUtils.addAdditionalLocalResources(localFiles, commonTaskLocalFiles); + TezCommonUtils.addAdditionalLocalResources(localFiles, commonTaskLocalFiles, "DAG " + getName()); return this; } @@ -661,11 +661,14 @@ public class DAG { dagCredentials.addAll(dataSource.getCredentials()); } if (dataSource.getAdditionalLocalFiles() != null) { - TezCommonUtils.addAdditionalLocalResources(dataSource.getAdditionalLocalFiles(), vertexLRs); + TezCommonUtils + .addAdditionalLocalResources(dataSource.getAdditionalLocalFiles(), vertexLRs, + "Vertex " + vertex.getName()); } } if (tezJarResources != null) { - TezCommonUtils.addAdditionalLocalResources(tezJarResources, vertexLRs); + TezCommonUtils + .addAdditionalLocalResources(tezJarResources, vertexLRs, "Vertex " + vertex.getName()); } if (binaryConfig != null) { vertexLRs.put(TezConstants.TEZ_PB_BINARY_CONF_NAME, binaryConfig); http://git-wip-us.apache.org/repos/asf/tez/blob/75412437/tez-api/src/main/java/org/apache/tez/dag/api/Vertex.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/dag/api/Vertex.java b/tez-api/src/main/java/org/apache/tez/dag/api/Vertex.java index 04acdaf..8d59928 100644 --- a/tez-api/src/main/java/org/apache/tez/dag/api/Vertex.java +++ b/tez-api/src/main/java/org/apache/tez/dag/api/Vertex.java @@ -254,7 +254,7 @@ public class Vertex { */ public Vertex addTaskLocalFiles(Map localFiles) { if (localFiles != null) { - TezCommonUtils.addAdditionalLocalResources(localFiles, taskLocalResources); + TezCommonUtils.addAdditionalLocalResources(localFiles, taskLocalResources, "Vertex " + getName()); } return this; } http://git-wip-us.apache.org/repos/asf/tez/blob/75412437/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java ---------------------------------------------------------------------- diff --git a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java index 23d2dbb..e8c4f74 100644 --- a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java +++ b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java @@ -19,16 +19,23 @@ package org.apache.tez.common; import java.io.IOException; +import java.util.Map; +import com.google.common.collect.Maps; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdfs.MiniDFSCluster; +import org.apache.hadoop.yarn.api.records.LocalResource; +import org.apache.hadoop.yarn.api.records.LocalResourceType; +import org.apache.hadoop.yarn.api.records.LocalResourceVisibility; +import org.apache.hadoop.yarn.api.records.URL; import org.apache.tez.client.TestTezClientUtils; import org.apache.tez.dag.api.TezConfiguration; import org.apache.tez.dag.api.TezConstants; +import org.apache.tez.dag.api.TezUncheckedException; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -242,4 +249,69 @@ public class TestTezCommonUtils { Assert.assertArrayEquals(expectedTokens, tokens); } + + @Test(timeout = 5000) + public void testAddAdditionalLocalResources() { + String lrName = "LR"; + Map originalLrs; + originalLrs= Maps.newHashMap(); + originalLrs.put(lrName, LocalResource.newInstance( + URL.newInstance("file", "localhost", 0, "/test"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 1)); + + + Map additionalLrs; + + // Same path, same size. + originalLrs= Maps.newHashMap(); + originalLrs.put(lrName, LocalResource.newInstance( + URL.newInstance("file", "localhost", 0, "/test"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 1)); + additionalLrs = Maps.newHashMap(); + additionalLrs.put(lrName, LocalResource.newInstance(URL.newInstance("file", "localhost", 0, "/test"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 1)); + TezCommonUtils.addAdditionalLocalResources(additionalLrs, originalLrs, ""); + + // Same path, different size. + originalLrs= Maps.newHashMap(); + originalLrs.put(lrName, LocalResource.newInstance( + URL.newInstance("file", "localhost", 0, "/test"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 1)); + additionalLrs = Maps.newHashMap(); + additionalLrs.put(lrName, LocalResource.newInstance(URL.newInstance("file", "localhost", 0, "/test"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 100, 1)); + try { + TezCommonUtils.addAdditionalLocalResources(additionalLrs, originalLrs, ""); + Assert.fail("Duplicate LRs with different sizes expected to fail"); + } catch (TezUncheckedException e) { + Assert.assertTrue(e.getMessage().contains("Duplicate Resources found with different size")); + } + + // Different path, same size, diff timestamp + originalLrs= Maps.newHashMap(); + originalLrs.put(lrName, LocalResource.newInstance( + URL.newInstance("file", "localhost", 0, "/test"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 1)); + additionalLrs = Maps.newHashMap(); + additionalLrs.put(lrName, LocalResource.newInstance(URL.newInstance("file", "localhost", 0, "/test2"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 100)); + TezCommonUtils.addAdditionalLocalResources(additionalLrs, originalLrs, ""); + + // Different path, different size + originalLrs= Maps.newHashMap(); + originalLrs.put(lrName, LocalResource.newInstance( + URL.newInstance("file", "localhost", 0, "/test"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 1)); + additionalLrs = Maps.newHashMap(); + additionalLrs.put(lrName, LocalResource.newInstance(URL.newInstance("file", "localhost", 0, "/test2"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 100, 1)); + try { + TezCommonUtils.addAdditionalLocalResources(additionalLrs, originalLrs, ""); + Assert.fail("Duplicate LRs with different sizes expected to fail"); + } catch (TezUncheckedException e) { + Assert.assertTrue(e.getMessage().contains("Duplicate Resources found with different size")); + } + + } + } http://git-wip-us.apache.org/repos/asf/tez/blob/75412437/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGVerify.java ---------------------------------------------------------------------- diff --git a/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGVerify.java b/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGVerify.java index 0697584..7edc7e5 100644 --- a/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGVerify.java +++ b/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGVerify.java @@ -956,33 +956,40 @@ public class TestDAGVerify { String lrName1 = "LR1"; lrs.put(lrName1, LocalResource.newInstance(URL.newInstance("file", "localhost", 0, "/test"), LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 1, 1)); + + // Same lr, different size + Map lrs2 = Maps.newHashMap(); + lrs2.put(lrName1, LocalResource.newInstance(URL.newInstance("file", "localhost", 0, "/test2"), + LocalResourceType.FILE, LocalResourceVisibility.PUBLIC, 100, 1)); + v1.addTaskLocalFiles(lrs); + // Allowed since the LR is the same. try { - v1.addTaskLocalFiles(lrs); + v1.addTaskLocalFiles(lrs2); Assert.fail(); } catch (TezUncheckedException e) { - Assert.assertTrue(e.getMessage().contains("Attempting to add duplicate resource")); + Assert.assertTrue(e.getMessage().contains("Duplicate Resources found with different size")); } DataSourceDescriptor ds = DataSourceDescriptor.create(InputDescriptor.create("I.class"), - null, -1, null, null, lrs); + null, -1, null, null, lrs2); v1.addDataSource("i1", ds); DAG dag = DAG.create("testDag"); dag.addVertex(v1); dag.addTaskLocalFiles(lrs); try { - dag.addTaskLocalFiles(lrs); + dag.addTaskLocalFiles(lrs2); Assert.fail(); } catch (TezUncheckedException e) { - Assert.assertTrue(e.getMessage().contains("Attempting to add duplicate resource")); + Assert.assertTrue(e.getMessage().contains("Duplicate Resources found with different size")); } try { // data source will add duplicate common files to vertex dag.createDag(new TezConfiguration(), null, null, null, true); Assert.fail(); } catch (TezUncheckedException e) { - Assert.assertTrue(e.getMessage().contains("Attempting to add duplicate resource")); + Assert.assertTrue(e.getMessage().contains("Duplicate Resources found with different size")); } }