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:
[email protected]
