[ https://issues.apache.org/jira/browse/GEODE-8947?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17285511#comment-17285511 ]
ASF GitHub Bot commented on GEODE-8947: --------------------------------------- DonalEvans commented on a change in pull request #6038: URL: https://github.com/apache/geode/pull/6038#discussion_r577155873 ########## File path: geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemoteOperationMessage.java ########## @@ -177,7 +179,28 @@ public void process(final ClusterDistributionManager dm) { sendReply(getSender(), this.processorId, dm, replyException, null, 0); return; } - dm.getExecutors().getWaitingThreadPool().execute(() -> doRemoteOperation(dm, cache)); + + String conserveSockets = getConserveSocketsSetting(dm); + if (conserveSockets != null && conserveSockets.equals("false")) { + // reply inline for CONSERVE_SOCKETS == false case. + doRemoteOperation(dm, cache); + return; + } + + if (isTransactional()) { + dm.getExecutors().getWaitingThreadPool().execute(() -> doRemoteOperation(dm, cache)); + } else { + // reply inline for non-transactional case. + doRemoteOperation(dm, cache); + } + } + + String getConserveSocketsSetting(ClusterDistributionManager dm) { + return dm.getSystem().getProperties().getProperty(CONSERVE_SOCKETS); Review comment: I think it would be better to get the value of `conserve-sockets` from `DistributionConfigImpl` rather than check if a system property has been set, since a user may be using the default value. Since the default value of `conserve-sockets` was changed in 1.14, this would lead to a difference in behaviour when checking if the system property is present or not. If that's not possible, then a check should be included to see what the default value of `conserve-sockets` is (`DistributionConfig.DEFAULT_CONSERVE_SOCKETS`) if the system property is not set so that the correct action can be taken. ########## File path: geode-core/src/test/java/org/apache/geode/internal/cache/tx/RemoteOperationMessageTest.java ########## @@ -302,6 +303,52 @@ public void processWithNullPointerExceptionFromOperationOnRegionWithSystemFailur .hasMessageContaining("system failure"); } + + @Test + public void processInvokesDoRemoteOperationIfConserveSocketsIsFalse() { + doReturn("false").when(msg).getConserveSocketsSetting(dm); + doNothing().when(msg).doRemoteOperation(dm, cache); + + msg.process(dm); + + verify(msg).doRemoteOperation(dm, cache); + verify(msg, never()).isTransactional(); + } + + @Test + public void processInvokesDoRemoteOperationIfConserveSocketsIsTrueAndNotTransactional() { + doReturn("true").when(msg).getConserveSocketsSetting(dm); + doReturn(false).when(msg).isTransactional(); + doNothing().when(msg).doRemoteOperation(dm, cache); + + msg.process(dm); + + verify(msg).doRemoteOperation(dm, cache); + verify(msg).isTransactional(); + } + + @Test + public void isTransactionalReturnsFalseIfTXUniqueIdIsNOTX() { + assertThat(msg.getTXUniqId()).isEqualTo(TXManagerImpl.NOTX); + assertThat(msg.isTransactional()).isFalse(); + } + + @Test + public void isTransactionalReturnsTrueIfCannotParticipateInTransaction() { + doReturn(1).when(msg).getTXUniqId(); + doReturn(false).when(msg).canParticipateInTransaction(); + + assertThat(msg.isTransactional()).isFalse(); + } + + @Test + public void isTransactionalReturnsFalseIfHasTXUniqueIdAndCanParticipateInTransaction() { Review comment: This test name should be "isTransactionalReturnsTrueIfHasTXUniqueIdAndCanParticipateInTransaction" ########## File path: geode-core/src/test/java/org/apache/geode/internal/cache/tx/RemoteOperationMessageTest.java ########## @@ -302,6 +303,52 @@ public void processWithNullPointerExceptionFromOperationOnRegionWithSystemFailur .hasMessageContaining("system failure"); } + + @Test + public void processInvokesDoRemoteOperationIfConserveSocketsIsFalse() { + doReturn("false").when(msg).getConserveSocketsSetting(dm); + doNothing().when(msg).doRemoteOperation(dm, cache); + + msg.process(dm); + + verify(msg).doRemoteOperation(dm, cache); + verify(msg, never()).isTransactional(); + } + + @Test + public void processInvokesDoRemoteOperationIfConserveSocketsIsTrueAndNotTransactional() { + doReturn("true").when(msg).getConserveSocketsSetting(dm); + doReturn(false).when(msg).isTransactional(); + doNothing().when(msg).doRemoteOperation(dm, cache); + + msg.process(dm); + + verify(msg).doRemoteOperation(dm, cache); + verify(msg).isTransactional(); + } + + @Test + public void isTransactionalReturnsFalseIfTXUniqueIdIsNOTX() { + assertThat(msg.getTXUniqId()).isEqualTo(TXManagerImpl.NOTX); + assertThat(msg.isTransactional()).isFalse(); + } + + @Test + public void isTransactionalReturnsTrueIfCannotParticipateInTransaction() { Review comment: This test name should be "isTransactionalReturnsFalseIfCannotParticipateInTransaction" ---------------------------------------------------------------- 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 > Performance degradations due to GEODE-8930 > ------------------------------------------ > > Key: GEODE-8947 > URL: https://issues.apache.org/jira/browse/GEODE-8947 > Project: Geode > Issue Type: Bug > Affects Versions: 1.12.1, 1.13.2, 1.14.0, 1.15.0 > Reporter: Donal Evans > Assignee: Eric Shu > Priority: Major > Labels: pull-request-available > > Following the changes introduced by GEODE-8930, performance degradations were > seen in situations involving P2P puts with replicated regions. -- This message was sent by Atlassian Jira (v8.3.4#803005)