DEVEXP-565 Using map of forests to determining if forests have replicas

Refactored the call for getting a map of forests - moved it into CommandContext so it's easier to reuse by separate commands. 

Note that `DeployOtherDatabasesCommand` is being reverted back as no changes are needed there anymore.

Author: rjrudin

src/main/java/com/marklogic/appdeployer/command/CommandContext.java

@@ -20,8 +20,13 @@
 import com.marklogic.mgmt.admin.AdminManager;
 import com.marklogic.mgmt.api.configuration.Configuration;
 import com.marklogic.mgmt.api.configuration.Configurations;
+import com.marklogic.mgmt.api.forest.Forest;
+import com.marklogic.mgmt.resource.forests.ForestManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -61,6 +66,36 @@ public void addCmaConfigurationToCombinedRequest(Configuration configuration) {
 		}
 	}
 
+	/**
+	 * Added to greatly speed up performance when getting details about all the existing primary forests for each
+	 * database in the cluster. In the event that anything fails while getting the forest details, the map won't be
+	 * added to the context and any code expecting to use the map will have to fall back to using /manage/v2.
+	 *
+	 * @since 4.5.3
+	 */
+	public Map<String, List<Forest>> getMapOfPrimaryForests() {
+		if (!appConfig.getCmaConfig().isDeployForests()) {
+			return null;
+		}
+		final String key = "ml-app-deployer-mapOfPrimaryForests";
+		if (contextMap.containsKey(key)) {
+			return (Map<String, List<Forest>>) contextMap.get(key);
+		}
+		Logger logger = LoggerFactory.getLogger(getClass());
+		try {
+			logger.info("Retrieving all forest details via CMA");
+			long start = System.currentTimeMillis();
+			Map<String, List<Forest>> mapOfPrimaryForests = new ForestManager(manageClient).getMapOfPrimaryForests();
+			logger.info("Finished retrieving all forests details via CMA; duration: " + (System.currentTimeMillis() - start));
+			contextMap.put(key, mapOfPrimaryForests);
+			return mapOfPrimaryForests;
+		} catch (Exception ex) {
+			logger.warn("Unable to retrieve all forest details, cause: " + ex.getMessage() + "; will fall back to " +
+				"using /manage/v2 when needed for getting details for a forest.");
+			return null;
+		}
+	}
+
 	public Configurations getCombinedCmaRequest() {
 		return (Configurations) contextMap.get(COMBINED_CMA_REQUEST_KEY);
 	}

src/main/java/com/marklogic/appdeployer/command/databases/DeployOtherDatabasesCommand.java

@@ -34,22 +34,13 @@
 import com.marklogic.mgmt.mapper.DefaultResourceMapper;
 import com.marklogic.mgmt.mapper.ResourceMapper;
 import com.marklogic.mgmt.resource.databases.DatabaseManager;
-import com.marklogic.mgmt.resource.forests.ForestManager;
 import com.marklogic.mgmt.util.ObjectMapperFactory;
 import com.marklogic.rest.util.JsonNodeUtil;
 
 import java.io.File;
 import java.io.FilenameFilter;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.stream.Collectors;
+import java.util.*;
 
 /**
  * As of release 3.14.0, this now handles all databases, not just "databases other than the default content database".
@@ -170,7 +161,7 @@ public void undo(CommandContext context) {
 	/**
 	 * If no database files are found, may still need to delete the content database in case no file exists for it.
 	 * That's because the command for creating a REST API server will not delete the content database by default.
-	 * <p>
+	 *
 	 * Per ticket #404, this will now do a check to see if the default content database filename is ignored. If so,
 	 * and there are no database files found, then the content database will not be deleted.
 	 *
@@ -401,8 +392,6 @@ protected void buildDeployDatabaseCommands(CommandContext context, List<Database
 	 * @param databasePlans
 	 */
 	protected void deployDatabasesAndForestsViaCma(CommandContext context, List<DatabasePlan> databasePlans) {
-		addForestMapToCommandContext(context, databasePlans);
-
 		Configuration dbConfig = new Configuration();
 		// Forests must be included in a separate configuration object
 		Configuration forestConfig = new Configuration();
@@ -426,28 +415,6 @@ protected void deployDatabasesAndForestsViaCma(CommandContext context, List<Data
 		});
 	}
 
-	/**
-	 * Added to greatly speed up performance when getting details about all the existing primary forests for each
-	 * database referenced by a plan. In the event that anything fails, the map won't be added to the command context
-	 * and any code expecting to use the map will just have to fall back to use /manage/v2.
-	 *
-	 * @param context
-	 * @param databasePlans
-	 */
-	private void addForestMapToCommandContext(CommandContext context, List<DatabasePlan> databasePlans) {
-		try {
-			Set<String> dbNames = databasePlans.stream().map(plan -> plan.getDatabaseName()).collect(Collectors.toSet());
-			logger.info("Retrieving all forest details via CMA");
-			long start = System.currentTimeMillis();
-			Map<String, List<Forest>> forestMap = new ForestManager(context.getManageClient()).getPrimaryForestsForDatabases(dbNames.toArray(new String[]{}));
-			logger.info("Finished retrieving all forests details via CMA; duration: " + (System.currentTimeMillis() - start));
-			context.getContextMap().put("ml-app-deployer-forestMap", forestMap);
-		} catch (Exception ex) {
-			logger.warn("Unable to retrieve all forest details, cause: " + ex.getMessage() + "; will fall back to " +
-				"using /manage/v2 when needed for getting details for a forest.");
-		}
-	}
-
 	/**
 	 * Each DatabasePlan is expected to have constructed a DeployForestCommand, but not executed it. Each
 	 * DeployForestCommand can then be used to build a list of forests. All of those forests can be combined into a

src/main/java/com/marklogic/appdeployer/command/forests/ConfigureForestReplicasCommand.java

@@ -185,22 +185,41 @@ protected List<Forest> determineForestsNeedingReplicas(String databaseName, Comm
 		ResourceMapper resourceMapper = new DefaultResourceMapper(api);
 
 		List<Forest> forestsNeedingReplicas = new ArrayList<>();
+		Map<String, List<Forest>> mapOfPrimaryForests = context.getMapOfPrimaryForests();
 
-		for (String forestName : dbMgr.getForestNames(databaseName)) {
-			logger.info(format("Checking the status of forest %s to determine if it is a primary forest and whether or not it has replicas already.", forestName));
-			ForestStatus status = forestManager.getForestStatus(forestName);
-			if (!status.isPrimary()) {
-				logger.info(format("Forest %s is not a primary forest, so not configuring replica forests", forestName));
-				continue;
-			}
-			if (status.hasReplicas()) {
-				logger.info(format("Forest %s already has replicas, so not configuring replica forests", forestName));
-				continue;
-			}
+		/**
+		 * In both blocks below, a forest is not included if it already has replicas. This logic dates back to 2015,
+		 * and is likely due to uncertainty over the various scenarios that can occur if a forest does already have
+		 * replicas. At least as of August 2023, MarkLogic recommends a single replica per forest. Given that no users
+		 * have asked for this check to not be performed and based on MarkLogic's recommendation, it seems reasonable
+		 * to leave this check in for now. However, some ad hoc testing has indicated that this check is unnecessary
+		 * and that it appears to safe to vary the number of replicas per forest. So it likely would be beneficial to
+		 * remove this check at some point.
+		 */
+		if (mapOfPrimaryForests != null && mapOfPrimaryForests.containsKey(databaseName)) {
+			mapOfPrimaryForests.get(databaseName).forEach(forest -> {
+				boolean forestHasReplicasAlready = forest.getForestReplica() != null && !forest.getForestReplica().isEmpty();
+				if (!forestHasReplicasAlready) {
+					forestsNeedingReplicas.add(forest);
+				}
+			});
+		} else {
+			for (String forestName : dbMgr.getForestNames(databaseName)) {
+				logger.info(format("Checking the status of forest %s to determine if it is a primary forest and whether or not it has replicas already.", forestName));
+				ForestStatus status = forestManager.getForestStatus(forestName);
+				if (!status.isPrimary()) {
+					logger.info(format("Forest %s is not a primary forest, so not configuring replica forests", forestName));
+					continue;
+				}
+				if (status.hasReplicas()) {
+					logger.info(format("Forest %s already has replicas, so not configuring replica forests", forestName));
+					continue;
+				}
 
-			String forestJson = forestManager.getPropertiesAsJson(forestName);
-			Forest forest = resourceMapper.readResource(forestJson, Forest.class);
-			forestsNeedingReplicas.add(forest);
+				String forestJson = forestManager.getPropertiesAsJson(forestName);
+				Forest forest = resourceMapper.readResource(forestJson, Forest.class);
+				forestsNeedingReplicas.add(forest);
+			}
 		}
 
 		return forestsNeedingReplicas;

src/main/java/com/marklogic/appdeployer/command/forests/DeployForestsCommand.java

@@ -132,11 +132,9 @@ public List<Forest> buildForests(CommandContext context, boolean includeReplicas
 
 		// As of 4.5.3, if CMA is enabled, then the context should contain a map of all the forests for each database
 		// being deployed. If it's not there, then /manage/v2 will be used instead.
-		if (context.getAppConfig().getCmaConfig().isDeployForests()) {
-			Map<String, List<Forest>> map = (Map<String, List<Forest>>) context.getContextMap().get("ml-app-deployer-forestMap");
-			if (map != null && map.containsKey(this.databaseName)) {
-				existingPrimaryForests = map.get(this.databaseName);
-			}
+		Map<String, List<Forest>> mapOfPrimaryForests = context.getMapOfPrimaryForests();
+		if (mapOfPrimaryForests != null && mapOfPrimaryForests.containsKey(this.databaseName)) {
+			existingPrimaryForests = mapOfPrimaryForests.get(this.databaseName);
 		}
 
 		if (existingPrimaryForests == null) {

src/main/java/com/marklogic/mgmt/resource/forests/ForestManager.java

@@ -28,7 +28,6 @@
 import org.springframework.web.util.UriComponentsBuilder;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -61,40 +60,36 @@ public ForestManager(ManageClient client) {
 	}
 
 	/**
-	 * Uses CMA to get back all forests in the cluster. Then returns a map where keys are the given database names,
+	 * Uses CMA to get back all forests in the cluster. Then returns a map where keys are the database names,
 	 * and each key has a list of primary forests for that database.
 	 *
-	 * @param databaseNames
 	 * @return
 	 * @since 4.5.3
 	 */
-	public Map<String, List<Forest>> getPrimaryForestsForDatabases(String... databaseNames) {
+	public Map<String, List<Forest>> getMapOfPrimaryForests() {
 		String uri = UriComponentsBuilder.fromUri(getManageClient().buildUri("/manage/v3"))
 			.queryParam("format", "json").queryParam("resource-type", "forest")
 			.encode().toUriString();
 
 		JsonNode json = getManageClient().getRestTemplate().exchange(uri, HttpMethod.GET, null, JsonNode.class).getBody();
-		// Config is an array of objects, and it should have a single object based on our request.
+		// Config is an array of objects, and it will have a single object based on our request.
 		ArrayNode allPrimaryForests = (ArrayNode) json.get("config").get(0).get("forest");
 		ResourceMapper mapper = new DefaultResourceMapper(new API(getManageClient()));
 
-		List<String> dbNames = Arrays.asList(databaseNames);
-		Map<String, List<Forest>> forestMap = new HashMap<>();
+		Map<String, List<Forest>> mapOfPrimaryForests = new HashMap<>();
 		allPrimaryForests.iterator().forEachRemaining(forest -> {
 			if (forest.has("database")) {
 				String db = forest.get("database").asText();
-				if (dbNames.contains(db)) {
-					List<Forest> forests = forestMap.get(db);
-					if (forests == null) {
-						forests = new ArrayList<>();
-						forestMap.put(db, forests);
-					}
-					forests.add(mapper.readResource(forest.toString(), Forest.class));
+				List<Forest> databaseForests = mapOfPrimaryForests.get(db);
+				if (databaseForests == null) {
+					databaseForests = new ArrayList<>();
+					mapOfPrimaryForests.put(db, databaseForests);
 				}
+				databaseForests.add(mapper.readResource(forest.toString(), Forest.class));
 			}
 		});
-		
-		return forestMap;
+
+		return mapOfPrimaryForests;
 	}
 
 	public void createJsonForestWithName(String name, String host) {

src/test/java/com/marklogic/mgmt/resource/forests/GetPrimaryForestsTest.java renamed to src/test/java/com/marklogic/mgmt/resource/forests/GetMapOfPrimaryForestsTest.java

@@ -8,10 +8,9 @@
 import java.util.Map;
 import java.util.Set;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-public class GetPrimaryForestsTest extends AbstractMgmtTest {
+public class GetMapOfPrimaryForestsTest extends AbstractMgmtTest {
 
 	/**
 	 * Simple test for verifying this method works for a couple OOTB databases. It is expected that the app-deployer
@@ -20,14 +19,12 @@ public class GetPrimaryForestsTest extends AbstractMgmtTest {
 	 */
 	@Test
 	void test() {
-		Map<String, List<Forest>> forestMap = new ForestManager(manageClient)
-			.getPrimaryForestsForDatabases("App-Services", "Documents");
+		Map<String, List<Forest>> mapOfPrimaryForests = new ForestManager(manageClient).getMapOfPrimaryForests();
 
-		Set<String> dbNames = forestMap.keySet();
+		Set<String> dbNames = mapOfPrimaryForests.keySet();
 		assertTrue(dbNames.contains("App-Services"));
 		assertTrue(dbNames.contains("Documents"));
-		assertEquals(2, dbNames.size());
-		assertTrue(forestMap.get("App-Services").size() > 0);
-		assertTrue(forestMap.get("Documents").size() > 0);
+		assertTrue(mapOfPrimaryForests.get("App-Services").size() > 0);
+		assertTrue(mapOfPrimaryForests.get("Documents").size() > 0);
 	}
 }