[ https://issues.apache.org/jira/browse/GEODE-8918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17282126#comment-17282126 ]
ASF GitHub Bot commented on GEODE-8918: --------------------------------------- kirklund commented on a change in pull request #6008: URL: https://github.com/apache/geode/pull/6008#discussion_r573344449 ########## File path: geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java ########## @@ -741,8 +741,8 @@ public boolean isPdxTypesRegion() { // This is for all regions except pdx Region if (!isPdxTypesRegion) { // Make sure we are distributing to only those senders whose id - // is available on this region - if (allGatewaySenderIds.contains(sender.getId())) { + // is available on this region and whose state is running + if (allGatewaySenderIds.contains(sender.getId()) && sender.isRunning()) { Review comment: Please consider writing a unit test for any code that only has integration/distributed test coverage. The following example is using a basic technique from Working Effectively with Legacy Code. For example, you could create a new method called: ``` @VisibleForTesting static boolean hasRunningGatewaySender(Set<GatewaySender> senders, GatewaySender sender) { return senders.contains(sender) && sender.isRunning(); } ``` And then you can add unit tests for this new method to `AbstractRegionTest`: ``` import java.util.Collections; import org.apache.geode.cache.wan.GatewaySender; @Test public void hasRunningGatewaySender_returnsFalse_ifSendersIsEmpty() { GatewaySender sender = mock(GatewaySender.class); boolean value = AbstractRegion.hasRunningGatewaySender(Collections.emptySet(), sender); assertThat(value).isFalse(); } ``` You can also write it as a non-static method, but the unit tests will require a bit more setUp. ---------------------------------------------------------------- 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 > Geode replication event forwarding does not honor GW sender state > ----------------------------------------------------------------- > > Key: GEODE-8918 > URL: https://issues.apache.org/jira/browse/GEODE-8918 > Project: Geode > Issue Type: Bug > Components: wan > Reporter: Mario Kevo > Assignee: Mario Kevo > Priority: Major > Labels: pull-request-available > > {color:#172b4d}With 3+ geo-red systems Geode replication has the forwarding > feature which means that receiving cluster will forward the event it just got > to all clusters it is connected to that have not yet received the > event.{color} > {color:#172b4d}This is possible because the originating cluster is setting > metadata in the replication event like this: > GatewaySenderEventCallbackArgument > [originalCallbackArg=null;originatingSenderId=1;recipientGatewayReceivers= > {color}{color:#172b4d}{3, 2}{color}{color:#172b4d}] > > Site receiving this event thus knows which is the originating site and which > sites should have received this event. All others will have this event > forwarded to. All this is legacy Geode behavior. > > However, originating site does not care if GW sender to a destination is > stopped or not - only the fact GW sender is *created and attached* to a > region is enough. This means if e.g. GW sender from Site1 to Site 3 is > stopped (and has been stopped for a while - so this has nothing to do with > timing) at the moment an event hits the replication it is only going to be > sent to Site 2 *but with the same metadata*_._ Hence Site 2 will not forward > to Site 3 (assuming it has a connection to it).{color} -- This message was sent by Atlassian Jira (v8.3.4#803005)