#423 Fix for replicas when forests per host exceeds number of hosts

Issue pertained to the number of forests on each host causing a failure in determining which host replicas should go on to.

Author: rjrudin

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

@@ -1,11 +1,12 @@
 package com.marklogic.appdeployer.command.forests;
 
 import com.marklogic.appdeployer.AppConfig;
+import com.marklogic.client.ext.helper.LoggingObject;
 import com.marklogic.mgmt.api.forest.ForestReplica;
 
 import java.util.Map;
 
-public abstract class AbstractReplicaBuilderStrategy implements ReplicaBuilderStrategy {
+public abstract class AbstractReplicaBuilderStrategy extends LoggingObject implements ReplicaBuilderStrategy {
 
 	/**
 	 * Configures the fast and large data directories for a replica based on what's in AppConfig for the given

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

@@ -35,6 +35,7 @@ public void buildReplicas(List<Forest> forests, ForestPlan forestPlan, AppConfig
 		}
 
 		for (String host : hostToForests.keySet()) {
+			logger.info("Determining replicas for host: " + host);
 
 			// availableHosts will be the hosts that we can put a forest's replicas on, which excludes the host where
 			// the forest lives. We also want to have the hosts in different order as we assign replicas to hosts, so
@@ -46,6 +47,8 @@ public void buildReplicas(List<Forest> forests, ForestPlan forestPlan, AppConfig
 				availableHosts.addAll(hostNames.subList(hostIndex + 1, hostNames.size()));
 			}
 			availableHosts.addAll(hostNames.subList(0, hostIndex));
+			final int availableHostCount = availableHosts.size();
+			logger.info("Available hosts for replicas: " + availableHosts);
 
 			int hostPointer = 0;
 
@@ -59,11 +62,12 @@ public void buildReplicas(List<Forest> forests, ForestPlan forestPlan, AppConfig
 					replicas.add(replica);
 
 					int replicaHostPointer = hostPointer + i - 1;
-					if (replicaHostPointer == availableHosts.size()) {
-						replicaHostPointer = 0;
+					if (replicaHostPointer >= availableHostCount) {
+						// Must do a modulo here in the event that there are more primary forests per host than number of hosts
+						replicaHostPointer %= availableHostCount;
 					}
-
 					replica.setHost(availableHosts.get(replicaHostPointer));
+					logger.info(format("Built replica '%s' for forest '%s' on host '%s'", replica.getReplicaName(), currForest.getForestName(), replica.getHost()));
 
 					dataDirectoryPointer++;
 					if (dataDirectoryPointer == replicaDataDirectories.size()) {
@@ -82,7 +86,7 @@ public void buildReplicas(List<Forest> forests, ForestPlan forestPlan, AppConfig
 
 				++hostPointer;
 
-				if (hostPointer == availableHosts.size()) {
+				if (hostPointer == availableHostCount) {
 					hostPointer = 0;
 				}
 

src/test/java/com/marklogic/appdeployer/command/forests/BuildForestReplicaTest.java

@@ -8,7 +8,11 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
 
 public class BuildForestReplicaTest extends Assert {
 
@@ -37,6 +41,56 @@ public void multipleForestsOnEachHost() {
 		assertEquals("host2", forests.get(5).getForestReplica().get(0).getHost());
 	}
 
+	/**
+	 * Test was added for https://github.com/marklogic-community/ml-app-deployer/issues/423, where a bug was detected
+	 * when the "host pointer" in the implementation code could exceed the number of hosts. This test reproduced the bug.
+	 * Just doing some basic assertions on the number of forests and replicas created, but the key is that the test no
+	 * longer fails.
+	 */
+	@Test
+	public void sameNumberOfForestsAsHosts() {
+		AppConfig appConfig = newAppConfig("mlForestsPerHost", "db,4");
+
+		List<Forest> forests = builder.buildForests(
+			new ForestPlan("db", "host1", "host2", "host3", "host4").withReplicaCount(3), appConfig);
+
+		assertEquals(16, forests.size());
+		forests.forEach(forest -> {
+			assertEquals(3, forest.getForestReplica().size());
+		});
+	}
+
+	/**
+	 * Similar to the above test, just even more forests. 
+	 */
+	@Test
+	public void numberOfForestsPerHostIsMoreThanDoubleTheNumberOfHosts() {
+		AppConfig appConfig = newAppConfig("mlForestsPerHost", "db,10");
+
+		List<Forest> forests = builder.buildForests(
+			new ForestPlan("db", "host1", "host2", "host3", "host4").withReplicaCount(3), appConfig);
+
+		assertEquals(40, forests.size());
+
+		Map<String, AtomicInteger> hostToReplicaCounts = new HashMap<>();
+		Stream.of("host1", "host2", "host3", "host4").forEach(host -> hostToReplicaCounts.put(host, new AtomicInteger(0)));
+
+		AtomicInteger replicaCount = new AtomicInteger(0);
+
+		forests.forEach(forest -> {
+			assertEquals(3, forest.getForestReplica().size());
+			forest.getForestReplica().forEach(replica -> {
+				hostToReplicaCounts.get(replica.getHost()).getAndIncrement();
+				replicaCount.getAndIncrement();
+			});
+		});
+
+		assertEquals("Expecting 120 replicas; we have 40 forests, and we expect 3 replicas for each", 120, replicaCount.get());
+		Stream.of("host1", "host2", "host3", "host4").forEach(host -> {
+			assertEquals("Each host should have 30 replicas, as there are 120 total", 30, hostToReplicaCounts.get(host).get());
+		});
+	}
+
 	@Test
 	public void customNamingStrategyWithDistributedStrategy() {
 		AppConfig appConfig = newAppConfig("mlForestsPerHost", "my-database,2");