You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/04/05 16:09:29 UTC
[GitHub] [geode] mreddington opened a new pull request, #7554: GEM-3124: Guarding membership addition code paths to omit membership …
mreddington opened a new pull request, #7554:
URL: https://github.com/apache/geode/pull/7554
This is a draft for Hydra testing.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on code in PR #7554:
URL: https://github.com/apache/geode/pull/7554#discussion_r857833106
##########
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:
Yeah, I do the same. Or if the comment doesn't provide value delete it or rewrite it.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao commented on pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
jinmeiliao commented on PR #7554:
URL: https://github.com/apache/geode/pull/7554#issuecomment-1115273126
closing this, work continues with #7639
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mhansonp commented on a diff in pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
mhansonp commented on code in PR #7554:
URL: https://github.com/apache/geode/pull/7554#discussion_r854391131
##########
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:
Does this sleep still need to be there? It was there when we were trying to sort out the issue, but it is it required?
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a diff in pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
kirklund commented on code in PR #7554:
URL: https://github.com/apache/geode/pull/7554#discussion_r857808320
##########
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:
I usually take those trailing comments and move them above the line they are commenting, so that's what I recommend doing here.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao closed pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
jinmeiliao closed pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
URL: https://github.com/apache/geode/pull/7554
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mhansonp commented on a diff in pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
mhansonp commented on code in PR #7554:
URL: https://github.com/apache/geode/pull/7554#discussion_r854391494
##########
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);
+
+ gfshRule.execute("connect --locator=localhost[" + locatorPort + "]", createRegionCommand);
+
+ gfshRule.execute("connect --locator=localhost[" + locatorPort + "]",
+ "put --key=\"key1\" --value=\"value1\" --region=\"" + REGION_NAME + "\"");
+ gfshRule.execute("connect --locator=localhost[" + locatorPort + "]",
+ "get --key=\"key1\" --region=\"" + REGION_NAME + "\"");
Review Comment:
These lines can be deleted.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mhansonp commented on a diff in pull request #7554: GEM-3124: Guarding membership addition code paths to omit membership …
Posted by GitBox <gi...@apache.org>.
mhansonp commented on code in PR #7554:
URL: https://github.com/apache/geode/pull/7554#discussion_r854389384
##########
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java:
##########
@@ -2345,7 +2368,9 @@ public void newMemberConnected(InternalDistributedMember member) {
// subsequent deadlock (#45566). Elder selection is now done when a view
// is installed.
try {
+ // if (!dm.getViewMembers().contains(member)) {
Review Comment:
probably should remove this test code.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org