kirklund commented on a change in pull request #5645:
URL: https://github.com/apache/geode/pull/5645#discussion_r509462790
##########
File path:
geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradePartitionRegionClearServerVersionMismatch.java
##########
@@ -109,20 +111,28 @@ public void before() {
region.put("A", "ValueA");
region.put("B", "ValueB");
});
+ }
+
+ @After
+ public void after() {
+ locator.stop();
+ serverNew.stop();
+ serverOld.stop();
}
/**
- * testClient_ServerVersionMismatchException - validates that when a client
invokes a partitioned
- * region clear on a cluster where one server is running an unsupported
version for this feature
- * we return a ServerVersionMismatchException
+ * testClient_ServerOperationException - validates that when a client
invokes a partitioned region
+ * clear on a cluster where one server is running an unsupported version for
this feature we
+ * return a ServerOperationException
*/
@Test
- public void testClient_ServerVersionMismatchException() throws Exception {
+ public void testClient_ServerOperationExceptionCurrentServerVersion() throws
Exception {
IgnoredException.addIgnoredException(ServerOperationException.class);
- final int locatorPort = locator.getPort();
+
// Get a client VM
- ClientVM clientVM = cluster.startClientVM(3, c ->
c.withLocatorConnection(locatorPort));
+ int serverPort = serverNew.getPort();
+ clientVM = cluster.startClientVM(3, oldVersion, c ->
c.withServerConnection(serverPort));
Review comment:
I think Gester is recommending expanding the testing to include:
1. new client --> new server
2. new client --> old server
3. old client --> new server
4. old client --> old server
If (3) and (4) throw UnsupportedOperationException without sending any
message to the server, then they could probably be collapsed into a single test.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionClear.java
##########
@@ -349,13 +348,14 @@ public void
allServerVersionsSupportPartitionRegionClear() {
&&
(internalDistributedMember.getVersion().isOlderThan(KnownVersion.GEODE_1_14_0)))
{
if (!memberNames.contains(internalDistributedMember.getName())) {
memberNames.add(internalDistributedMember.getName());
- logger.info("MLH adding " + internalDistributedMember.getName());
}
}
}
if (!memberNames.isEmpty()) {
- throw new ServerVersionMismatchException(memberNames, "Partitioned
Region Clear",
- KnownVersion.GEODE_1_14_0.toString());
+ throw new UnsupportedOperationException(
+ "A server's " + memberNames + " version was too old (< "
Review comment:
This message reads weird. Maybe change to:
```
"Server(s) " + memberNames + " are running a version older than (<" +
KnownVersion.GEODE_1_14_0 + ">) which does not support partitioned region
clear.");
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
