pivotal-jbarrett commented on code in PR #7554:
URL: https://github.com/apache/geode/pull/7554#discussion_r855531377
##########
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java:
##########
@@ -227,7 +248,9 @@ public class ClusterDistributionManager implements
DistributionManager {
private Map<InternalDistributedMember, Collection<String>>
hostedLocatorsWithSharedConfiguration =
Collections.emptyMap();
- /** a map keyed on InternalDistributedMember, to direct channels to other
systems */
+ /**
+ * a map keyed on InternalDistributedMember, to direct channels to other
systems
+ */
// protected final Map channelMap = CFactory.createCM();
Review Comment:
Cleanup of javadoc on a commented out member, please delete the member and
javadoc.
##########
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java:
##########
@@ -353,7 +380,9 @@ static ClusterDistributionManager
create(InternalDistributedSystem system,
}
}
}
+ // if (!system.getDistributionManager().getViewMembers().contains(id))
{
Review Comment:
Delete commented out code.
##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/persistence/MissingDiskStoreAcceptanceTest.java:
##########
@@ -70,40 +70,60 @@ public void setUp() throws Exception {
server1Folder =
temporaryFolder.newFolder(SERVER_1_NAME).toPath().toAbsolutePath();
server2Folder =
temporaryFolder.newFolder(SERVER_2_NAME).toPath().toAbsolutePath();
- int[] ports = getRandomAvailableTCPPorts(3);
+ int[] ports = getRandomAvailableTCPPorts(9);
locatorPort = ports[0];
int server1Port = ports[1];
int server2Port = ports[2];
+ int httpPort1 = ports[3];
+ int httpPort2 = ports[4];
+ int httpPort3 = ports[5];
+ int jmxPort1 = ports[6];
+ int jmxPort2 = ports[7];
+ int jmxPort3 = ports[8];
String startLocatorCommand = String.join(" ",
"start locator",
"--name=" + LOCATOR_NAME,
"--dir=" + locatorFolder,
"--port=" + locatorPort,
+ "--http-service-port=" + httpPort1,
+ "--J=-Dgemfire.jmx-manager-port=" + jmxPort1,
"--locators=localhost[" + locatorPort + "]");
startServer1Command = String.join(" ",
"start server",
"--name=" + SERVER_1_NAME,
"--dir=" + server1Folder,
"--locators=localhost[" + locatorPort + "]",
+ "--http-service-port=" + httpPort2,
+ "--J=-Dgemfire.jmx-manager-port=" + jmxPort2,
"--server-port=" + server1Port);
startServer2Command = String.join(" ",
"start server",
"--name=" + SERVER_2_NAME,
"--dir=" + server2Folder,
"--locators=localhost[" + locatorPort + "]",
+ "--http-service-port=" + httpPort3,
Review Comment:
Does this test really need to start the http service on the servers?
##########
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java:
##########
@@ -958,13 +986,13 @@ public Collection<String>
getHostedLocators(InternalDistributedMember member) {
* Returns a copy of the map of all members hosting locators. The key is the
member, and the value
* is a collection of host[port] strings. If a bind-address was used for a
locator then the form
* is bind-addr[port].
- *
+ * <p>
Review Comment:
Is it possible this PR is conflicting with one recently to cleanup Javadocs?
##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/persistence/MissingDiskStoreAcceptanceTest.java:
##########
@@ -70,40 +70,60 @@ public void setUp() throws Exception {
server1Folder =
temporaryFolder.newFolder(SERVER_1_NAME).toPath().toAbsolutePath();
server2Folder =
temporaryFolder.newFolder(SERVER_2_NAME).toPath().toAbsolutePath();
- int[] ports = getRandomAvailableTCPPorts(3);
+ int[] ports = getRandomAvailableTCPPorts(9);
locatorPort = ports[0];
int server1Port = ports[1];
int server2Port = ports[2];
+ int httpPort1 = ports[3];
+ int httpPort2 = ports[4];
+ int httpPort3 = ports[5];
+ int jmxPort1 = ports[6];
+ int jmxPort2 = ports[7];
+ int jmxPort3 = ports[8];
String startLocatorCommand = String.join(" ",
"start locator",
"--name=" + LOCATOR_NAME,
"--dir=" + locatorFolder,
"--port=" + locatorPort,
+ "--http-service-port=" + httpPort1,
+ "--J=-Dgemfire.jmx-manager-port=" + jmxPort1,
"--locators=localhost[" + locatorPort + "]");
startServer1Command = String.join(" ",
"start server",
"--name=" + SERVER_1_NAME,
"--dir=" + server1Folder,
"--locators=localhost[" + locatorPort + "]",
+ "--http-service-port=" + httpPort2,
+ "--J=-Dgemfire.jmx-manager-port=" + jmxPort2,
"--server-port=" + server1Port);
startServer2Command = String.join(" ",
"start server",
"--name=" + SERVER_2_NAME,
"--dir=" + server2Folder,
"--locators=localhost[" + locatorPort + "]",
+ "--http-service-port=" + httpPort3,
+ "--J=-Dgemfire.jmx-manager-port=" + jmxPort3,
"--server-port=" + server2Port);
String createRegionCommand = String.join(" ",
"create region",
"--name=" + REGION_NAME,
"--type=REPLICATE_PERSISTENT");
- gfshRule.execute(startLocatorCommand, startServer1Command,
startServer2Command,
- createRegionCommand);
+ gfshRule.execute(startLocatorCommand);
+ gfshRule.execute(startServer1Command, startServer2Command);
+ Thread.sleep(10000);
Review Comment:
Would be better to `await` until we can assert that the server is started.
Arbitrary sleeps make for slower and flakier tests.
##########
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java:
##########
@@ -2373,7 +2398,7 @@ public void memberDeparted(InternalDistributedMember
theId, boolean crashed, Str
message.setCrashed(crashed);
message.setAlertListenerExpected(true);
message.setIgnoreAlertListenerRemovalFailure(true); // we don't know
if it was a listener
- // so
+ // so
Review Comment:
Cleanup wacky comment format please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
