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