Merge pull request #310 from ibi-group/deployment-perf-fixes

Deployment perf fixes

src/main/java/com/conveyal/datatools/manager/controllers/api/DeploymentController.java

@@ -9,7 +9,6 @@
 import com.amazonaws.services.ec2.model.Instance;
 import com.amazonaws.services.ec2.model.Reservation;
 import com.amazonaws.services.s3.AmazonS3;
-import com.amazonaws.services.s3.AmazonS3ClientBuilder;
 import com.amazonaws.services.s3.AmazonS3URI;
 import com.conveyal.datatools.common.status.MonitorableJob;
 import com.conveyal.datatools.common.utils.AWSUtils;
@@ -60,7 +59,6 @@
  * These methods are mapped to API endpoints by Spark.
  */
 public class DeploymentController {
-    private static JsonManager<Deployment> json = new JsonManager<>(Deployment.class, JsonViews.UserInterface.class);
     private static final Logger LOG = LoggerFactory.getLogger(DeploymentController.class);
     private static Map<String, DeployJob> deploymentJobsByServer = new HashMap<>();
     private static final AmazonEC2 ec2 = AmazonEC2Client.builder().build();
@@ -494,22 +492,29 @@ private static String deploy (Request req, Response res) {
     }
 
     public static void register (String apiPrefix) {
-        post(apiPrefix + "secure/deployments/:id/deploy/:target", DeploymentController::deploy, json::write);
+        // Construct JSON managers which help serialize the response. Slim JSON is the generic JSON view. Full JSON
+        // contains additional fields (at the moment just #ec2Instances) and should only be used when the controller
+        // returns a single deployment (slimJson is better suited for a collection). If fullJson is attempted for use
+        // with a collection, massive performance issues will ensure (mainly due to multiple calls to AWS EC2).
+        JsonManager<Deployment> slimJson = new JsonManager<>(Deployment.class, JsonViews.UserInterface.class);
+        JsonManager<Deployment> fullJson = new JsonManager<>(Deployment.class, JsonViews.UserInterface.class);
+        fullJson.addMixin(Deployment.class, Deployment.DeploymentWithEc2InstancesMixin.class);
+
+        post(apiPrefix + "secure/deployments/:id/deploy/:target", DeploymentController::deploy, slimJson::write);
         post(apiPrefix + "secure/deployments/:id/deploy/", ((request, response) -> {
             logMessageAndHalt(request, 400, "Must provide valid deployment target name");
             return null;
-        }), json::write);
+        }), slimJson::write);
         options(apiPrefix + "secure/deployments", (q, s) -> "");
         get(apiPrefix + "secure/deployments/:id/download", DeploymentController::downloadDeployment);
         get(apiPrefix + "secure/deployments/:id/artifact", DeploymentController::downloadBuildArtifact);
-        get(apiPrefix + "secure/deployments/:id/ec2", DeploymentController::fetchEC2InstanceSummaries, json::write);
-        delete(apiPrefix + "secure/deployments/:id/ec2", DeploymentController::terminateEC2InstanceForDeployment, json::write);
-        get(apiPrefix + "secure/deployments/:id", DeploymentController::getDeployment, json::write);
-        delete(apiPrefix + "secure/deployments/:id", DeploymentController::deleteDeployment, json::write);
-        get(apiPrefix + "secure/deployments", DeploymentController::getAllDeployments, json::write);
-        post(apiPrefix + "secure/deployments", DeploymentController::createDeployment, json::write);
-//        post(apiPrefix + "secure/deployments/:id/ec2", DeploymentController::addEC2InstanceToDeployment, json::write);
-        put(apiPrefix + "secure/deployments/:id", DeploymentController::updateDeployment, json::write);
-        post(apiPrefix + "secure/deployments/fromfeedsource/:id", DeploymentController::createDeploymentFromFeedSource, json::write);
+        get(apiPrefix + "secure/deployments/:id/ec2", DeploymentController::fetchEC2InstanceSummaries, slimJson::write);
+        delete(apiPrefix + "secure/deployments/:id/ec2", DeploymentController::terminateEC2InstanceForDeployment, slimJson::write);
+        get(apiPrefix + "secure/deployments/:id", DeploymentController::getDeployment, fullJson::write);
+        delete(apiPrefix + "secure/deployments/:id", DeploymentController::deleteDeployment, fullJson::write);
+        get(apiPrefix + "secure/deployments", DeploymentController::getAllDeployments, slimJson::write);
+        post(apiPrefix + "secure/deployments", DeploymentController::createDeployment, fullJson::write);
+        put(apiPrefix + "secure/deployments/:id", DeploymentController::updateDeployment, fullJson::write);
+        post(apiPrefix + "secure/deployments/fromfeedsource/:id", DeploymentController::createDeploymentFromFeedSource, fullJson::write);
     }
 }

src/main/java/com/conveyal/datatools/manager/models/Deployment.java

@@ -69,10 +69,12 @@ public class Deployment extends Model implements Serializable {
 
     public List<DeployJob.DeploySummary> deployJobSummaries = new ArrayList<>();
 
-    @JsonView(JsonViews.DataDump.class)
     public String projectId;
 
-    @JsonProperty("project")
+    /**
+     * Get parent project for deployment. Note: at one point this was a JSON property of this class, but severe
+     * performance issues prevent this field from scaling to be fetched/assigned to a large collection of deployments.
+     */
     public Project parentProject() {
         return Persistence.projects.getById(projectId);
     }
@@ -116,8 +118,7 @@ public List<SummarizedFeedVersion> retrieveFeedVersions() {
         return ret;
     }
 
-    /** All of the feed versions used in this deployment, summarized so that the Internet won't break */
-    @JsonProperty("ec2Instances")
+    /** Fetch ec2 instances tagged with this deployment's ID. */
     public List<EC2InstanceSummary> retrieveEC2Instances() {
         if (!"true".equals(DataManager.getConfigPropertyAsText("modules.deployment.ec2.enabled"))) return Collections.EMPTY_LIST;
         Filter deploymentFilter = new Filter("tag:deploymentId", Collections.singletonList(id));
@@ -139,14 +140,6 @@ public List<EC2InstanceSummary> retrieveEC2Instances() {
         );
     }
 
-    public void storeFeedVersions(Collection<FeedVersion> versions) {
-        feedVersionIds = new ArrayList<>(versions.size());
-
-        for (FeedVersion version : versions) {
-            feedVersionIds.add(version.id);
-        }
-    }
-
     /**
      * Public URL at which the OSM extract should be downloaded. This should be null if the extract should be downloaded
      * from an extract server. Extract type should be a .pbf.
@@ -558,10 +551,23 @@ public boolean boundsAreValid () {
      */
     public abstract static class DeploymentFullFeedVersionMixin {
         @JsonIgnore
-        public abstract Collection<SummarizedFeedVersion> retrievefeedVersions();
+        public abstract Collection<SummarizedFeedVersion> retrieveFeedVersions();
 
-//        @JsonProperty("feedVersions")
+        @JsonProperty("feedVersions")
         @JsonIgnore(false)
         public abstract Collection<FeedVersion> retrieveFullFeedVersions ();
     }
+
+    /**
+     * A MixIn to be applied to this deployment, for returning a single deployment, so that the list of ec2Instances is
+     * included in the JSON response.
+     *
+     * Usually a mixin would be used on an external class, but since we are changing one thing about a single class, it seemed
+     * unnecessary to define a new view.
+     */
+    public abstract static class DeploymentWithEc2InstancesMixin {
+
+        @JsonProperty("ec2Instances")
+        public abstract Collection<FeedVersion> retrieveEC2Instances ();
+    }
 }