You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@falcon.apache.org by pe...@apache.org on 2016/07/15 15:46:31 UTC

falcon git commit: FALCON-2073 Handle with NULL corner case

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 <yz...@hortonworks.com>

Reviewers: Peeyush <pe...@apache.org>, Balu <ba...@apache.org>

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 <yz...@hortonworks.com>
Authored: Fri Jul 15 21:16:03 2016 +0530
Committer: peeyush b <pb...@hortonworks.com>
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());
             }
         }
     }