Skip to content
This repository was archived by the owner on Sep 16, 2024. It is now read-only.

Commit fef2c8f

Browse files
rjrudinrjrudin
committed
#439 Fixing bugs with database forests on one host
mlDatabaseGroups now works correctly when selecting a host. And if primary forests are already on a host, that host will always be selected.
1 parent 4e2de1a commit fef2c8f

File tree

4 files changed

+101
-30
lines changed

4 files changed

+101
-30
lines changed

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

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import com.marklogic.mgmt.resource.databases.DatabaseManager;
1414
import com.marklogic.mgmt.resource.forests.ForestManager;
1515
import com.marklogic.mgmt.resource.hosts.DefaultHostNameProvider;
16-
import com.marklogic.mgmt.resource.hosts.HostManager;
1716
import org.springframework.util.StringUtils;
1817

1918
import java.io.File;
@@ -114,18 +113,28 @@ protected void createForestsViaForestEndpoint(CommandContext context, List<Fores
114113
* @return
115114
*/
116115
public List<Forest> buildForests(CommandContext context, boolean includeReplicas) {
117-
String template = buildForestTemplate(context, new ForestManager(context.getManageClient()));
116+
// Need to know what primary forests exist already in case more need to be added, or a new host has been added
117+
List<Forest> existingPrimaryForests = getExistingPrimaryForests(context, this.databaseName);
118+
return buildForests(context, includeReplicas, existingPrimaryForests);
119+
}
118120

119-
final List<String> allHostNames = new HostManager(context.getManageClient()).getHostNames();
120-
ForestHostNames forestHostNames = determineHostNamesForForest(context, allHostNames);
121+
/**
122+
*
123+
* @param context
124+
* @param includeReplicas
125+
* @param existingPrimaryForests
126+
* @return
127+
*/
128+
protected List<Forest> buildForests(CommandContext context, boolean includeReplicas, List<Forest> existingPrimaryForests) {
129+
ForestHostNames forestHostNames = determineHostNamesForForest(context, existingPrimaryForests);
130+
131+
final String template = buildForestTemplate(context, new ForestManager(context.getManageClient()));
121132

122-
// Need to know what primary forests exist already in case more need to be added, or a new host has been added
123-
List<Forest> forests = getExistingPrimaryForests(context, this.databaseName);
124133
ForestPlan forestPlan = new ForestPlan(this.databaseName, forestHostNames.getPrimaryForestHostNames())
125134
.withReplicaHostNames(forestHostNames.getReplicaForestHostNames())
126135
.withTemplate(template)
127136
.withForestsPerDataDirectory(this.forestsPerHost)
128-
.withExistingForests(forests);
137+
.withExistingForests(existingPrimaryForests);
129138

130139
if (includeReplicas) {
131140
Map<String, Integer> map = context.getAppConfig().getDatabaseNamesAndReplicaCounts();
@@ -182,12 +191,12 @@ protected String buildForestTemplate(CommandContext context, ForestManager fores
182191

183192
/**
184193
* @param context
185-
* @param allHostNames
194+
* @param existingPrimaryForests
186195
* @return a ForestHostNames instance that defines the list of host names that can be used for primary forests and
187196
* that can be used for replica forests. As of 4.1.0, the only reason these will differ is when a database is
188197
* configured to only have forests on one host, or when the deprecated setCreateForestsOnOneHost method is used.
189198
*/
190-
protected ForestHostNames determineHostNamesForForest(CommandContext context, final List<String> allHostNames) {
199+
protected ForestHostNames determineHostNamesForForest(CommandContext context, List<Forest> existingPrimaryForests) {
191200
Set<String> databaseNames = context.getAppConfig().getDatabasesWithForestsOnOneHost();
192201
boolean onlyOnOneHost = databaseNames != null && databaseNames.contains(this.databaseName);
193202

@@ -200,9 +209,16 @@ protected ForestHostNames determineHostNamesForForest(CommandContext context, fi
200209
List<String> primaryForestHostNames = new ArrayList<>();
201210

202211
if (!createForestsOnEachHost || onlyOnOneHost) {
203-
String first = allHostNames.get(0);
204-
logger.info(format("Only creating forests on the first host: " + first));
205-
primaryForestHostNames.add(first);
212+
// If the database should only have forests on one host and such forests already exist, then use the host
213+
// that the forests exist on
214+
if (!existingPrimaryForests.isEmpty()) {
215+
primaryForestHostNames.add(existingPrimaryForests.get(0).getHost());
216+
}
217+
else {
218+
String first = candidateHostNames.get(0);
219+
logger.info(format("Only creating forests on the first host: " + first));
220+
primaryForestHostNames.add(first);
221+
}
206222
} else {
207223
primaryForestHostNames.addAll(candidateHostNames);
208224
}

‎src/main/java/com/marklogic/mgmt/api/forest/Forest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ public Forest(API api, String forestName) {
6767
setForestName(forestName);
6868
}
6969

70+
public Forest(String host, String forestName) {
71+
setHost(host);
72+
setForestName(forestName);
73+
}
74+
7075
@Override
7176
protected String getResourceLabel() {
7277
return getForestName();

‎src/test/java/com/marklogic/appdeployer/command/forests/CreateForestsOnOneHostTest.java‎

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
package com.marklogic.appdeployer.command.forests;
22

33
import com.marklogic.appdeployer.AppConfig;
4+
import com.marklogic.appdeployer.DefaultAppConfigFactory;
45
import com.marklogic.appdeployer.command.CommandContext;
5-
import static org.junit.jupiter.api.Assertions.*;
6+
import com.marklogic.mgmt.api.forest.Forest;
7+
import com.marklogic.mgmt.util.SimplePropertySource;
68
import org.junit.jupiter.api.Test;
79

8-
import java.util.ArrayList;
9-
import java.util.HashSet;
10-
import java.util.List;
11-
import java.util.Set;
10+
import java.util.*;
1211

13-
public class CreateForestsOnOneHostTest {
12+
import static org.junit.jupiter.api.Assertions.assertEquals;
13+
14+
public class CreateForestsOnOneHostTest {
1415

1516
@Test
1617
public void test() {
@@ -19,34 +20,83 @@ public void test() {
1920

2021
DeployForestsCommand command = new DeployForestsCommand("test-db");
2122

22-
List<String> fakeHostNames = new ArrayList<>();
23-
fakeHostNames.add("host1");
24-
fakeHostNames.add("host2");
25-
fakeHostNames.add("host3");
26-
2723
HostCalculator hostCalculator = new DefaultHostCalculator(new TestHostNameProvider("host1", "host2", "host3"));
2824
command.setHostCalculator(hostCalculator);
2925

30-
ForestHostNames hostNames = command.determineHostNamesForForest(context, fakeHostNames);
26+
ForestHostNames hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
3127
assertEquals(3, hostNames.getPrimaryForestHostNames().size());
3228

3329
command.setCreateForestsOnEachHost(false);
34-
hostNames = command.determineHostNamesForForest(context, fakeHostNames);
30+
hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
3531
assertEquals(1, hostNames.getPrimaryForestHostNames().size());
3632
assertEquals("host1", hostNames.getPrimaryForestHostNames().get(0));
3733
assertEquals(3, hostNames.getReplicaForestHostNames().size(),
3834
"When forests aren't created on each host, all hosts should still be available for replica forests, " +
3935
"with the expectation that the primary forest host will still not be used");
4036

4137
command.setCreateForestsOnEachHost(true);
42-
hostNames = command.determineHostNamesForForest(context, fakeHostNames);
38+
hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
4339
assertEquals(3, hostNames.getPrimaryForestHostNames().size());
4440

4541
Set<String> names = new HashSet<>();
4642
names.add("test-db");
4743
appConfig.setDatabasesWithForestsOnOneHost(names);
48-
hostNames = command.determineHostNamesForForest(context, fakeHostNames);
44+
hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
4945
assertEquals(1, hostNames.getPrimaryForestHostNames().size());
5046
assertEquals(3, hostNames.getReplicaForestHostNames().size());
5147
}
48+
49+
/**
50+
* Added for ticket #439, where the wrong host was being selected when forests already exist on a host that isn'
51+
* the first one in the list.
52+
*/
53+
@Test
54+
void secondHostAlreadyHasForests() {
55+
DeployForestsCommand command = new DeployForestsCommand("test-db");
56+
command.setHostCalculator(new DefaultHostCalculator(new TestHostNameProvider("host1", "host2", "host3")));
57+
58+
AppConfig appConfig = new DefaultAppConfigFactory(new SimplePropertySource(
59+
"mlDatabasesWithForestsOnOneHost", "test-db",
60+
"mlForestsPerHost", "test-db,2"
61+
)).newAppConfig();
62+
63+
CommandContext context = new CommandContext(appConfig, null, null);
64+
65+
final List<Forest> existingForests = Arrays.asList(new Forest("host2", "test-db-1"));
66+
ForestHostNames hostNames = command.determineHostNamesForForest(context, existingForests);
67+
68+
assertEquals(1, hostNames.getPrimaryForestHostNames().size(), "Only one host name should exist since the " +
69+
"database should only have forests on one host");
70+
assertEquals("host2", hostNames.getPrimaryForestHostNames().get(0), "Because there's already a forest on " +
71+
"host2, that should be the primary forest host name, even though host1 is first in the list");
72+
73+
List<Forest> forestsToCreate = command.buildForests(context, false, existingForests);
74+
assertEquals(1, forestsToCreate.size(), "Because forests per host is 2 and host2 already has 1 forest, 1 more " +
75+
"forest needs to be created");
76+
assertEquals("host2", forestsToCreate.get(0).getHost());
77+
assertEquals("test-db-2", forestsToCreate.get(0).getForestName());
78+
}
79+
80+
@Test
81+
void forestsShouldGoOnHostInSecondGroup() {
82+
DeployForestsCommand command = new DeployForestsCommand("test-db");
83+
TestHostNameProvider hostNameProvider = new TestHostNameProvider();
84+
hostNameProvider.addGroupHostNames("group1", "host1-1", "host1-2");
85+
hostNameProvider.addGroupHostNames("group2", "host2-1", "host2-2");
86+
command.setHostCalculator(new DefaultHostCalculator(hostNameProvider));
87+
88+
AppConfig appConfig = new DefaultAppConfigFactory(new SimplePropertySource(
89+
"mlDatabasesWithForestsOnOneHost", "test-db",
90+
"mlDatabaseGroups", "test-db,group2"
91+
)).newAppConfig();
92+
93+
CommandContext context = new CommandContext(appConfig, null, null);
94+
95+
ForestHostNames hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
96+
assertEquals(1, hostNames.getPrimaryForestHostNames().size());
97+
assertEquals("host2-1", hostNames.getPrimaryForestHostNames().get(0), "mlDatabaseGroups says that the " +
98+
"database should only have forests on hosts in group2, so host2-1 - the first host in group2 - should " +
99+
"be selected");
100+
}
101+
52102
}

‎src/test/java/com/marklogic/appdeployer/command/forests/CreateForestsOnSpecificHostsTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void test() {
2626

2727
command.setHostCalculator(new DefaultHostCalculator(new TestHostNameProvider(fakeHostNames.toArray(new String[]{}))));
2828
// Verify we get all 3 hosts back when nothing special is configured
29-
ForestHostNames hostNames = command.determineHostNamesForForest(context, fakeHostNames);
29+
ForestHostNames hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
3030
assertEquals(3, hostNames.getPrimaryForestHostNames().size());
3131
assertEquals(3, hostNames.getReplicaForestHostNames().size());
3232

@@ -37,7 +37,7 @@ public void test() {
3737
appConfig = factory.newAppConfig();
3838
context = new CommandContext(appConfig, null, null);
3939

40-
hostNames = command.determineHostNamesForForest(context, fakeHostNames);
40+
hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
4141
assertEquals(2, hostNames.getPrimaryForestHostNames().size());
4242
assertEquals(2, hostNames.getReplicaForestHostNames().size());
4343
assertTrue(hostNames.getPrimaryForestHostNames().contains("host1"));
@@ -47,7 +47,7 @@ public void test() {
4747
props.setProperty("mlDatabaseHosts", "some-other-db,host2,test-db,bad-host|host2");
4848
appConfig = factory.newAppConfig();
4949
context = new CommandContext(appConfig, null, null);
50-
hostNames = command.determineHostNamesForForest(context, fakeHostNames);
50+
hostNames = command.determineHostNamesForForest(context, new ArrayList<>());
5151
assertEquals(1, hostNames.getPrimaryForestHostNames().size());
5252
assertEquals(1, hostNames.getReplicaForestHostNames().size());
5353
assertTrue(hostNames.getPrimaryForestHostNames().contains("host2"));

0 commit comments

Comments
 (0)