This is an automated email from the ASF dual-hosted git repository.
baunsgaard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/main by this push:
new 49acf287f5 [MINOR] fix federated warning of ssl if ssl is not enabled
49acf287f5 is described below
commit 49acf287f5c538b7e531eb229d6b45b40528e5af
Author: Sebastian Baunsgaard <baunsga...@apache.org>
AuthorDate: Fri Sep 20 15:16:20 2024 +0200
[MINOR] fix federated warning of ssl if ssl is not enabled
This commit reduce the logging, and avoids allocating
the ssl certificate in cases where ssl is not enabled.
This reduce the logging warnings from netty on loading
ssl elements that use java unsafe.
---
.../controlprogram/federated/FederatedData.java | 28 +--------
.../controlprogram/federated/FederatedSSLUtil.java | 67 ++++++++++++++++++++++
.../controlprogram/federated/FederatedWorker.java | 21 ++++---
3 files changed, 84 insertions(+), 32 deletions(-)
diff --git
a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedData.java
b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedData.java
index 8d3347b454..6611e1a9c2 100644
---
a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedData.java
+++
b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedData.java
@@ -70,8 +70,7 @@ public class FederatedData {
/** Thread pool specific for the federated requests */
private static EventLoopGroup workerGroup = null;
- /** A Singleton constructed SSL context, that only is assigned if ssl
is enabled. */
- private static SslContextMan sslInstance = null;
+
private final Types.DataType _dataType;
private final InetSocketAddress _address;
@@ -245,7 +244,7 @@ public class FederatedData {
cp.addLast("NetworkTrafficCounter", new
NetworkTrafficCounter(FederatedStatistics::logServerTraffic));
if(ssl)
- cp.addLast(createSSLHandler(ch,
address));
+
cp.addLast(FederatedSSLUtil.createSSLHandler(ch, address));
if(timeout > -1)
cp.addLast(new
ReadTimeoutHandler(timeout));
@@ -280,9 +279,7 @@ public class FederatedData {
}
}
- private static SslHandler createSSLHandler(SocketChannel ch,
InetSocketAddress address) {
- return SslConstructor().context.newHandler(ch.alloc(),
address.getAddress().getHostAddress(), address.getPort());
- }
+
public static void resetFederatedSites() {
_allFedSites.clear();
@@ -320,25 +317,6 @@ public class FederatedData {
}
}
- private static class SslContextMan {
- protected final SslContext context;
-
- private SslContextMan() {
- try {
- context =
SslContextBuilder.forClient().trustManager(InsecureTrustManagerFactory.INSTANCE).build();
- }
- catch(SSLException e) {
- throw new DMLRuntimeException("Static SSL setup
failed for client side", e);
- }
- }
- }
-
- private static SslContextMan SslConstructor() {
- if(sslInstance == null)
- return new SslContextMan();
- else
- return sslInstance;
- }
@Override
public String toString() {
diff --git
a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedSSLUtil.java
b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedSSLUtil.java
new file mode 100644
index 0000000000..f1300ef0f4
--- /dev/null
+++
b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedSSLUtil.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sysds.runtime.controlprogram.federated;
+
+import java.net.InetSocketAddress;
+
+import javax.net.ssl.SSLException;
+
+import org.apache.sysds.runtime.DMLRuntimeException;
+
+import io.netty.channel.socket.SocketChannel;
+import io.netty.handler.ssl.SslContext;
+import io.netty.handler.ssl.SslContextBuilder;
+import io.netty.handler.ssl.SslHandler;
+import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
+
+public class FederatedSSLUtil {
+
+ private FederatedSSLUtil(){
+ // private constructor.
+ }
+
+ /** A Singleton constructed SSL context, that only is assigned if ssl
is enabled. */
+ private static SslContextMan sslInstance = null;
+
+ protected static SslContextMan SslConstructor() {
+ if(sslInstance == null)
+ return new SslContextMan();
+ else
+ return sslInstance;
+ }
+
+ protected static SslHandler createSSLHandler(SocketChannel ch,
InetSocketAddress address) {
+ return SslConstructor().context.newHandler(ch.alloc(),
address.getAddress().getHostAddress(), address.getPort());
+ }
+
+
+ private static class SslContextMan {
+ protected final SslContext context;
+
+ private SslContextMan() {
+ try {
+ context =
SslContextBuilder.forClient().trustManager(InsecureTrustManagerFactory.INSTANCE).build();
+ }
+ catch(SSLException e) {
+ throw new DMLRuntimeException("Static SSL setup
failed for client side", e);
+ }
+ }
+ }
+}
diff --git
a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorker.java
b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorker.java
index 8d9ad28d04..e04d5cbc7b 100644
---
a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorker.java
+++
b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorker.java
@@ -189,19 +189,26 @@ public class FederatedWorker {
private ChannelInitializer<SocketChannel> createChannel(boolean ssl) {
try {
// TODO add ability to use real ssl files, not self
signed certificates.
- final SelfSignedCertificate cert = new
SelfSignedCertificate();
- final SslContext cont2 =
SslContextBuilder.forServer(cert.certificate(), cert.privateKey()).build();
+ final SelfSignedCertificate cert;
+ final SslContext cont2;
+ final boolean sslEnabled =
ConfigurationManager.getDMLConfig().getBooleanValue(DMLConfig.USE_SSL_FEDERATED_COMMUNICATION)
|| ssl;
+
+ if(ssl) {
+ cert = new SelfSignedCertificate();
+ cont2 =
SslContextBuilder.forServer(cert.certificate(), cert.privateKey()).build();
+ }
+ else {
+ cert = null;
+ cont2 = null;
+ }
return new ChannelInitializer<>() {
@Override
public void initChannel(SocketChannel ch) {
final ChannelPipeline cp =
ch.pipeline();
- if(ConfigurationManager.getDMLConfig()
-
.getBooleanValue(DMLConfig.USE_SSL_FEDERATED_COMMUNICATION)) {
-
cp.addLast(cont2.newHandler(ch.alloc()));
- }
- if(ssl)
+ if(sslEnabled)
cp.addLast(cont2.newHandler(ch.alloc()));
+
final
Optional<ImmutablePair<ChannelInboundHandlerAdapter,
ChannelOutboundHandlerAdapter>> compressionStrategy =
FederationUtils.compressionStrategy();
cp.addLast("NetworkTrafficCounter", new
NetworkTrafficCounter(FederatedStatistics::logWorkerTraffic));
cp.addLast("CompressionDecodingStartStatistics", new
CompressionDecoderStartStatisticsHandler());
