You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2022/04/08 23:31:08 UTC

Change in asterixdb[cheshire-cat]: [NO ISSUE][NET] SSL Socket Fixes

From Ali Alsuliman <al...@gmail.com>:

Ali Alsuliman has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16065 )


Change subject: [NO ISSUE][NET] SSL Socket Fixes
......................................................................

[NO ISSUE][NET] SSL Socket Fixes

- user model changes: no
- storage format changes: no
- interface changes: no

Details:

- On SSL socket handshake failure, deliver any remaining data to requester.
- Since SSL sockets may return 0 as read bytes due to failure
  to decrypt a complete encrypted block, we need to attempt
  to read again until a complete block is decrypted.

Change-Id: I3fbbf80beb588cc3c700ff5eeb66e0d018dfacfe
---
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
M hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
M hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
7 files changed, 14 insertions(+), 9 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/65/16065/1

diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
index 6b97306..db395a9 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
@@ -83,6 +83,7 @@
         setStatus(CATCHING_UP);
         appCtx.getThreadExecutor().execute(() -> {
             try {
+                Thread.currentThread().setName("Replica " + id.toString() + " Synchronizer");
                 new ReplicaSynchronizer(appCtx, this).sync();
                 setStatus(IN_SYNC);
             } catch (Exception e) {
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
index 440f8ef..5855468 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
@@ -57,7 +57,7 @@
     @Override
     public void run() {
         final String nodeId = appCtx.getServiceContext().getNodeId();
-        Thread.currentThread().setName(nodeId + RemoteLogsNotifier.class.getSimpleName());
+        Thread.currentThread().setName(RemoteLogsNotifier.class.getSimpleName() + ":" + nodeId);
         while (!Thread.currentThread().isInterrupted()) {
             try {
                 final RemoteLogRecord logRecord = remoteLogsQ.take();
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
index dd953c4..8f7ed56 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
@@ -38,7 +38,6 @@
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 import org.apache.hyracks.api.replication.IReplicationJob;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexReplicationJob;
-import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -152,7 +151,7 @@
         if (!replicationJobsQ.isEmpty()) {
             return;
         }
-        LOGGER.log(Level.INFO, "No pending replication jobs. Closing connections to replicas");
+        LOGGER.trace("no pending replication jobs; closing connections to replicas");
         for (ReplicationDestination dest : destinations) {
             dest.getReplicas().stream().map(PartitionReplica.class::cast).forEach(PartitionReplica::close);
         }
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
index 7f6439c..712d70e 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
@@ -49,10 +49,10 @@
         byteBuffer.clear();
         byteBuffer.limit(length);
 
-        while (byteBuffer.remaining() > 0 && socketChannel.read(byteBuffer) > 0);
+        while (byteBuffer.remaining() > 0 && socketChannel.read(byteBuffer) >= 0);
 
         if (byteBuffer.remaining() > 0) {
-            throw new EOFException();
+            throw new EOFException("could not read all data from source; remaining bytes: " + byteBuffer.remaining());
         }
 
         byteBuffer.flip();
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
index 5eef84e..d8ad522 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
@@ -53,7 +53,7 @@
             final File localFile = ioManager.resolve(file).getFile();
             if (localFile.exists()) {
                 Files.delete(localFile.toPath());
-                LOGGER.info(() -> "Deleted file: " + localFile.getAbsolutePath());
+                LOGGER.debug(() -> "Deleted file: " + localFile.getAbsolutePath());
             } else {
                 LOGGER.warn(() -> "Requested to delete a non-existing file: " + localFile.getAbsolutePath());
             }
diff --git a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
index f9bf5c7..a4d1b99 100644
--- a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
+++ b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
@@ -127,7 +127,7 @@
                     break;
                 case CLOSED:
                     close();
-                    return -1;
+                    return decryptedBytes;
                 default:
                     throw new IllegalStateException("Invalid SSL result status: " + result.getStatus());
             }
@@ -192,6 +192,9 @@
             engine.closeOutbound();
             try {
                 new SslHandshake(this).handshake();
+            } catch (Exception e) {
+                // ignore exceptions on best effort graceful close handshake
+                LOGGER.trace("ssl socket close handshake failed", e);
             } finally {
                 socketChannel.close();
             }
@@ -239,7 +242,8 @@
                 close();
             }
         } catch (Exception e) {
-            LOGGER.warn("failed to close socket gracefully", e);
+            // ignore close exception since we are closing quietly
+            LOGGER.trace("failed to close socket gracefully", e);
         }
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
index 4f0c3a8..b2cd435 100644
--- a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
+++ b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
@@ -57,7 +57,8 @@
             try {
                 closeable.close();
             } catch (IOException e) {
-                LOGGER.warn("Failed to close", e);
+                // ignore since we are closing quietly
+                LOGGER.trace("failed to close", e);
             }
         }
     }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16065
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: cheshire-cat
Gerrit-Change-Id: I3fbbf80beb588cc3c700ff5eeb66e0d018dfacfe
Gerrit-Change-Number: 16065
Gerrit-PatchSet: 1
Gerrit-Owner: Ali Alsuliman <al...@gmail.com>
Gerrit-MessageType: newchange

Change in asterixdb[cheshire-cat]: [NO ISSUE][NET] SSL Socket Fixes

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Jenkins <je...@fulliautomatix.ics.uci.edu>:

Jenkins has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16065 )

Change subject: [NO ISSUE][NET] SSL Socket Fixes
......................................................................


Patch Set 1: Integration-Tests+1

Integration Tests Successful

https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/13086/ : SUCCESS


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16065
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: cheshire-cat
Gerrit-Change-Id: I3fbbf80beb588cc3c700ff5eeb66e0d018dfacfe
Gerrit-Change-Number: 16065
Gerrit-PatchSet: 1
Gerrit-Owner: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Al Hubail <mh...@uci.edu>
Gerrit-CC: Anon. E. Moose #1000171
Gerrit-Comment-Date: Sat, 09 Apr 2022 00:43:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in asterixdb[cheshire-cat]: [NO ISSUE][NET] SSL Socket Fixes

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Ali Alsuliman <al...@gmail.com>:

Ali Alsuliman has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16065 )


Change subject: [NO ISSUE][NET] SSL Socket Fixes
......................................................................

[NO ISSUE][NET] SSL Socket Fixes

- user model changes: no
- storage format changes: no
- interface changes: no

Details:

- On SSL socket handshake failure, deliver any remaining data to requester.
- Since SSL sockets may return 0 as read bytes due to failure
  to decrypt a complete encrypted block, we need to attempt
  to read again until a complete block is decrypted.

Change-Id: I3fbbf80beb588cc3c700ff5eeb66e0d018dfacfe
---
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
M asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
M hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
M hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
7 files changed, 14 insertions(+), 9 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/65/16065/1

diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
index 6b97306..db395a9 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/api/PartitionReplica.java
@@ -83,6 +83,7 @@
         setStatus(CATCHING_UP);
         appCtx.getThreadExecutor().execute(() -> {
             try {
+                Thread.currentThread().setName("Replica " + id.toString() + " Synchronizer");
                 new ReplicaSynchronizer(appCtx, this).sync();
                 setStatus(IN_SYNC);
             } catch (Exception e) {
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
index 440f8ef..5855468 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/logging/RemoteLogsNotifier.java
@@ -57,7 +57,7 @@
     @Override
     public void run() {
         final String nodeId = appCtx.getServiceContext().getNodeId();
-        Thread.currentThread().setName(nodeId + RemoteLogsNotifier.class.getSimpleName());
+        Thread.currentThread().setName(RemoteLogsNotifier.class.getSimpleName() + ":" + nodeId);
         while (!Thread.currentThread().isInterrupted()) {
             try {
                 final RemoteLogRecord logRecord = remoteLogsQ.take();
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
index dd953c4..8f7ed56 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
@@ -38,7 +38,6 @@
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 import org.apache.hyracks.api.replication.IReplicationJob;
 import org.apache.hyracks.storage.am.lsm.common.api.ILSMIndexReplicationJob;
-import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -152,7 +151,7 @@
         if (!replicationJobsQ.isEmpty()) {
             return;
         }
-        LOGGER.log(Level.INFO, "No pending replication jobs. Closing connections to replicas");
+        LOGGER.trace("no pending replication jobs; closing connections to replicas");
         for (ReplicationDestination dest : destinations) {
             dest.getReplicas().stream().map(PartitionReplica.class::cast).forEach(PartitionReplica::close);
         }
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
index 7f6439c..712d70e 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/NetworkingUtil.java
@@ -49,10 +49,10 @@
         byteBuffer.clear();
         byteBuffer.limit(length);
 
-        while (byteBuffer.remaining() > 0 && socketChannel.read(byteBuffer) > 0);
+        while (byteBuffer.remaining() > 0 && socketChannel.read(byteBuffer) >= 0);
 
         if (byteBuffer.remaining() > 0) {
-            throw new EOFException();
+            throw new EOFException("could not read all data from source; remaining bytes: " + byteBuffer.remaining());
         }
 
         byteBuffer.flip();
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
index 5eef84e..d8ad522 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/messaging/DeleteFileTask.java
@@ -53,7 +53,7 @@
             final File localFile = ioManager.resolve(file).getFile();
             if (localFile.exists()) {
                 Files.delete(localFile.toPath());
-                LOGGER.info(() -> "Deleted file: " + localFile.getAbsolutePath());
+                LOGGER.debug(() -> "Deleted file: " + localFile.getAbsolutePath());
             } else {
                 LOGGER.warn(() -> "Requested to delete a non-existing file: " + localFile.getAbsolutePath());
             }
diff --git a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
index f9bf5c7..a4d1b99 100644
--- a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
+++ b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/sockets/SslSocketChannel.java
@@ -127,7 +127,7 @@
                     break;
                 case CLOSED:
                     close();
-                    return -1;
+                    return decryptedBytes;
                 default:
                     throw new IllegalStateException("Invalid SSL result status: " + result.getStatus());
             }
@@ -192,6 +192,9 @@
             engine.closeOutbound();
             try {
                 new SslHandshake(this).handshake();
+            } catch (Exception e) {
+                // ignore exceptions on best effort graceful close handshake
+                LOGGER.trace("ssl socket close handshake failed", e);
             } finally {
                 socketChannel.close();
             }
@@ -239,7 +242,8 @@
                 close();
             }
         } catch (Exception e) {
-            LOGGER.warn("failed to close socket gracefully", e);
+            // ignore close exception since we are closing quietly
+            LOGGER.trace("failed to close socket gracefully", e);
         }
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
index 4f0c3a8..b2cd435 100644
--- a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
+++ b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/NetworkUtil.java
@@ -57,7 +57,8 @@
             try {
                 closeable.close();
             } catch (IOException e) {
-                LOGGER.warn("Failed to close", e);
+                // ignore since we are closing quietly
+                LOGGER.trace("failed to close", e);
             }
         }
     }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16065
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: cheshire-cat
Gerrit-Change-Id: I3fbbf80beb588cc3c700ff5eeb66e0d018dfacfe
Gerrit-Change-Number: 16065
Gerrit-PatchSet: 1
Gerrit-Owner: Ali Alsuliman <al...@gmail.com>
Gerrit-MessageType: newchange