[ 
https://issues.apache.org/jira/browse/GEODE-7845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17218426#comment-17218426
 ] 

ASF GitHub Bot commented on GEODE-7845:
---------------------------------------

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


> Rollingupgrade should not conflict with the new ClearPRMessage 
> ---------------------------------------------------------------
>
>                 Key: GEODE-7845
>                 URL: https://issues.apache.org/jira/browse/GEODE-7845
>             Project: Geode
>          Issue Type: Improvement
>          Components: core
>            Reporter: Xiaojian Zhou
>            Assignee: Mark Hanson
>            Priority: Major
>              Labels: GeodeCommons, pull-request-available
>             Fix For: 1.14.0
>
>
> PartitionedRegion clear introduced a new ClearPRMessage. In case of doing 
> rolling upgrade, the user called PR.clear. The new ClearPRMessage should not 
> fail. It should not be sent to the old members. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to