jojochuang commented on code in PR #10030:
URL: https://github.com/apache/ozone/pull/10030#discussion_r3089714944
##########
hadoop-ozone/integration-test-recon/pom.xml:
##########
@@ -217,12 +217,17 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
- <configuration>
Review Comment:
in effect, this change:
scopes the configuration to a specific execution (analyze), and
makes the ignore-list additive across parent/child POM inheritance, reducing
the chance that this module accidentally wipes out ignore rules defined in a
parent.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java:
##########
@@ -130,13 +159,30 @@ public void start() throws IOException {
return;
}
+ ThreadFactory factory = new ThreadFactoryBuilder()
+ .setDaemon(true)
+ .setNameFormat(CLIENT_NAME + "-ELG-%d")
+ .build();
+
+ final Class<? extends Channel> channelType;
+ if (Epoll.isAvailable()) {
+ eventLoopGroup = new EpollEventLoopGroup(0, factory);
+ channelType = EpollSocketChannel.class;
+ } else {
+ eventLoopGroup = new NioEventLoopGroup(0, factory);
+ channelType = NioSocketChannel.class;
+ }
+ LOG.info("{} channel type {}", CLIENT_NAME, channelType.getSimpleName());
+
Review Comment:
this change should split into a separate PR.
and I believe there are other places we want to use epoll whenever possible.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java:
##########
@@ -435,18 +434,29 @@ public void testS3Auth() throws Exception {
}
@Test
- public void testRemoteException() {
+ public void testRemoteException() throws IOException {
UserGroupInformation realUser =
UserGroupInformation.createRemoteUser("realUser");
UserGroupInformation proxyUser =
UserGroupInformation.createProxyUser("user", realUser);
- assertThrows(AccessControlException.class, () -> {
Review Comment:
unrelated to shading.
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/protocolPB/TestGrpcOmTransportConcurrentFailover.java:
##########
@@ -301,5 +322,38 @@ public int getFailureCount() {
return failureCount.get();
}
}
+
+ private static final class Proto2Marshaller<T> implements
MethodDescriptor.Marshaller<T> {
Review Comment:
there's another one in TestS3GrpcOmTransport.java which is exactly the same.
##########
hadoop-hdds/common/pom.xml:
##########
@@ -248,6 +248,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
+ <useIncrementalCompilation>false</useIncrementalCompilation>
Review Comment:
"Disabling incremental compilation forces a cleaner, dependency-correct
rebuild behavior for that module’s compilation step, so that when an API change
happens, Maven/javac are much less likely to leave behind stale .class files
that were compiled against old symbols.
In short: it trades some compile speed for build determinism and
correctness, which matters a lot in PRs like this one (migrating shaded
gRPC/Netty/Ratis dependencies) where API surfaces and classpaths can shift in
ways that incremental compilation is particularly bad at handling."
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/protocolPB/TestGrpcOmTransportConcurrentFailover.java:
##########
@@ -301,5 +322,38 @@ public int getFailureCount() {
return failureCount.get();
}
}
+
+ private static final class Proto2Marshaller<T> implements
MethodDescriptor.Marshaller<T> {
Review Comment:
and I wonder if the other Proto2Marshaller in GrpcOmTransport can conslidate
too.
--
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]
