hanishakoneru commented on a change in pull request #2945: URL: https://github.com/apache/ozone/pull/2945#discussion_r808483443
########## File path: hadoop-ozone/common/pom.xml ########## @@ -48,6 +48,17 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd";> <dependency> <groupId>io.netty</groupId> <artifactId>netty-handler-proxy</artifactId> + </dependency> + <dependency> + <groupId>io.netty</groupId> + <artifactId>netty-tcnative-boringssl-static</artifactId> + <version>2.0.38.Final</version> <!-- See table for correct version --> Review comment: Can you please add the version to variable in the main pom.xml and use that variable here. (For example - io.grpc.version is defined in ozone/pom.xml and reused in ozone-common/pom.xml). It makes it easier to track and update versions. ########## File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OmTransportFactory.java ########## @@ -34,26 +37,37 @@ OmTransport createOmTransport(ConfigurationSource source, static OmTransport create(ConfigurationSource conf, UserGroupInformation ugi, String omServiceId) throws IOException { - OmTransportFactory factory = createFactory(); + OmTransportFactory factory = createFactory(conf); return factory.createOmTransport(conf, ugi, omServiceId); } - static OmTransportFactory createFactory() throws IOException { - ServiceLoader<OmTransportFactory> transportFactoryServiceLoader = - ServiceLoader.load(OmTransportFactory.class); - Iterator<OmTransportFactory> iterator = - transportFactoryServiceLoader.iterator(); - if (iterator.hasNext()) { - return iterator.next(); - } + static OmTransportFactory createFactory(ConfigurationSource conf) + throws IOException { try { + // if configured transport class is different than the default + // Hadoop3OmTransportFactory, then check service loader for + // transport class and instantiate it + if (conf + .get(OZONE_OM_TRANSPORT_CLASS, + OZONE_OM_TRANSPORT_CLASS_DEFAULT) != + OZONE_OM_TRANSPORT_CLASS_DEFAULT) { Review comment: This check is not very robust. If the default transport factory is changed, then it would still end up loading Hadoop3OmTransportFactory instead of the new default. ########## File path: hadoop-ozone/common/pom.xml ########## @@ -48,6 +48,17 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd";> <dependency> <groupId>io.netty</groupId> <artifactId>netty-handler-proxy</artifactId> + </dependency> + <dependency> + <groupId>io.netty</groupId> + <artifactId>netty-tcnative-boringssl-static</artifactId> + <version>2.0.38.Final</version> <!-- See table for correct version --> + <scope>runtime</scope> + </dependency> + <dependency> + <groupId>io.netty</groupId> + <artifactId>netty-tcnative</artifactId> + <version>2.0.38.Final</version> <!-- See table for correct version --> Review comment: Which table is being referred to here? I didn't understand this comment here. Is 2.0.38 not the final version which needs to be used here? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
