ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jonathanhur...@apache.org
Subject ambari git commit: AMBARI-18404 - Upgrade Summary Endpoint Throws NPEs Due To JPA Cached Entities With Missing IDs (jonathanhurley)
Date Fri, 16 Sep 2016 04:12:17 GMT
Repository: ambari
Updated Branches:
  refs/heads/branch-2.4 d7056ba9a -> 1f43692b1


AMBARI-18404 - Upgrade Summary Endpoint Throws NPEs Due To JPA Cached Entities With Missing
IDs (jonathanhurley)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/1f43692b
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/1f43692b
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/1f43692b

Branch: refs/heads/branch-2.4
Commit: 1f43692b14dd9776f7687b1705d8d43f674528b0
Parents: d7056ba
Author: Jonathan Hurley <jhurley@hortonworks.com>
Authored: Thu Sep 15 12:12:06 2016 -0400
Committer: Jonathan Hurley <jhurley@hortonworks.com>
Committed: Fri Sep 16 00:11:20 2016 -0400

----------------------------------------------------------------------
 .../actionmanager/ActionDBAccessorImpl.java     | 14 +++++-----
 .../server/actionmanager/HostRoleCommand.java   | 16 +++++++++++
 .../orm/entities/HostRoleCommandEntity.java     | 19 +++++++++++++
 .../ambari/server/topology/HostRequest.java     | 21 ++++++--------
 .../actionmanager/TestActionDBAccessorImpl.java | 29 ++++++++++++++++++++
 5 files changed, 80 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/1f43692b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
index b7e7f2d..c31ca7e 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
@@ -344,7 +344,6 @@ public class ActionDBAccessorImpl implements ActionDBAccessor {
       StageEntity stageEntity = stage.constructNewPersistenceEntity();
       stageEntities.add(stageEntity);
       stageEntity.setClusterId(clusterId);
-      //TODO refactor to reduce merges
       stageEntity.setRequest(requestEntity);
       stageDAO.create(stageEntity);
 
@@ -353,9 +352,6 @@ public class ActionDBAccessorImpl implements ActionDBAccessor {
       for (HostRoleCommand hostRoleCommand : orderedHostRoleCommands) {
         HostRoleCommandEntity hostRoleCommandEntity = hostRoleCommand.constructNewPersistenceEntity();
         hostRoleCommandEntity.setStage(stageEntity);
-
-        HostEntity hostEntity = null;
-
         hostRoleCommandDAO.create(hostRoleCommandEntity);
 
         assert hostRoleCommandEntity.getTaskId() != null;
@@ -365,6 +361,7 @@ public class ActionDBAccessorImpl implements ActionDBAccessor {
         String output = "output-" + hostRoleCommandEntity.getTaskId() + ".txt";
         String error = "errors-" + hostRoleCommandEntity.getTaskId() + ".txt";
 
+        HostEntity hostEntity = null;
         if (null != hostRoleCommandEntity.getHostId()) {
           hostEntity = hostDAO.findById(hostRoleCommandEntity.getHostId());
           if (hostEntity == null) {
@@ -372,6 +369,7 @@ public class ActionDBAccessorImpl implements ActionDBAccessor {
             LOG.error(msg);
             throw new AmbariException(msg);
           }
+
           hostRoleCommandEntity.setHostEntity(hostEntity);
 
           try {
@@ -401,9 +399,10 @@ public class ActionDBAccessorImpl implements ActionDBAccessor {
         hostRoleCommandEntity.setExecutionCommand(executionCommandEntity);
 
         executionCommandDAO.create(hostRoleCommandEntity.getExecutionCommand());
-        hostRoleCommandDAO.merge(hostRoleCommandEntity);
+        hostRoleCommandEntity = hostRoleCommandDAO.merge(hostRoleCommandEntity);
+
         if (null != hostEntity) {
-          hostDAO.merge(hostEntity);
+          hostEntity = hostDAO.merge(hostEntity);
         }
       }
 
@@ -411,8 +410,9 @@ public class ActionDBAccessorImpl implements ActionDBAccessor {
         roleSuccessCriteriaDAO.create(roleSuccessCriteriaEntity);
       }
 
-      stageDAO.create(stageEntity);
+      stageEntity = stageDAO.merge(stageEntity);
     }
+
     requestEntity.setStages(stageEntities);
     requestDAO.merge(requestEntity);
   }

http://git-wip-us.apache.org/repos/asf/ambari/blob/1f43692b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java
b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java
index ff2ce92..85c8e9f 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java
@@ -209,6 +209,22 @@ public class HostRoleCommand {
 
     hostRoleCommandEntity.setEvent(event.getEventJson());
 
+    // set IDs if the wrapping object has them - they are most likely
+    // non-updatable in JPA since they are retrieved from a relationship,
+    // however the JPA cache may choose to not refresh the entity so they would
+    // end up being null if not set before persisting the command entity
+    if (requestId >= 0) {
+      hostRoleCommandEntity.setRequestId(requestId);
+    }
+
+    if (stageId >= 0) {
+      hostRoleCommandEntity.setStageId(stageId);
+    }
+
+    if (taskId >= 0) {
+      hostRoleCommandEntity.setTaskId(taskId);
+    }
+
     return hostRoleCommandEntity;
   }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/1f43692b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java
b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java
index 6288091..74271b9 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java
@@ -511,8 +511,27 @@ public class HostRoleCommandEntity {
     return stage;
   }
 
+  /**
+   * Sets the associated {@link StageEntity} for this command. If the
+   * {@link StageEntity} has been persisted, then this will also set the
+   * commands stage and request ID fields.
+   *
+   * @param stage
+   */
   public void setStage(StageEntity stage) {
     this.stage = stage;
+
+    // ensure that the IDs are also set since they may not be retrieved from JPA
+    // when this entity is cached
+    if (null != stage) {
+      if (null == stageId) {
+        stageId = stage.getStageId();
+      }
+
+      if (null == requestId) {
+        requestId = stage.getRequestId();
+      }
+    }
   }
 
   public HostEntity getHostEntity() {

http://git-wip-us.apache.org/repos/asf/ambari/blob/1f43692b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java
b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java
index 4dd6f97..2136b68 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java
@@ -18,6 +18,15 @@
 
 package org.apache.ambari.server.topology;
 
+import static org.apache.ambari.server.controller.internal.ProvisionAction.INSTALL_ONLY;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.ambari.server.actionmanager.HostRoleCommand;
 import org.apache.ambari.server.api.predicate.InvalidQueryException;
 import org.apache.ambari.server.api.predicate.PredicateCompiler;
@@ -36,15 +45,6 @@ import org.apache.ambari.server.state.host.HostImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-
-import static org.apache.ambari.server.controller.internal.ProvisionAction.INSTALL_ONLY;
-
 /**
  * Represents a set of requests to a single host such as install, start, etc.
  */
@@ -341,9 +341,6 @@ public class HostRequest implements Comparable<HostRequest> {
     for (HostRoleCommand task : logicalTasks.values()) {
       HostRoleCommandEntity entity = task.constructNewPersistenceEntity();
       // the above method doesn't set all of the fields for some unknown reason
-      entity.setRequestId(task.getRequestId());
-      entity.setStageId(task.getStageId());
-      entity.setTaskId(task.getTaskId());
       entity.setOutputLog(task.getOutputLog());
       entity.setErrorLog(task.errorLog);
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/1f43692b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
index 0813dff..bf9d0db 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
@@ -27,6 +27,7 @@ import java.util.Collections;
 import java.util.List;
 
 import javax.persistence.EntityManager;
+import javax.persistence.NamedQuery;
 
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.Role;
@@ -611,6 +612,34 @@ public class TestActionDBAccessorImpl {
 
   }
 
+  /**
+   * Tests that entities created int he {@link ActionDBAccessor} can be
+   * retrieved with their IDs intact. EclipseLink seems to execute the
+   * {@link NamedQuery} but then use its cached entities to fill in the data.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testEntitiesCreatedWithIDs() throws Exception {
+    List<Stage> stages = new ArrayList<Stage>();
+    Stage stage = createStubStage(hostName, requestId, stageId);
+
+    stages.add(stage);
+
+    Request request = new Request(stages, clusters);
+
+    // persist entities
+    db.persistActions(request);
+
+    // query entities immediately to ensure IDs are populated
+    List<HostRoleCommandEntity> commandEntities = hostRoleCommandDAO.findByRequest(requestId);
+    Assert.assertEquals(2, commandEntities.size());
+
+    for (HostRoleCommandEntity entity : commandEntities) {
+      Assert.assertEquals(Long.valueOf(requestId), entity.getRequestId());
+      Assert.assertEquals(Long.valueOf(stageId), entity.getStageId());
+    }
+  }
 
   private static class TestActionDBAccessorModule extends AbstractModule {
     @Override


Mime
View raw message