You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by GitBox <gi...@apache.org> on 2022/04/18 06:19:35 UTC

[GitHub] [incubator-eventmesh] ruanwenjun commented on a diff in pull request #835: [ISSUE #834] Support nacos registry

ruanwenjun commented on code in PR #835:
URL: https://github.com/apache/incubator-eventmesh/pull/835#discussion_r851899656


##########
tools/third-party-dependencies/known-dependencies.txt:
##########
@@ -124,4 +124,10 @@ system-rules-1.16.1.jar
 truth-0.30.jar
 zipkin-2.23.2.jar
 zipkin-reporter-2.16.3.jar
-zipkin-sender-okhttp3-2.16.3.jar
\ No newline at end of file
+zipkin-sender-okhttp3-2.16.3.jar
+httpasyncclient-4.1.3.jar
+httpcore-nio-4.4.6.jar
+javassist-3.21.0-GA.jar
+nacos-client-2.0.4.jar
+reflections-0.9.11.jar
+snakeyaml-1.23.jar

Review Comment:
   It needed to add notice into `NOTICE`



##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/EventMeshTCPServer.java:
##########
@@ -292,37 +293,14 @@ public void shutdown() throws Exception {
         logger.info("--------------------------EventMeshTCPServer Shutdown");
     }
 
-    private void selfRegisterToRegistry() throws Exception {
-
-        boolean registerResult = registerToRegistry();
-        if (!registerResult) {
-            throw new EventMeshException("eventMesh fail to register");
-        }
-
-        tcpRegisterTask = scheduler.scheduleAtFixedRate(
-                () -> {
-                    try {
-                        boolean heartbeatResult = registerToRegistry();
-                        if (!heartbeatResult) {
-                            logger.error("selfRegisterToRegistry fail");
-                        }
-                    } catch (Exception ex) {
-                        logger.error("selfRegisterToRegistry fail", ex);
-                    }
-                },
-                eventMeshTCPConfiguration.eventMeshRegisterIntervalInMills,
-                eventMeshTCPConfiguration.eventMeshRegisterIntervalInMills,
-                TimeUnit.MILLISECONDS);
-    }
-
     public boolean registerToRegistry() {

Review Comment:
   ```suggestion
       public boolean register() {
   ```



##########
eventmesh-spi/src/main/java/org/apache/eventmesh/spi/EventMeshExtensionFactory.java:
##########
@@ -35,8 +35,7 @@
  * The extension fetching factory, all extension plugins should be fetched by this factory.
  * And all the extension plugins defined in eventmesh should have {@link EventMeshSPI} annotation.
  */
-public enum EventMeshExtensionFactory {
-    ;
+public class EventMeshExtensionFactory {

Review Comment:
   Use `enum` here is just want to make a singleton class, you can simply to add a private constructor if you want to remove the enum, but I don't think this is a needed change.



##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/EventMeshServer.java:
##########
@@ -101,18 +122,26 @@ public void start() throws Exception {
         if (eventMeshHttpConfiguration != null && eventMeshHttpConfiguration.eventMeshServerSecurityEnable) {
             acl.start();
         }
-
+        // registry start
         if (eventMeshTCPConfiguration != null
-                && eventMeshTCPConfiguration.eventMeshTcpServerEnabled
-                && eventMeshTCPConfiguration.eventMeshServerRegistryEnable) {
+            && eventMeshTCPConfiguration.eventMeshTcpServerEnabled
+            && eventMeshTCPConfiguration.eventMeshServerRegistryEnable) {
+            registry.start();
+        }
+        if (eventMeshHttpConfiguration != null && eventMeshHttpConfiguration.eventMeshServerRegistryEnable) {
+            registry.start();
+        }
+        if (eventMeshGrpcConfiguration != null && eventMeshGrpcConfiguration.eventMeshServerRegistryEnable) {
             registry.start();
         }

Review Comment:
   Can we simplify this? When we need to register, we may need to register not matter it use http/tcp/grpc.



##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/EventMeshTCPServer.java:
##########
@@ -334,35 +312,35 @@ public boolean registerToRegistry() {
     }
 
     private void selfUnRegisterToRegistry() throws Exception {

Review Comment:
   Suggest rename to `unRegister`



##########
eventmesh-registry-plugin/eventmesh-registry-nacos/src/main/java/org/apache/eventmesh/registry/nacos/service/NacosRegistryService.java:
##########
@@ -0,0 +1,178 @@
+/*
+ * 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.eventmesh.registry.nacos.service;
+
+import org.apache.eventmesh.api.exception.RegistryException;
+import org.apache.eventmesh.api.registry.RegistryService;
+import org.apache.eventmesh.api.registry.dto.EventMeshDataInfo;
+import org.apache.eventmesh.api.registry.dto.EventMeshRegisterInfo;
+import org.apache.eventmesh.api.registry.dto.EventMeshUnRegisterInfo;
+import org.apache.eventmesh.common.config.CommonConfiguration;
+import org.apache.eventmesh.common.utils.ConfigurationContextUtil;
+import org.apache.eventmesh.registry.nacos.constant.NacosConstant;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.alibaba.nacos.api.exception.NacosException;
+import com.alibaba.nacos.api.naming.NamingService;
+import com.alibaba.nacos.api.naming.pojo.Instance;
+import com.alibaba.nacos.client.naming.NacosNamingService;
+
+public class NacosRegistryService implements RegistryService {
+
+    private static final Logger logger = LoggerFactory.getLogger(NacosRegistryService.class);
+
+    private static final AtomicBoolean INIT_STATUS = new AtomicBoolean(false);
+
+    private static final AtomicBoolean START_STATUS = new AtomicBoolean(false);
+
+    private String serverAddr;
+
+    private String username;
+
+    private String password;
+
+    private NamingService namingService;
+
+
+    @Override
+    public void init() throws RegistryException {
+        boolean update = INIT_STATUS.compareAndSet(false, true);
+        if (!update) {
+            return;
+        }
+        for (String key : ConfigurationContextUtil.KEYS) {
+            CommonConfiguration commonConfiguration = ConfigurationContextUtil.get(key);
+            if (null == commonConfiguration) {
+                continue;
+            }
+            if (StringUtils.isBlank(commonConfiguration.namesrvAddr)) {
+                throw new RegistryException("namesrvAddr cannot be null");
+            }
+            this.serverAddr = commonConfiguration.namesrvAddr;
+            this.username = commonConfiguration.eventMeshRegistryPluginUsername;
+            this.password = commonConfiguration.eventMeshRegistryPluginPassword;
+            break;
+        }
+    }
+
+    @Override
+    public void start() throws RegistryException {
+        boolean update = START_STATUS.compareAndSet(false, true);
+        if (!update) {
+            return;
+        }
+        try {
+            Properties properties = new Properties();
+            properties.setProperty(NacosConstant.SERVER_ADDR, serverAddr);
+            properties.setProperty(NacosConstant.USERNAME, username);
+            properties.setProperty(NacosConstant.PASSWORD, password);
+            namingService = new NacosNamingService(properties);
+        } catch (NacosException e) {
+            logger.error("[NacosRegistryService][start] error", e);
+            throw new RegistryException(e.getMessage());
+        }
+    }
+
+    @Override
+    public void shutdown() throws RegistryException {
+        INIT_STATUS.compareAndSet(true, false);
+        START_STATUS.compareAndSet(true, false);
+        try {
+            namingService.shutDown();
+        } catch (NacosException e) {
+            logger.error("[NacosRegistryService][shutdown] error", e);
+            throw new RegistryException(e.getMessage());
+        }
+        logger.info("NacosRegistryService close");
+    }
+
+    @Override
+    public List<EventMeshDataInfo> findEventMeshInfoByCluster(String clusterName) throws RegistryException {
+        return null;

Review Comment:
   It needed to add a `TODO` here, if this is not finished.



##########
eventmesh-registry-plugin/eventmesh-registry-nacos/src/main/java/org/apache/eventmesh/registry/nacos/service/NacosRegistryService.java:
##########
@@ -0,0 +1,178 @@
+/*
+ * 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.eventmesh.registry.nacos.service;
+
+import org.apache.eventmesh.api.exception.RegistryException;
+import org.apache.eventmesh.api.registry.RegistryService;
+import org.apache.eventmesh.api.registry.dto.EventMeshDataInfo;
+import org.apache.eventmesh.api.registry.dto.EventMeshRegisterInfo;
+import org.apache.eventmesh.api.registry.dto.EventMeshUnRegisterInfo;
+import org.apache.eventmesh.common.config.CommonConfiguration;
+import org.apache.eventmesh.common.utils.ConfigurationContextUtil;
+import org.apache.eventmesh.registry.nacos.constant.NacosConstant;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.alibaba.nacos.api.exception.NacosException;
+import com.alibaba.nacos.api.naming.NamingService;
+import com.alibaba.nacos.api.naming.pojo.Instance;
+import com.alibaba.nacos.client.naming.NacosNamingService;
+
+public class NacosRegistryService implements RegistryService {
+
+    private static final Logger logger = LoggerFactory.getLogger(NacosRegistryService.class);
+
+    private static final AtomicBoolean INIT_STATUS = new AtomicBoolean(false);
+
+    private static final AtomicBoolean START_STATUS = new AtomicBoolean(false);
+
+    private String serverAddr;
+
+    private String username;
+
+    private String password;
+
+    private NamingService namingService;
+
+

Review Comment:
   Please remove the extra empty line.



-- 
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: dev-unsubscribe@eventmesh.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org