You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/11/24 09:31:58 UTC
[GitHub] [geode] nabarunnag opened a new pull request #5770: GEM-2935: Closing the queue region when senders are stopped.
nabarunnag opened a new pull request #5770:
URL: https://github.com/apache/geode/pull/5770
* cleanQueues are applicable only while using persistence
* when we are closing the region, the disk files are not deleted.
* hence the values will still be maintained on restart.
* Advantage will be that we are creating the queue region and its cache listener together
* previously the region was not closed and cache listener was attached using mutators.
* this caused secondary events to be missed before the cache listener is activated
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
----------------------------------------------------------------
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
[GitHub] [geode] DonalEvans commented on a change in pull request #5770: GEODE-8745: Closing the queue region when senders are stopped.
Posted by GitBox <gi...@apache.org>.
DonalEvans commented on a change in pull request #5770:
URL: https://github.com/apache/geode/pull/5770#discussion_r532787287
##########
File path: geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANPersistenceEnabledGatewaySenderDUnitTest.java
##########
@@ -603,27 +605,32 @@ 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(() -> WANTestBase.stopSender("ln"));
vm5.invoke(() -> WANTestBase.stopSender("ln"));
- logger.info("Stopped all the senders. ");
- // Create receiver on remote site
- createReceiverInVMs(vm2, vm3);
+ logger.info("Stopped all the senders. ");
Review comment:
Instead of using the logger in this test to track progress, you could instead use the `invoke()` method signature that passes a String description/name as the first parameter, which allows anyone debugging the test to see when the invocation in question starts and finishes. Just personal preference, but I think it makes things neater and provides a little more information to anyone looking at the test output.
----------------------------------------------------------------
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
[GitHub] [geode] nabarunnag commented on a change in pull request #5770: GEODE-8745: Closing the queue region when senders are stopped.
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #5770:
URL: https://github.com/apache/geode/pull/5770#discussion_r533013553
##########
File path: geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANPersistenceEnabledGatewaySenderDUnitTest.java
##########
@@ -603,27 +605,32 @@ 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(() -> WANTestBase.stopSender("ln"));
vm5.invoke(() -> WANTestBase.stopSender("ln"));
- logger.info("Stopped all the senders. ");
- // Create receiver on remote site
- createReceiverInVMs(vm2, vm3);
+ logger.info("Stopped all the senders. ");
Review comment:
A valid point Donal. I am creating another GEODE ticket to just remove all the logger.info in tests. There are a couple of more statements that went in other commits.
----------------------------------------------------------------
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
[GitHub] [geode] nabarunnag merged pull request #5770: GEODE-8745: Closing the queue region when senders are stopped.
Posted by GitBox <gi...@apache.org>.
nabarunnag merged pull request #5770:
URL: https://github.com/apache/geode/pull/5770
----------------------------------------------------------------
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
[GitHub] [geode] nabarunnag commented on a change in pull request #5770: GEODE-8745: Closing the queue region when senders are stopped.
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #5770:
URL: https://github.com/apache/geode/pull/5770#discussion_r532776796
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java
##########
@@ -111,10 +111,9 @@ public SerialGatewaySenderEventProcessor(AbstractGatewaySender sender, String id
ThreadsMonitoring tMonitoring, boolean cleanQueues) {
super("Event Processor for GatewaySender_" + id, sender, tMonitoring);
+ initializeMessageQueue(id, cleanQueues);
Review comment:
This change is because the CacheListener to handle the unprocessedEventsMap is set here, hence I wanted the listener to be up and running before the map is initialized so the no events are missed because they were put in the map before the listener was running.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java
##########
@@ -111,10 +111,9 @@ public SerialGatewaySenderEventProcessor(AbstractGatewaySender sender, String id
ThreadsMonitoring tMonitoring, boolean cleanQueues) {
super("Event Processor for GatewaySender_" + id, sender, tMonitoring);
+ initializeMessageQueue(id, cleanQueues);
Review comment:
This change is because the CacheListener to handle the unprocessedEventsMap is set here, hence I wanted the listener to be up and running before the map is initialized so that no events are missed because they were put in the map before the listener was running.
----------------------------------------------------------------
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