Repository: falcon Updated Branches: refs/heads/master 0da207404 -> b6d2c134b
FALCON-2073 Handle with NULL corner case Changes: 1. EntityGraph::getDependents should return empty list instead of NULL value if there is no dependent entities. Affected method include OozieWorkflowEngine::updateDependant and AbstractEntityManager::getDependencies. Also removed unnecessary existing NULL checks after the change. 2. In LogProvider::populateActionLogUrls, handle with the NULL case where there is no file status under the specified path. 3. FalconClient::getDependency should return empty list instead of NULL value if there is no dependent entities. Affected method include FalconEntityCLI::entityCommand. Extra minor changes in this patch: 4. A regular expression misusage in LogProvider::getActionName. To match special character ".", should use "[.]" instead of "." which will try to match any character. 5. Fix a rat check error in Falcon CLI due to the introduction of Spring shell commands. Author: yzheng-hortonworks <[email protected]> Reviewers: Peeyush <[email protected]>, Balu <[email protected]> Closes #223 from yzheng-hortonworks/FALCON-2073 and squashes the following commits: 526b0ad [yzheng-hortonworks] review by balu 33c5420 [yzheng-hortonworks] revision c69f9a4 [yzheng-hortonworks] FALCON-2073 Handle with NULL corner case Project: http://git-wip-us.apache.org/repos/asf/falcon/repo Commit: http://git-wip-us.apache.org/repos/asf/falcon/commit/b6d2c134 Tree: http://git-wip-us.apache.org/repos/asf/falcon/tree/b6d2c134 Diff: http://git-wip-us.apache.org/repos/asf/falcon/diff/b6d2c134 Branch: refs/heads/master Commit: b6d2c134b979ce1bb130916992553a54c5e911ed Parents: 0da2074 Author: yzheng-hortonworks <[email protected]> Authored: Fri Jul 15 21:16:03 2016 +0530 Committer: peeyush b <[email protected]> Committed: Fri Jul 15 21:16:03 2016 +0530 ---------------------------------------------------------------------- cli/pom.xml | 9 ++++ .../org/apache/falcon/client/FalconClient.java | 4 +- .../apache/falcon/entity/v0/EntityGraph.java | 6 +-- .../entity/v0/EntityIntegrityChecker.java | 4 -- .../falcon/entity/v0/EntityGraphTest.java | 8 ++-- .../org/apache/falcon/logging/LogProvider.java | 43 ++++++++++---------- .../falcon/resource/AbstractEntityManager.java | 20 ++++----- 7 files changed, 49 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/falcon/blob/b6d2c134/cli/pom.xml ---------------------------------------------------------------------- diff --git a/cli/pom.xml b/cli/pom.xml index a41e6d9..e0a8968 100644 --- a/cli/pom.xml +++ b/cli/pom.xml @@ -215,6 +215,15 @@ </execution> </executions> </plugin> + <plugin> + <groupId>org.apache.rat</groupId> + <artifactId>apache-rat-plugin</artifactId> + <configuration> + <excludes> + <exclude>falcon-cli-hist.log</exclude> + </excludes> + </configuration> + </plugin> </plugins> </build> </project> http://git-wip-us.apache.org/repos/asf/falcon/blob/b6d2c134/client/src/main/java/org/apache/falcon/client/FalconClient.java ---------------------------------------------------------------------- diff --git a/client/src/main/java/org/apache/falcon/client/FalconClient.java b/client/src/main/java/org/apache/falcon/client/FalconClient.java index 371828f..4716019 100644 --- a/client/src/main/java/org/apache/falcon/client/FalconClient.java +++ b/client/src/main/java/org/apache/falcon/client/FalconClient.java @@ -470,8 +470,8 @@ public class FalconClient extends AbstractFalconClient { checkIfSuccessful(clientResponse); EntityList result = clientResponse.getEntity(EntityList.class); - if (result == null || result.getElements() == null) { - return null; + if (result == null) { + return new EntityList(); } return result; } http://git-wip-us.apache.org/repos/asf/falcon/blob/b6d2c134/common/src/main/java/org/apache/falcon/entity/v0/EntityGraph.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/entity/v0/EntityGraph.java b/common/src/main/java/org/apache/falcon/entity/v0/EntityGraph.java index b5f6788..35b334e 100644 --- a/common/src/main/java/org/apache/falcon/entity/v0/EntityGraph.java +++ b/common/src/main/java/org/apache/falcon/entity/v0/EntityGraph.java @@ -56,9 +56,9 @@ public final class EntityGraph implements ConfigurationChangeListener { public Set<Entity> getDependents(Entity entity) throws FalconException { Node entityNode = new Node(entity.getEntityType(), entity.getName()); + Set<Entity> dependents = new HashSet<Entity>(); if (graph.containsKey(entityNode)) { ConfigurationStore store = ConfigurationStore.get(); - Set<Entity> dependents = new HashSet<Entity>(); for (Node node : graph.get(entityNode)) { Entity dependentEntity = store.get(node.type, node.name); if (dependentEntity != null) { @@ -67,10 +67,10 @@ public final class EntityGraph implements ConfigurationChangeListener { LOG.error("Dependent entity {} was not found in configuration store.", node); } } - return dependents; } else { - return null; + LOG.error("Entity node {} not found in entity graph.", entityNode); } + return dependents; } @Override http://git-wip-us.apache.org/repos/asf/falcon/blob/b6d2c134/common/src/main/java/org/apache/falcon/entity/v0/EntityIntegrityChecker.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/entity/v0/EntityIntegrityChecker.java b/common/src/main/java/org/apache/falcon/entity/v0/EntityIntegrityChecker.java index 4c7e913..a8b1854 100644 --- a/common/src/main/java/org/apache/falcon/entity/v0/EntityIntegrityChecker.java +++ b/common/src/main/java/org/apache/falcon/entity/v0/EntityIntegrityChecker.java @@ -35,10 +35,6 @@ public final class EntityIntegrityChecker { public static Pair<String, EntityType>[] referencedBy(Entity entity) throws FalconException { Set<Entity> deps = EntityGraph.get().getDependents(entity); - if (deps == null) { - return null; - } - switch (entity.getEntityType()) { case CLUSTER: return filter(deps, EntityType.FEED, EntityType.PROCESS); http://git-wip-us.apache.org/repos/asf/falcon/blob/b6d2c134/common/src/test/java/org/apache/falcon/entity/v0/EntityGraphTest.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/falcon/entity/v0/EntityGraphTest.java b/common/src/test/java/org/apache/falcon/entity/v0/EntityGraphTest.java index b41cc03..20f2871 100644 --- a/common/src/test/java/org/apache/falcon/entity/v0/EntityGraphTest.java +++ b/common/src/test/java/org/apache/falcon/entity/v0/EntityGraphTest.java @@ -273,10 +273,10 @@ public class EntityGraphTest extends AbstractTestBase { store.remove(EntityType.PROCESS, process.getName()); entities = graph.getDependents(cluster); - Assert.assertTrue(entities == null); + Assert.assertTrue(entities.isEmpty()); entities = graph.getDependents(process); - Assert.assertTrue(entities == null); + Assert.assertTrue(entities.isEmpty()); } @Test @@ -355,7 +355,7 @@ public class EntityGraphTest extends AbstractTestBase { Assert.assertTrue(entities.contains(f3)); entities = graph.getDependents(p2); - Assert.assertTrue(entities == null); + Assert.assertTrue(entities.isEmpty()); entities = graph.getDependents(f1); Assert.assertEquals(entities.size(), 2); @@ -363,7 +363,7 @@ public class EntityGraphTest extends AbstractTestBase { Assert.assertTrue(entities.contains(cluster)); entities = graph.getDependents(f2); - Assert.assertTrue(entities == null); + Assert.assertTrue(entities.isEmpty()); entities = graph.getDependents(f3); Assert.assertEquals(entities.size(), 2); http://git-wip-us.apache.org/repos/asf/falcon/blob/b6d2c134/oozie/src/main/java/org/apache/falcon/logging/LogProvider.java ---------------------------------------------------------------------- diff --git a/oozie/src/main/java/org/apache/falcon/logging/LogProvider.java b/oozie/src/main/java/org/apache/falcon/logging/LogProvider.java index ff7707a..b1a5df3 100644 --- a/oozie/src/main/java/org/apache/falcon/logging/LogProvider.java +++ b/oozie/src/main/java/org/apache/falcon/logging/LogProvider.java @@ -113,34 +113,35 @@ public final class LogProvider { + EntityUtil.fromUTCtoURIDate(instance.instance) + "/" + formattedRunId + "/*"); FileStatus[] actions = fs.globStatus(actionPaths); - InstanceAction[] instanceActions = new InstanceAction[actions.length - 1]; - instance.actions = instanceActions; - int i = 0; - for (FileStatus file : actions) { - Path filePath = file.getPath(); - String dfsBrowserUrl = getDFSbrowserUrl( - ClusterHelper.getStorageUrl(cluster), - EntityUtil.getLogPath(cluster, entity) + "/job-" - + EntityUtil.fromUTCtoURIDate(instance.instance) + "/" - + formattedRunId, file.getPath().getName()); - if (filePath.getName().equals("oozie.log")) { - instance.logFile = dfsBrowserUrl; - continue; + if (actions != null && actions.length > 0) { + InstanceAction[] instanceActions = new InstanceAction[actions.length - 1]; + instance.actions = instanceActions; + int i = 0; + for (FileStatus file : actions) { + Path filePath = file.getPath(); + String dfsBrowserUrl = getDFSbrowserUrl( + ClusterHelper.getStorageUrl(cluster), + EntityUtil.getLogPath(cluster, entity) + "/job-" + + EntityUtil.fromUTCtoURIDate(instance.instance) + "/" + + formattedRunId, file.getPath().getName()); + if (filePath.getName().equals("oozie.log")) { + instance.logFile = dfsBrowserUrl; + continue; + } + + InstanceAction instanceAction = new InstanceAction( + getActionName(filePath.getName()), + getActionStatus(filePath.getName()), dfsBrowserUrl); + instanceActions[i++] = instanceAction; } - - InstanceAction instanceAction = new InstanceAction( - getActionName(filePath.getName()), - getActionStatus(filePath.getName()), dfsBrowserUrl); - instanceActions[i++] = instanceAction; } - return instance; } private String getActionName(String fileName) { - return fileName.replaceAll("_SUCCEEDED.log", "").replaceAll( - "_FAILED.log", ""); + return fileName.replaceAll("_SUCCEEDED\\.log", "").replaceAll( + "_FAILED\\.log", ""); } private String getActionStatus(String fileName) { http://git-wip-us.apache.org/repos/asf/falcon/blob/b6d2c134/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java ---------------------------------------------------------------------- diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java index 8856d80..185c830 100644 --- a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java +++ b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java @@ -567,17 +567,15 @@ public abstract class AbstractEntityManager extends AbstractMetadataResource { //now obtain locks for all dependent entities if any. Set<Entity> affectedEntities = EntityGraph.get().getDependents(entity); - if (affectedEntities != null) { - for (Entity e : affectedEntities) { - if (memoryLocks.acquireLock(e, command)) { - tokenList.add(e); - LOG.debug("{} on entity {} has acquired lock on {}", command, entity, e); - } else { - LOG.error("Error while trying to acquire lock on {}. Releasing already obtained locks", - e.toShortString()); - throw new FalconException("There are multiple update commands running for dependent entity " - + e.toShortString()); - } + for (Entity e : affectedEntities) { + if (memoryLocks.acquireLock(e, command)) { + tokenList.add(e); + LOG.debug("{} on entity {} has acquired lock on {}", command, entity, e); + } else { + LOG.error("Error while trying to acquire lock on {}. Releasing already obtained locks", + e.toShortString()); + throw new FalconException("There are multiple update commands running for dependent entity " + + e.toShortString()); } } }
