[
https://issues.apache.org/jira/browse/GEODE-8745?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17246767#comment-17246767
]
ASF GitHub Bot commented on GEODE-8745:
---------------------------------------
DonalEvans commented on a change in pull request #5821:
URL: https://github.com/apache/geode/pull/5821#discussion_r539569203
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java
##########
@@ -638,6 +639,17 @@ protected boolean basicHandlePrimaryDestroy(final EventID
eventId) {
ew.event.release();
statistics.incUnprocessedEventsRemovedByPrimary();
return true;
+ } else if (addToUnprocessedTokens) {
+ // Secondary event may not have arrived
+ Long mapValue =
+ Long.valueOf(System.currentTimeMillis() +
AbstractGatewaySender.TOKEN_TIMEOUT);
+ Long oldv = this.unprocessedTokens.put(eventId, mapValue);
+ if (oldv == null) {
+ statistics.incUnprocessedTokensAddedByPrimary();
+ } else {
+ // its ok for oldv to be non-null
+ // this shouldn't happen anymore @todo add an assertion here
+ }
Review comment:
This empty block can probably be replaced with just a comment.
##########
File path:
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANPersistenceEnabledGatewaySenderDUnitTest.java
##########
@@ -605,26 +603,21 @@ public void
testReplicatedRegionPersistentWanGateway_restartSenderWithCleanQueue
vm7.invoke(() ->
WANTestBase.createPersistentReplicatedRegion(getTestMethodName() + "_RR", "ln",
isOffHeap()));
- vm4.invoke(() -> WANTestBase.pauseSender("ln"));
- vm5.invoke(() -> WANTestBase.pauseSender("ln"));
-
- vm4.invoke(() -> WANTestBase.doPuts(getTestMethodName() + "_RR", 1000));
-
- logger.info("Completed puts in the region");
+ vm4.invoke("Puts in the region" + getTestMethodName() + "_RR",
+ () -> WANTestBase.doPuts(getTestMethodName() + "_RR", 1000));
- vm4.invoke(() -> WANTestBase.stopSender("ln"));
- vm5.invoke(() -> WANTestBase.stopSender("ln"));
+ vm4.invoke("Stopping ln sender", () -> WANTestBase.stopSender("ln"));
+ vm5.invoke("Stopping ln sender", () -> WANTestBase.stopSender("ln"));
- logger.info("Stopped all the senders. ");
-
- AsyncInvocation inv1 = vm4.invokeAsync(() ->
WANTestBase.startSenderwithCleanQueues("ln"));
- logger.info("Started the sender in vm 4");
+ createReceiverInVMs(vm2, vm3);
- vm5.invoke(() -> WANTestBase.startSenderwithCleanQueues("ln"));
- logger.info("Started the sender in vm 5");
+ AsyncInvocation inv1 = vm4.invokeAsync("Starting sender with clean queues",
Review comment:
The compiler warning here can be fixed by using `AsyncInvocation<?>
inv1`.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java
##########
@@ -638,6 +639,17 @@ protected boolean basicHandlePrimaryDestroy(final EventID
eventId) {
ew.event.release();
statistics.incUnprocessedEventsRemovedByPrimary();
return true;
+ } else if (addToUnprocessedTokens) {
+ // Secondary event may not have arrived
+ Long mapValue =
+ Long.valueOf(System.currentTimeMillis() +
AbstractGatewaySender.TOKEN_TIMEOUT);
Review comment:
This can just be `Long mapValue = System.currentTimeMillis() +
AbstractGatewaySender.TOKEN_TIMEOUT;`
##########
File path:
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANPersistenceEnabledGatewaySenderDUnitTest.java
##########
@@ -577,17 +577,15 @@ public void
testReplicatedRegionPersistentWanGateway_restartSenderWithCleanQueue
Integer nyPort = (Integer) vm1.invoke(() ->
WANTestBase.createFirstRemoteLocator(2, lnPort));
createCacheInVMs(nyPort, vm2, vm3);
- createReceiverInVMs(vm2, vm3);
createCacheInVMs(lnPort, vm4, vm5, vm6, vm7);
- String firstDStore = (String) vm4.invoke(() ->
WANTestBase.createSenderWithDiskStore("ln", 2,
- false, 100, 10, false, true, null, null, true));
- String secondDStore = (String) vm5.invoke(() ->
WANTestBase.createSenderWithDiskStore("ln", 2,
- false, 100, 10, false, true, null, null, true));
-
- logger.info("The first ds is " + firstDStore);
- logger.info("The second ds is " + secondDStore);
+ String firstDStore =
+ (String) vm4.invoke("Creating DS", () ->
WANTestBase.createSenderWithDiskStore("ln", 2,
+ false, 100, 10, false, true, null, null, true));
+ String secondDStore =
+ (String) vm5.invoke("Creating DS", () ->
WANTestBase.createSenderWithDiskStore("ln", 2,
+ false, 100, 10, false, true, null, null, true));
Review comment:
These variables are never used, so I think it should be okay to change
these to just
```
vm4.invoke("Creating DS", () -> WANTestBase.createSenderWithDiskStore("ln",
2, false, 100, 10,
false, true, null, null, true));
vm5.invoke("Creating DS", () -> WANTestBase.createSenderWithDiskStore("ln",
2, false, 100, 10,
false, true, null, null, true));
```
----------------------------------------------------------------
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
> Closing the region backing the queue when the serial gateway sender is
> stopped.
> -------------------------------------------------------------------------------
>
> Key: GEODE-8745
> URL: https://issues.apache.org/jira/browse/GEODE-8745
> Project: Geode
> Issue Type: Task
> Components: wan
> Reporter: Nabarun Nag
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.14.0
>
>
> In the commit for GEODE-7458, when the sender is stopped, the region backing
> the queues are no more closed, but just remove the cache listeners.
> This is causing a problem, as the regions continue to exist, it keeps on
> storing entry events and hence the queue size never gets to zero.
> Also, as the region exists but before attaching the cache listener when
> restarting the sender leads to entries being never removed from the
> unprocessed event map.
>
> As mention in the PR for GEODE-7458 - "This option is only applicable for
> Gateway Senders with enabled persistence."
> Hence believe that it is ok to close the region as the disk files will still
> be maintained. so when we restart the values can be obtained back from the
> disk stores.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
