You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2020/04/30 16:53:58 UTC
[geode] branch feature/GEODE-8020revert created (now 22cc6b5)
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a change to branch feature/GEODE-8020revert
in repository https://gitbox.apache.org/repos/asf/geode.git.
at 22cc6b5 Revert "GEODE-8020: buffer corruption in SSL communications (#4994)"
This branch includes the following new commits:
new 22cc6b5 Revert "GEODE-8020: buffer corruption in SSL communications (#4994)"
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[geode] 01/01: Revert "GEODE-8020: buffer corruption in SSL
communications (#4994)"
Posted by bs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch feature/GEODE-8020revert
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 22cc6b5c8200069d2282ca296af15f8025af2821
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Thu Apr 30 09:51:45 2020 -0700
Revert "GEODE-8020: buffer corruption in SSL communications (#4994)"
This reverts commit ec8db54ad7f342542762beb8f3e912dff44e3a53.
The fix for GEODE-8020 reduced performance in SSL benchmark tests.
Unfortunately this revert leaves SSL communications broken, but there
are no CI tests that show that this is the case.
---
.../geode/ClusterCommunicationsDUnitTest.java | 33 +---------------------
.../apache/geode/internal/net/NioSslEngine.java | 4 +--
.../org/apache/geode/internal/tcp/Connection.java | 16 ++++-------
.../geode/internal/net/NioSslEngineTest.java | 5 ----
4 files changed, 9 insertions(+), 49 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/ClusterCommunicationsDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/ClusterCommunicationsDUnitTest.java
index 09f94e5..3ae79fd 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/ClusterCommunicationsDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/ClusterCommunicationsDUnitTest.java
@@ -43,8 +43,6 @@ import java.io.DataOutput;
import java.io.File;
import java.io.IOException;
import java.io.Serializable;
-import java.lang.reflect.Field;
-import java.nio.Buffer;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -71,7 +69,6 @@ import org.apache.geode.distributed.Locator;
import org.apache.geode.distributed.internal.ClusterDistributionManager;
import org.apache.geode.distributed.internal.DMStats;
import org.apache.geode.distributed.internal.DirectReplyProcessor;
-import org.apache.geode.distributed.internal.DistributionImpl;
import org.apache.geode.distributed.internal.DistributionMessage;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
import org.apache.geode.distributed.internal.MessageWithReply;
@@ -79,15 +76,12 @@ import org.apache.geode.distributed.internal.OperationExecutors;
import org.apache.geode.distributed.internal.ReplyException;
import org.apache.geode.distributed.internal.ReplyMessage;
import org.apache.geode.distributed.internal.SerialAckedMessage;
-import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.distributed.internal.membership.gms.membership.GMSJoinLeave;
import org.apache.geode.internal.AvailablePortHelper;
import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.cache.DirectReplyMessage;
-import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.serialization.DeserializationContext;
import org.apache.geode.internal.serialization.SerializationContext;
-import org.apache.geode.internal.tcp.Connection;
import org.apache.geode.test.dunit.DistributedTestUtils;
import org.apache.geode.test.dunit.Host;
import org.apache.geode.test.dunit.VM;
@@ -149,7 +143,7 @@ public class ClusterCommunicationsDUnitTest implements Serializable {
}
@Test
- public void createEntryAndVerifyUpdate() throws Exception {
+ public void createEntryAndVerifyUpdate() {
int locatorPort = createLocator(getVM(0));
for (int i = 1; i <= NUM_SERVERS; i++) {
createCacheAndRegion(getVM(i), locatorPort);
@@ -163,9 +157,6 @@ public class ClusterCommunicationsDUnitTest implements Serializable {
}
for (int i = 1; i <= NUM_SERVERS; i++) {
verifyUpdatedEntry(getVM(i));
- if (!disableTcp) {
- verifyBufferType(getVM(i));
- }
}
}
@@ -254,28 +245,6 @@ public class ClusterCommunicationsDUnitTest implements Serializable {
});
}
- private void verifyBufferType(VM vm) throws Exception {
- vm.invoke("verify connection type", () -> {
- assertThat(cache).isNotNull();
- InternalCache internalCache = (InternalCache) cache;
- final DistributionImpl distribution =
- (DistributionImpl) internalCache.getDistributionManager().getDistribution();
- InternalDistributedMember locatorMember =
- (InternalDistributedMember) distribution.getCoordinator();
- final Connection connection =
- distribution.getDirectChannel().getConduit().getConnection(locatorMember, false, false,
- System.currentTimeMillis(), 15000, 0);
- Field inputBufferField = Connection.class.getDeclaredField("inputBuffer");
- inputBufferField.setAccessible(true);
- Buffer inputBuffer = (Buffer) inputBufferField.get(connection);
- // SSL connections use heap buffers for decoding efficiency. Non-SSL connections use
- // direct buffers since they don't need to be accessed as much
- assertThat(inputBuffer.isDirect()).isNotEqualTo(useSSL);
- });
- }
-
-
-
private void performCreate(VM memberVM) {
memberVM.invoke("perform create", () -> cache
.getRegion(regionName).put("testKey", "testValue"));
diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java b/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java
index 424e53a..2d55fa3 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java
@@ -74,7 +74,7 @@ public class NioSslEngine implements NioFilter {
int packetBufferSize = engine.getSession().getPacketBufferSize();
this.engine = engine;
this.bufferPool = bufferPool;
- this.myNetData = bufferPool.acquireNonDirectSenderBuffer(packetBufferSize);
+ this.myNetData = bufferPool.acquireDirectSenderBuffer(packetBufferSize);
this.peerAppData = bufferPool.acquireNonDirectReceiveBuffer(appBufferSize);
}
@@ -301,7 +301,7 @@ public class NioSslEngine implements NioFilter {
ByteBuffer buffer = wrappedBuffer;
int requiredSize = engine.getSession().getPacketBufferSize();
if (buffer == null) {
- buffer = bufferPool.acquireNonDirectBuffer(bufferType, requiredSize);
+ buffer = bufferPool.acquireDirectBuffer(bufferType, requiredSize);
} else if (buffer.capacity() < requiredSize) {
buffer = bufferPool.expandWriteBufferIfNeeded(bufferType, buffer, requiredSize);
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
index 089eb2e..d384aa1 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
@@ -1662,8 +1662,8 @@ public class Connection implements Runnable {
} catch (IOException e) {
// "Socket closed" check needed for Solaris jdk 1.4.2_08
if (!isSocketClosed() && !"Socket closed".equalsIgnoreCase(e.getMessage())) {
- if (logger.isInfoEnabled() && !isIgnorableIOException(e)) {
- logger.info("{} io exception for {}", p2pReaderName(), this, e);
+ if (logger.isDebugEnabled() && !isIgnorableIOException(e)) {
+ logger.debug("{} io exception for {}", p2pReaderName(), this, e);
}
if (e.getMessage().contains("interrupted by a call to WSACancelBlockingCall")) {
if (logger.isDebugEnabled()) {
@@ -1720,7 +1720,7 @@ public class Connection implements Runnable {
if (inputBuffer != null) {
getBufferPool().releaseReceiveBuffer(inputBuffer);
}
- inputBuffer = getBufferPool().acquireNonDirectReceiveBuffer(packetBufferSize);
+ inputBuffer = getBufferPool().acquireDirectReceiveBuffer(packetBufferSize);
}
if (channel.socket().getReceiveBufferSize() < packetBufferSize) {
channel.socket().setReceiveBufferSize(packetBufferSize);
@@ -1763,13 +1763,9 @@ public class Connection implements Runnable {
}
msg = msg.toLowerCase();
-
- if (e instanceof SSLException && msg.contains("status = closed")) {
- return true; // engine has been closed - this is normal
- }
-
- return (msg.contains("forcibly closed") || msg.contains("reset by peer")
- || msg.contains("connection reset") || msg.contains("socket is closed"));
+ return msg.contains("forcibly closed")
+ || msg.contains("reset by peer")
+ || msg.contains("connection reset");
}
private static boolean validMsgType(int msgType) {
diff --git a/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java b/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java
index 720ef62..b42d566 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java
@@ -86,11 +86,6 @@ public class NioSslEngineTest {
}
@Test
- public void engineUsesHeapBuffers() {
- assertThat(nioSslEngine.myNetData.isDirect()).isFalse();
- }
-
- @Test
public void handshake() throws Exception {
SocketChannel mockChannel = mock(SocketChannel.class);
when(mockChannel.read(any(ByteBuffer.class))).thenReturn(100, 100, 100, 0);