[GitHub] [geode] DonalEvans commented on a change in pull request #5777: GEODE-8742: fix list gateways command when dispatcher-threads is set …

[GitBox <gi...@apache.org>]

DonalEvans commented on a change in pull request #5777:
URL: https://github.com/apache/geode/pull/5777#discussion_r533686032



##########
File path: geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/wancommand/ListGatewaysCommandDUnitTest.java
##########
@@ -450,6 +450,31 @@ public void testListGatewaysReceiversOnlyFalseAndSendersOnlyFalseReturnsSendersA
         .hasRowSize(expectedGwReceiverSectionSize).hasColumns().contains("Port", "Member");
   }
 
+  @Test
+  public void testListGatewaysWithOneDispatcherThread() {
+    String command =
+        "create gateway-sender --id=ln_Serial --remote-distributed-system-id=2 --dispatcher-threads=1";
+
+    int lnPort = locatorSite1.getPort();
+    int nyPort = locatorSite2.getPort();

Review comment:
       This variable is never used.

##########
File path: geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueueDUnitTest.java
##########
@@ -110,18 +111,18 @@ public void unprocessedTokensMapShouldDrainCompletely() throws Exception {
     // Give the unprocessedTokens map time to empty
     Thread.sleep(2000);
 
-    int numUnprocessedTokensVM4 = vm4.invoke(() -> unprocessedTokensSize("ln"));
+    long numUnprocessedTokensVM4 = vm4.invoke(() -> unprocessedTokensSize("ln"));
     assertThat(numUnprocessedTokensVM4).isEqualTo(0);
 
-    int numUnprocessedTokensVM5 = vm5.invoke(() -> unprocessedTokensSize("ln"));
+    long numUnprocessedTokensVM5 = vm5.invoke(() -> unprocessedTokensSize("ln"));
     assertThat(numUnprocessedTokensVM5).isEqualTo(0);
   }
 
-  private int unprocessedTokensSize(String senderId) {
+  private long unprocessedTokensSize(String senderId) {
     AbstractGatewaySender sender = (AbstractGatewaySender) findGatewaySender(senderId);
-    SerialGatewaySenderEventProcessor processor =
-        (SerialGatewaySenderEventProcessor) sender.getEventProcessor();
-    return processor.numUnprocessedEventTokens();
+    AbstractGatewaySenderEventProcessor processor =
+        sender.getEventProcessor();
+    return processor.getNumEventsDispatched();

Review comment:
       It appears that the name of this method no longer matches what it's doing. The method returns the number of events dispatched, not the number of unprocessed events. This also impacts tests that use this method, since they are now asserting something different.

##########
File path: geode-wan/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderImpl.java
##########
@@ -117,15 +117,8 @@ private void start(boolean cleanQueues) {
   }
 
   protected AbstractGatewaySenderEventProcessor createEventProcessor(boolean cleanQueues) {
-    AbstractGatewaySenderEventProcessor eventProcessor;
-    if (getDispatcherThreads() > 1) {
-      eventProcessor = new RemoteConcurrentSerialGatewaySenderEventProcessor(
-          SerialGatewaySenderImpl.this, getThreadMonitorObj(), cleanQueues);
-    } else {
-      eventProcessor = new RemoteSerialGatewaySenderEventProcessor(SerialGatewaySenderImpl.this,
-          getId(), getThreadMonitorObj(), cleanQueues);
-    }
-    return eventProcessor;
+    return new RemoteConcurrentSerialGatewaySenderEventProcessor(
+        SerialGatewaySenderImpl.this, getThreadMonitorObj(), cleanQueues);

Review comment:
       Would it be possible to address the underlying problem with using `RemoteSerialGatewaySenderEventProcessor` rather than simply not using it? There appears to be an NPE thrown when calling `GatewaySenderMXBean.isConnected()` on the `GatewaySenderMXBean` proxy in `ListGatewayCommand` when `dispatcher-threads=1`, which this change does not directly address. It would be better to address the root cause of this issue if possible rather than just avoiding it, I think.




----------------------------------------------------------------
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:
users@infra.apache.org