You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/02/09 04:30:22 UTC

[GitHub] [hadoop] jojochuang commented on a change in pull request #3966: [WIP] HADOOP-15980 : Enable TLS in RPC client/server

jojochuang commented on a change in pull request #3966:
URL: https://github.com/apache/hadoop/pull/3966#discussion_r802256657



##########
File path: hadoop-common-project/hadoop-common/pom.xml
##########
@@ -393,6 +393,13 @@
       <artifactId>lz4-java</artifactId>
       <scope>provided</scope>
     </dependency>
+    <!-- will be shaded into the hadoop-common jar for RPC use only -->
+    <dependency>
+      <groupId>io.netty</groupId>
+      <artifactId>netty-all</artifactId>
+      <version>4.1.27.Final</version>

Review comment:
       We manage dependency versions in hadoop-project/pom.xml, so let's not specify the version here.
   It's currently at 4.1.68.Final

##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
##########
@@ -59,6 +59,16 @@
       "ipc.client.rpc-timeout.ms";
   /** Default value for IPC_CLIENT_RPC_TIMEOUT_KEY. */
   public static final int IPC_CLIENT_RPC_TIMEOUT_DEFAULT = 120000;
+  /** Enable the experimental use of netty instead of nio. */
+  public static final String IPC_SERVER_NETTY_ENABLE_KEY =

Review comment:
       IMO, if they are backward compatible, we can consider enabling it by default in Hadoop 3.4.0, and perhaps disable it by default in lower branches.

##########
File path: hadoop-common-project/hadoop-common/pom.xml
##########
@@ -669,6 +676,36 @@
           </filesets>
         </configuration>
       </plugin>
+        <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-shade-plugin</artifactId>
+            <version>${maven-shade-plugin.version}</version>
+            <executions>
+                <execution>
+                    <phase>package</phase>
+                    <goals>
+                        <goal>shade</goal>
+                    </goals>
+                    <configuration>
+                        <artifactSet>
+                            <includes>
+                                <include>io.netty:netty-all:jar:*</include>
+                            </includes>
+                            <excludes>
+                                <!-- this is netty 3 -->
+                                <exclude>*:netty</exclude>
+                            </excludes>
+                        </artifactSet>
+                        <relocations>
+                            <relocation>
+                                <pattern>io.netty</pattern>
+                                <shadedPattern>hrpc.io.netty</shadedPattern>

Review comment:
       This relocation path will break HBase (it has classpath checks forbidding unfamiliar prefixes)
   Can we change it to org.apache.hadoop.io.netty?
   In fact, it would be a good idea to shade netty into hadoop-thirdparty (https://github.com/apache/hadoop-thirdparty)which relocates to org.apache.hadoop.thirdparty.*




-- 
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.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org