HDDS-11883. SCM HA: Move proxy object creation code to SCMRatisServer (apache#7914)

nandakumar131 · web-flow · commit 1513c34e2f2b · 2025-02-20T11:43:17.000+01:00
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogStateManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogStateManagerImpl.java
@@ -19,7 +19,6 @@
 
 import com.google.common.base.Preconditions;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
@@ -28,11 +27,10 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol;
+import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType;
 import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.DeletedBlocksTransaction;
 import org.apache.hadoop.hdds.scm.container.ContainerID;
 import org.apache.hadoop.hdds.scm.container.ContainerManager;
-import org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler;
 import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer;
 import org.apache.hadoop.hdds.utils.db.Table;
@@ -302,18 +300,11 @@ public DeletedBlockLogStateManager build() {
       Preconditions.checkNotNull(conf);
       Preconditions.checkNotNull(table);
 
-      final DeletedBlockLogStateManager impl =
-          new DeletedBlockLogStateManagerImpl(conf, table, containerManager,
-              transactionBuffer);
+      final DeletedBlockLogStateManager impl = new DeletedBlockLogStateManagerImpl(
+          conf, table, containerManager, transactionBuffer);
 
-      final SCMHAInvocationHandler invocationHandler =
-          new SCMHAInvocationHandler(SCMRatisProtocol.RequestType.BLOCK,
-              impl, scmRatisServer);
-
-      return (DeletedBlockLogStateManager) Proxy.newProxyInstance(
-          SCMHAInvocationHandler.class.getClassLoader(),
-          new Class<?>[]{DeletedBlockLogStateManager.class},
-          invocationHandler);
+      return scmRatisServer.getProxyHandler(RequestType.BLOCK,
+          DeletedBlockLogStateManager.class, impl);
     }
   }
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java
@@ -35,7 +35,6 @@
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.Striped;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.util.EnumMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -58,7 +57,6 @@
 import org.apache.hadoop.hdds.scm.container.states.ContainerState;
 import org.apache.hadoop.hdds.scm.container.states.ContainerStateMap;
 import org.apache.hadoop.hdds.scm.ha.ExecutionUtil;
-import org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler;
 import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer;
 import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
@@ -611,13 +609,8 @@ public ContainerStateManager build() throws IOException {
           conf, pipelineMgr, table, transactionBuffer,
           containerReplicaPendingOps);
 
-      final SCMHAInvocationHandler invocationHandler =
-          new SCMHAInvocationHandler(RequestType.CONTAINER, csm,
-              scmRatisServer);
-
-      return (ContainerStateManager) Proxy.newProxyInstance(
-          SCMHAInvocationHandler.class.getClassLoader(),
-          new Class<?>[]{ContainerStateManager.class}, invocationHandler);
+      return scmRatisServer.getProxyHandler(RequestType.CONTAINER,
+          ContainerStateManager.class, csm);
     }
 
   }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServer.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hdds.scm.ha;
 
 import java.io.IOException;
+import java.lang.reflect.Proxy;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
@@ -70,4 +71,11 @@ SCMRatisResponse submitRequest(SCMRatisRequest request)
 
   RaftPeerId getLeaderId();
 
+  default <T> T getProxyHandler(final RequestType type, final Class<T> intf, final T impl) {
+    final SCMHAInvocationHandler invocationHandler =
+        new SCMHAInvocationHandler(type, impl, this);
+    return intf.cast(Proxy.newProxyInstance(getClass().getClassLoader(),
+        new Class<?>[] {intf}, invocationHandler));
+  }
+
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java
@@ -23,7 +23,6 @@
 
 import com.google.common.base.Preconditions;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.math.BigInteger;
 import java.security.cert.X509Certificate;
 import java.time.LocalDate;
@@ -330,13 +329,7 @@ public StateManager build() {
 
         final StateManager impl = new StateManagerImpl(table, buffer);
 
-        final SCMHAInvocationHandler invocationHandler
-            = new SCMHAInvocationHandler(SEQUENCE_ID, impl, ratisServer);
-
-        return (StateManager) Proxy.newProxyInstance(
-            SCMHAInvocationHandler.class.getClassLoader(),
-            new Class<?>[]{StateManager.class},
-            invocationHandler);
+        return ratisServer.getProxyHandler(SEQUENCE_ID, StateManager.class, impl);
       }
     }
   }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/StatefulServiceStateManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/StatefulServiceStateManagerImpl.java
@@ -20,7 +20,6 @@
 import com.google.common.base.Preconditions;
 import com.google.protobuf.ByteString;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType;
 import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer;
 import org.apache.hadoop.hdds.utils.db.Table;
@@ -135,13 +134,8 @@ public StatefulServiceStateManager build() {
           new StatefulServiceStateManagerImpl(statefulServiceConfig,
               transactionBuffer);
 
-      final SCMHAInvocationHandler invocationHandler =
-          new SCMHAInvocationHandler(RequestType.STATEFUL_SERVICE_CONFIG,
-              stateManager, scmRatisServer);
-
-      return (StatefulServiceStateManager) Proxy.newProxyInstance(
-          SCMHAInvocationHandler.class.getClassLoader(),
-          new Class<?>[]{StatefulServiceStateManager.class}, invocationHandler);
+      return scmRatisServer.getProxyHandler(RequestType.STATEFUL_SERVICE_CONFIG,
+          StatefulServiceStateManager.class, stateManager);
     }
   }
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateManagerImpl.java
@@ -19,7 +19,6 @@
 
 import com.google.common.base.Preconditions;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.util.Collection;
 import java.util.List;
 import java.util.NavigableSet;
@@ -28,9 +27,8 @@
 import org.apache.hadoop.hdds.client.ReplicationConfig;
 import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
-import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol;
+import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType;
 import org.apache.hadoop.hdds.scm.container.ContainerID;
-import org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler;
 import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer;
 import org.apache.hadoop.hdds.scm.node.NodeManager;
@@ -380,13 +378,8 @@ public PipelineStateManager build() throws IOException {
           new PipelineStateManagerImpl(
               pipelineStore, nodeManager, transactionBuffer);
 
-      final SCMHAInvocationHandler invocationHandler =
-          new SCMHAInvocationHandler(SCMRatisProtocol.RequestType.PIPELINE,
-              pipelineStateManager, scmRatisServer);
-
-      return (PipelineStateManager) Proxy.newProxyInstance(
-          SCMHAInvocationHandler.class.getClassLoader(),
-          new Class<?>[]{PipelineStateManager.class}, invocationHandler);
+      return scmRatisServer.getProxyHandler(RequestType.PIPELINE,
+          PipelineStateManager.class, pipelineStateManager);
     }
   }
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationHandlerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationHandlerImpl.java
@@ -23,14 +23,12 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.nio.file.Files;
 import java.nio.file.StandardCopyOption;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 import org.apache.commons.io.FileUtils;
-import org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler;
 import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
 import org.apache.hadoop.hdds.security.SecurityConfig;
@@ -230,13 +228,8 @@ public RootCARotationHandler build() {
       final RootCARotationHandler impl =
           new RootCARotationHandlerImpl(scm, rootCARotationManager);
 
-      final SCMHAInvocationHandler invocationHandler
-          = new SCMHAInvocationHandler(CERT_ROTATE, impl, ratisServer);
-
-      return (RootCARotationHandler) Proxy.newProxyInstance(
-          SCMHAInvocationHandler.class.getClassLoader(),
-          new Class<?>[]{RootCARotationHandler.class},
-          invocationHandler);
+      return ratisServer.getProxyHandler(CERT_ROTATE,
+          RootCARotationHandler.class, impl);
     }
   }
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/ScmSecretKeyStateBuilder.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/ScmSecretKeyStateBuilder.java
@@ -17,9 +17,7 @@
 
 package org.apache.hadoop.hdds.scm.security;
 
-import java.lang.reflect.Proxy;
 import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol;
-import org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler;
 import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.security.symmetric.SecretKeyState;
 import org.apache.hadoop.hdds.security.symmetric.SecretKeyStateImpl;
@@ -47,13 +45,7 @@ public ScmSecretKeyStateBuilder setRatisServer(
 
   public SecretKeyState build() {
     final SecretKeyState impl = new SecretKeyStateImpl(secretKeyStore);
-
-    final SCMHAInvocationHandler scmhaInvocationHandler =
-        new SCMHAInvocationHandler(SCMRatisProtocol.RequestType.SECRET_KEY,
-            impl, scmRatisServer);
-
-    return (SecretKeyState) Proxy.newProxyInstance(
-        SCMHAInvocationHandler.class.getClassLoader(),
-        new Class<?>[]{SecretKeyState.class}, scmhaInvocationHandler);
+    return scmRatisServer.getProxyHandler(SCMRatisProtocol.RequestType.SECRET_KEY,
+       SecretKeyState.class, impl);
   }
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMCertStore.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMCertStore.java
@@ -21,7 +21,6 @@
 
 import com.google.common.base.Preconditions;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.math.BigInteger;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
@@ -31,7 +30,6 @@
 import java.util.concurrent.locks.ReentrantLock;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
 import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol;
-import org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler;
 import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStore;
 import org.apache.hadoop.hdds.security.exception.SCMSecurityException;
@@ -227,17 +225,9 @@ public Builder setRatisServer(final SCMRatisServer ratisServer) {
     }
 
     public CertificateStore build() {
-      final SCMCertStore scmCertStore = new SCMCertStore(metadataStore
-      );
-
-      final SCMHAInvocationHandler scmhaInvocationHandler =
-          new SCMHAInvocationHandler(SCMRatisProtocol.RequestType.CERT_STORE,
-              scmCertStore, scmRatisServer);
-
-      return (CertificateStore) Proxy.newProxyInstance(
-          SCMHAInvocationHandler.class.getClassLoader(),
-          new Class<?>[]{CertificateStore.class}, scmhaInvocationHandler);
-
+      final SCMCertStore scmCertStore = new SCMCertStore(metadataStore);
+      return scmRatisServer.getProxyHandler(SCMRatisProtocol.RequestType.CERT_STORE,
+         CertificateStore.class, scmCertStore);
     }
   }
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/FinalizationStateManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/FinalizationStateManagerImpl.java
@@ -20,11 +20,9 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol;
-import org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler;
 import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer;
 import org.apache.hadoop.hdds.scm.metadata.Replicate;
@@ -331,14 +329,9 @@ public FinalizationStateManager build() throws IOException {
       Preconditions.checkNotNull(finalizationStore);
       Preconditions.checkNotNull(transactionBuffer);
       Preconditions.checkNotNull(upgradeFinalizer);
-      final SCMHAInvocationHandler invocationHandler =
-          new SCMHAInvocationHandler(SCMRatisProtocol.RequestType.FINALIZE,
-              new FinalizationStateManagerImpl(this),
-              scmRatisServer);
-
-      return (FinalizationStateManager) Proxy.newProxyInstance(
-          SCMHAInvocationHandler.class.getClassLoader(),
-          new Class<?>[]{FinalizationStateManager.class}, invocationHandler);
+
+      return scmRatisServer.getProxyHandler(SCMRatisProtocol.RequestType.FINALIZE,
+        FinalizationStateManager.class, new FinalizationStateManagerImpl(this));
     }
   }
 }
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestReplicationAnnotation.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestReplicationAnnotation.java
@@ -22,7 +22,6 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.IOException;
-import java.lang.reflect.Proxy;
 import java.util.List;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
@@ -43,7 +42,6 @@
  * Tests on {@link org.apache.hadoop.hdds.scm.metadata.Replicate}.
  */
 public class TestReplicationAnnotation {
-  private SCMHAInvocationHandler scmhaInvocationHandler;
   private SCMRatisServer scmRatisServer;
 
   @BeforeEach
@@ -118,20 +116,16 @@ public GrpcTlsConfig getGrpcTlsConfig() {
       public RaftPeerId getLeaderId() {
         return RaftPeerId.valueOf(UUID.randomUUID().toString());
       }
+
     };
   }
 
   @Test
   public void testReplicateAnnotationBasic() throws Throwable {
 
-    scmhaInvocationHandler = new SCMHAInvocationHandler(
-        RequestType.CONTAINER, null, scmRatisServer);
+    ContainerStateManager proxy = scmRatisServer.getProxyHandler(RequestType.CONTAINER,
+        ContainerStateManager.class, null);
 
-    ContainerStateManager proxy =
-        (ContainerStateManager) Proxy.newProxyInstance(
-            SCMHAInvocationHandler.class.getClassLoader(),
-            new Class<?>[]{ContainerStateManager.class},
-            scmhaInvocationHandler);
     IOException e =
         assertThrows(IOException.class,
             () -> proxy.addContainer(HddsProtos.ContainerInfoProto.getDefaultInstance()));
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMCertStore.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMCertStore.java
@@ -21,6 +21,11 @@
 import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType.OM;
 import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType.SCM;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.AdditionalAnswers.returnsLastArg;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.math.BigInteger;
 import java.nio.file.Files;
@@ -32,6 +37,8 @@
 import org.apache.hadoop.hdds.HddsConfigKeys;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
+import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol;
+import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStore;
 import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStoreImpl;
 import org.apache.hadoop.hdds.security.SecurityConfig;
@@ -66,8 +73,12 @@ public void setUp(@TempDir Path tempDir) throws Exception {
     securityConfig = new SecurityConfig(config);
     keyPair = KeyStoreTestUtil.generateKeyPair("RSA");
 
+    final SCMRatisServer ratisServer = mock(SCMRatisServer.class);
+    when(ratisServer.getProxyHandler(eq(SCMRatisProtocol.RequestType.CERT_STORE),
+        eq(CertificateStore.class), any(CertificateStore.class)))
+        .then(returnsLastArg());
     scmMetadataStore = new SCMMetadataStoreImpl(config);
-    scmCertStore = new SCMCertStore.Builder().setRatisServer(null)
+    scmCertStore = new SCMCertStore.Builder().setRatisServer(ratisServer)
         .setMetadaStore(scmMetadataStore)
         .build();
 

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,6 @@`
`23`	`23`
`24`	`24`	`import com.google.common.base.Preconditions;`
`25`	`25`	`import java.io.IOException;`
`26`		`-import java.lang.reflect.Proxy;`
`27`	`26`	`import java.math.BigInteger;`
`28`	`27`	`import java.security.cert.X509Certificate;`
`29`	`28`	`import java.time.LocalDate;`
`@@ -330,13 +329,7 @@ public StateManager build() {`
`330`	`329`
`331`	`330`	`final StateManager impl = new StateManagerImpl(table, buffer);`
`332`	`331`
`333`		`- final SCMHAInvocationHandler invocationHandler`
`334`		`- = new SCMHAInvocationHandler(SEQUENCE_ID, impl, ratisServer);`
`335`		`-`
`336`		`- return (StateManager) Proxy.newProxyInstance(`
`337`		`- SCMHAInvocationHandler.class.getClassLoader(),`
`338`		`- new Class<?>[]{StateManager.class},`
`339`		`- invocationHandler);`
	`332`	`+ return ratisServer.getProxyHandler(SEQUENCE_ID, StateManager.class, impl);`
`340`	`333`	`}`
`341`	`334`	`}`
`342`	`335`	`}`