You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@linkis.apache.org by "peacewong (via GitHub)" <gi...@apache.org> on 2023/06/07 08:48:34 UTC

[GitHub] [linkis] peacewong opened a new pull request, #4608: [WIP] ECM stateless feature optimization

peacewong opened a new pull request, #4608:
URL: https://github.com/apache/linkis/pull/4608

   <!--
   Thank you for sending the PR! We appreciate you spending the time to work on these changes.
   You can learn more about contributing to Apache Linkis here: https://linkis.apache.org/community/how-to-contribute
   Happy contributing!
   -->
   
   ### What is the purpose of the change
   
   EngineConn-Core defines the the abstractions and interfaces of the EngineConn core functions.
   The Engine Service in Linkis 0.x is refactored, EngineConn will handle the engine connection 
   and session management.
   
   ### Related issues/PRs
   
   Related issues: #4605,#4606,#4607
   Related pr: #4510, #4452
   
   
   ### Brief change log
   
   - Define the core abstraction and interfaces of the EngineConn Factory;
   - Define the core abstraction and interfaces of Executor Manager.
   
   
   ### Checklist
   
   - [x] I have read the [Contributing Guidelines on pull requests](https://github.com/facebook/docusaurus/blob/main/CONTRIBUTING.md#pull-requests).
   - [ ] I have explained the need for this PR and the problem it solves
   - [ ] I have explained the changes or the new features added to this PR
   - [ ] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [ ] I have verified that this change is backward compatible (If not, please discuss on the [Linkis mailing list](https://linkis.apache.org/community/how-to-subscribe) first)
   - [ ] **If this is a code change**: I have written unit tests to fully verify the new behavior.
   
   
   
   <!--
   
   Note
   
   1. Mark the PR title as `[WIP] title` until it's ready to be reviewed.
      如果PR还未准备好被review,请在标题上添加[WIP]标识(WIP work in progress)
   
   2. Always add/update tests for any changes unless you have a good reason.
      除非您有充分的理由,否则任何修改都需要添加/更新测试
      
   3. Always update the documentation to reflect the changes made in the PR.
      始终更新文档以反映 PR 中所做的更改  
      
   4. After the PR is submitted, please pay attention to the execution result of git action check. 
      If there is any failure, please adjust it in time
      PR提交后,请关注git action check 执行结果,关键的check失败时,请及时修正
      
   5. Before the pr is merged, if the commit is missing, you can continue to commit the code
       在未合并前,如果提交有遗漏,您可以继续提交代码 
   
   6. After you submit PR, you can add assistant WeChat, the WeChat QR code is 
      https://user-images.githubusercontent.com/7869972/176336986-d6b9be8f-d1d3-45f1-aa45-8e6adf5dd244.png 
      您提交pr后,可以添加助手微信,微信二维码为
      https://user-images.githubusercontent.com/7869972/176336986-d6b9be8f-d1d3-45f1-aa45-8e6adf5dd244.png
   
   -->
   


-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] CharlieYan24 commented on pull request #4608: [WIP] ECM stateless feature optimization

Posted by "CharlieYan24 (via GitHub)" <gi...@apache.org>.
CharlieYan24 commented on PR #4608:
URL: https://github.com/apache/linkis/pull/4608#issuecomment-1581828918

   have download code, and finished review, LGTM.


-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] peacewong commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "peacewong (via GitHub)" <gi...@apache.org>.
peacewong commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1225235318


##########
linkis-computation-governance/linkis-engineconn-manager/linkis-engineconn-manager-server/src/main/scala/org/apache/linkis/ecm/server/service/impl/ProcessEngineConnLaunchService.scala:
##########
@@ -52,110 +48,79 @@ abstract class ProcessEngineConnLaunchService extends AbstractEngineConnLaunchSe
   def setLocalDirsHandleService(localDirsHandleService: LocalDirsHandleService): Unit =
     this.localDirsHandleService = localDirsHandleService
 
-  override def waitEngineConnStart(
+  override def startEngineConnMonitorStart(
       request: EngineConnLaunchRequest,
       conn: EngineConn,
       duration: Long

Review Comment:
   Yes



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on PR #4608:
URL: https://github.com/apache/linkis/pull/4608#issuecomment-1585692603

   LGTM


-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] peacewong commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "peacewong (via GitHub)" <gi...@apache.org>.
peacewong commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1225235449


##########
linkis-computation-governance/linkis-engineconn-manager/linkis-engineconn-manager-server/src/main/java/org/apache/linkis/ecm/server/service/impl/DefaultEngineConnKillService.java:
##########
@@ -0,0 +1,286 @@
+/*
+ * 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.linkis.ecm.server.service.impl;
+
+import org.apache.linkis.common.ServiceInstance;
+import org.apache.linkis.common.utils.Utils;
+import org.apache.linkis.ecm.server.conf.ECMConfiguration;
+import org.apache.linkis.ecm.server.service.EngineConnKillService;
+import org.apache.linkis.engineconn.common.conf.EngineConnConf;
+import org.apache.linkis.governance.common.utils.GovernanceUtils;
+import org.apache.linkis.manager.common.constant.AMConstant;
+import org.apache.linkis.manager.common.protocol.engine.EngineStopRequest;
+import org.apache.linkis.manager.common.protocol.engine.EngineStopResponse;
+import org.apache.linkis.manager.common.protocol.engine.EngineSuicideRequest;
+import org.apache.linkis.rpc.Sender;
+import org.apache.linkis.rpc.message.annotation.Receiver;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DefaultEngineConnKillService implements EngineConnKillService {
+
+  private static final Logger logger = LoggerFactory.getLogger(DefaultEngineConnKillService.class);
+
+  private static final ThreadPoolExecutor ecYarnAppKillService =
+      Utils.newCachedThreadPool(10, "ECM-Kill-EC-Yarn-App", true);
+
+  @Override
+  @Receiver
+  public EngineStopResponse dealEngineConnStop(EngineStopRequest engineStopRequest) {
+    logger.info("received EngineStopRequest " + engineStopRequest);
+    String pid = null;
+    if (AMConstant.PROCESS_MARK.equals(engineStopRequest.getIdentifierType())
+        && StringUtils.isNotBlank(engineStopRequest.getIdentifier())) {
+      pid = engineStopRequest.getIdentifier();
+    }
+    logger.info("dealEngineConnStop return pid: {}", pid);
+    EngineStopResponse response = new EngineStopResponse();
+    if (StringUtils.isNotBlank(pid)) {
+      if (!killEngineConnByPid(pid, engineStopRequest.getServiceInstance())) {
+        response.setStopStatus(false);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " failed.");
+      } else {
+        response.setStopStatus(true);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " succeed.");
+      }
+    } else {
+      String processPort = engineStopRequest.getServiceInstance().getInstance().split(":")[1];
+      logger.warn("Kill EC {} by port {}", engineStopRequest.getServiceInstance(), processPort);
+      if (!killEngineConnByPort(processPort, engineStopRequest.getServiceInstance())) {
+        response.setStopStatus(false);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " failed.");
+      } else {
+        response.setStopStatus(true);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " succeed.");
+      }
+      response.setStopStatus(false);

Review Comment:
   Need to remove



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224345579


##########
linkis-computation-governance/linkis-engineconn-manager/linkis-engineconn-manager-server/src/main/java/org/apache/linkis/ecm/server/service/impl/DefaultEngineConnKillService.java:
##########
@@ -0,0 +1,286 @@
+/*
+ * 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.linkis.ecm.server.service.impl;
+
+import org.apache.linkis.common.ServiceInstance;
+import org.apache.linkis.common.utils.Utils;
+import org.apache.linkis.ecm.server.conf.ECMConfiguration;
+import org.apache.linkis.ecm.server.service.EngineConnKillService;
+import org.apache.linkis.engineconn.common.conf.EngineConnConf;
+import org.apache.linkis.governance.common.utils.GovernanceUtils;
+import org.apache.linkis.manager.common.constant.AMConstant;
+import org.apache.linkis.manager.common.protocol.engine.EngineStopRequest;
+import org.apache.linkis.manager.common.protocol.engine.EngineStopResponse;
+import org.apache.linkis.manager.common.protocol.engine.EngineSuicideRequest;
+import org.apache.linkis.rpc.Sender;
+import org.apache.linkis.rpc.message.annotation.Receiver;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DefaultEngineConnKillService implements EngineConnKillService {
+
+  private static final Logger logger = LoggerFactory.getLogger(DefaultEngineConnKillService.class);
+
+  private static final ThreadPoolExecutor ecYarnAppKillService =
+      Utils.newCachedThreadPool(10, "ECM-Kill-EC-Yarn-App", true);
+
+  @Override
+  @Receiver
+  public EngineStopResponse dealEngineConnStop(EngineStopRequest engineStopRequest) {
+    logger.info("received EngineStopRequest " + engineStopRequest);
+    String pid = null;
+    if (AMConstant.PROCESS_MARK.equals(engineStopRequest.getIdentifierType())
+        && StringUtils.isNotBlank(engineStopRequest.getIdentifier())) {
+      pid = engineStopRequest.getIdentifier();
+    }
+    logger.info("dealEngineConnStop return pid: {}", pid);
+    EngineStopResponse response = new EngineStopResponse();
+    if (StringUtils.isNotBlank(pid)) {
+      if (!killEngineConnByPid(pid, engineStopRequest.getServiceInstance())) {
+        response.setStopStatus(false);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " failed.");
+      } else {
+        response.setStopStatus(true);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " succeed.");
+      }
+    } else {
+      String processPort = engineStopRequest.getServiceInstance().getInstance().split(":")[1];
+      logger.warn("Kill EC {} by port {}", engineStopRequest.getServiceInstance(), processPort);
+      if (!killEngineConnByPort(processPort, engineStopRequest.getServiceInstance())) {
+        response.setStopStatus(false);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " failed.");
+      } else {
+        response.setStopStatus(true);
+        response.setMsg(
+            "Kill engine " + engineStopRequest.getServiceInstance().toString() + " succeed.");
+      }
+      response.setStopStatus(false);

Review Comment:
   set   response.setStopStatus(false)?



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224320584


##########
linkis-computation-governance/linkis-computation-governance-common/src/main/scala/org/apache/linkis/governance/common/utils/GovernanceUtils.scala:
##########
@@ -73,13 +73,41 @@ object GovernanceUtils extends Logging {
     }
   }
 
+  def killProcessByPort(port: String, desc: String, isSudo: Boolean): Unit = {
+    val subProcessKillScriptPath = Configuration.getLinkisHome() + "/sbin/kill-process-by-port.sh"
+    if (
+        StringUtils.isBlank(subProcessKillScriptPath) || !new File(subProcessKillScriptPath)
+          .exists()
+    ) {
+      logger.error(s"Failed to locate kill-script, $subProcessKillScriptPath not exist")
+    } else if (StringUtils.isNotBlank(port)) {
+      val cmd = if (isSudo) {
+        Array("sudo", "sh", subProcessKillScriptPath, String.valueOf(port))

Review Comment:
   Input string type about  port   parameter
   String.valueOf(port)   can be removed
    



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] peacewong commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "peacewong (via GitHub)" <gi...@apache.org>.
peacewong commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1225234782


##########
linkis-computation-governance/linkis-manager/linkis-application-manager/src/main/java/org/apache/linkis/manager/am/service/em/DefaultEMEngineService.java:
##########
@@ -116,11 +114,18 @@ public void stopEngine(EngineNode engineNode, EMNode emNode) {
     engineStopRequest.setIdentifierType(engineNode.getMark());
     engineStopRequest.setIdentifier(engineNode.getIdentifier());
 
-    ECResourceInfoRecord ecResourceInfo =
-        ecResourceInfoService.getECResourceInfoRecordByInstance(
-            engineNode.getServiceInstance().getInstance());
+    ECResourceInfoRecord ecResourceInfo = null;
+    if (StringUtils.isNotBlank(engineNode.getTicketId())) {
+      ecResourceInfo = ecResourceInfoService.getECResourceInfoRecord(engineNode.getTicketId());
+    } else {
+      ecResourceInfo =
+          ecResourceInfoService.getECResourceInfoRecordByInstance(
+              engineNode.getServiceInstance().getInstance());
+    }
+    ecResourceInfoService.getECResourceInfoRecordByInstance(
+        engineNode.getServiceInstance().getInstance());

Review Comment:
   Yes need to remove



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224293636


##########
linkis-dist/package/sbin/kill-process-by-port.sh:
##########
@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+#
+# 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.
+#
+port=$1
+shellDir=`dirname $0`
+workDir=`cd ${shellDir}/..;pwd`
+if [ "$LINKIS_HOME" = "" ]
+then
+  LINKIS_HOME=$workDir
+fi
+pid=`ps -ef | grep server.port=$port | grep  EngineConnServer | awk '{print $2}'`
+echo "`date '+%Y-%m-%d %H:%M:%S'` Get port $port pid is $pid"
+if [ "$pid" != "" ]
+then
+  sh $LINKIS_HOME/sbin/kill-process-by-pid.sh $pid
+fi

Review Comment:
   it just kill ec service by given port maybe batter   adjust filename 
   kill-ec-process-by-port.sh



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] peacewong commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "peacewong (via GitHub)" <gi...@apache.org>.
peacewong commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1225234126


##########
linkis-dist/package/sbin/kill-process-by-port.sh:
##########
@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+#
+# 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.
+#
+port=$1
+shellDir=`dirname $0`
+workDir=`cd ${shellDir}/..;pwd`
+if [ "$LINKIS_HOME" = "" ]
+then
+  LINKIS_HOME=$workDir
+fi
+pid=`ps -ef | grep server.port=$port | grep  EngineConnServer | awk '{print $2}'`
+echo "`date '+%Y-%m-%d %H:%M:%S'` Get port $port pid is $pid"
+if [ "$pid" != "" ]
+then
+  sh $LINKIS_HOME/sbin/kill-process-by-pid.sh $pid
+fi

Review Comment:
   yes



##########
linkis-computation-governance/linkis-manager/linkis-manager-common/src/main/java/org/apache/linkis/manager/common/entity/persistence/ECResourceInfoRecord.java:
##########
@@ -87,6 +89,14 @@ public String getLabelValue() {
     return labelValue;
   }
 
+  public String getEngineType() {
+    if (StringUtils.isNotBlank(labelValue)) {
+      return labelValue.split(",")[1].split("-")[0];

Review Comment:
   ok



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] peacewong commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "peacewong (via GitHub)" <gi...@apache.org>.
peacewong commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1225235005


##########
linkis-computation-governance/linkis-computation-governance-common/src/main/scala/org/apache/linkis/governance/common/utils/GovernanceUtils.scala:
##########
@@ -73,13 +73,41 @@ object GovernanceUtils extends Logging {
     }
   }
 
+  def killProcessByPort(port: String, desc: String, isSudo: Boolean): Unit = {
+    val subProcessKillScriptPath = Configuration.getLinkisHome() + "/sbin/kill-process-by-port.sh"
+    if (
+        StringUtils.isBlank(subProcessKillScriptPath) || !new File(subProcessKillScriptPath)
+          .exists()
+    ) {
+      logger.error(s"Failed to locate kill-script, $subProcessKillScriptPath not exist")
+    } else if (StringUtils.isNotBlank(port)) {
+      val cmd = if (isSudo) {
+        Array("sudo", "sh", subProcessKillScriptPath, String.valueOf(port))

Review Comment:
   yes



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] peacewong commented on pull request #4608: ECM stateless feature optimization

Posted by "peacewong (via GitHub)" <gi...@apache.org>.
peacewong commented on PR #4608:
URL: https://github.com/apache/linkis/pull/4608#issuecomment-1585580553

   already updated, ping @casionone 


-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224336478


##########
linkis-computation-governance/linkis-engineconn-manager/linkis-engineconn-manager-server/src/main/scala/org/apache/linkis/ecm/server/service/impl/ProcessEngineConnLaunchService.scala:
##########
@@ -52,110 +48,79 @@ abstract class ProcessEngineConnLaunchService extends AbstractEngineConnLaunchSe
   def setLocalDirsHandleService(localDirsHandleService: LocalDirsHandleService): Unit =
     this.localDirsHandleService = localDirsHandleService
 
-  override def waitEngineConnStart(
+  override def startEngineConnMonitorStart(
       request: EngineConnLaunchRequest,
       conn: EngineConn,
       duration: Long

Review Comment:
   ec start timeout parameter  duration  no used? can remove? 
   



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224340285


##########
linkis-computation-governance/linkis-manager/linkis-application-manager/src/main/java/org/apache/linkis/manager/rm/service/impl/DefaultResourceManager.java:
##########
@@ -140,20 +140,39 @@ public void register(ServiceInstance serviceInstance, NodeResource resource) {
     eMInstanceLabel.setInstance(serviceInstance.getInstance());
 
     NodeResource emResource = labelResourceService.getLabelResource(eMInstanceLabel);
+    boolean registerResourceFlag = true;
     if (emResource != null) {
-      logger.warn(serviceInstance + " has been registered, now update resource.");
-      if (!emResource.getResourceType().equals(resource.getResourceType())) {
-        throw new RMWarnException(
-            RMErrorCode.LABEL_DUPLICATED.getErrorCode(),
-            MessageFormat.format(
-                RMErrorCode.LABEL_DUPLICATED.getErrorDesc(),
-                serviceInstance,
-                emResource.getResourceType(),
-                resource.getResourceType()));
+      registerResourceFlag = false;
+      logger.warn("ECM {} has been registered, resource is {}.", serviceInstance, emResource);
+      Resource leftResource = emResource.getLeftResource();
+      if (leftResource != null && Resource.getZeroResource(leftResource).moreThan(leftResource)) {
+        logger.warn(
+            "ECM {} has been registered, but left Resource <0 need to register resource.",
+            serviceInstance);
+        registerResourceFlag = true;
+      }
+      Resource usedResource = emResource.getLockedResource().add(emResource.getUsedResource());
+      if (usedResource.moreThan(emResource.getMaxResource())) {
+        logger.warn(
+            "ECM {}  has been registered, but usedResource > MaxResource  need to register resource.",
+            serviceInstance);
+        registerResourceFlag = true;
+      }
+      if (!(resource.getMaxResource().equalsTo(emResource.getMaxResource()))) {
+        logger.warn(
+            "ECM {} has been registered, but Inconsistent newly registered resources  need to register resource.",

Review Comment:
   Inconsistent ->inconsistent



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] peacewong commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "peacewong (via GitHub)" <gi...@apache.org>.
peacewong commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1225234139


##########
linkis-computation-governance/linkis-engineconn/linkis-engineconn-executor/accessible-executor/src/main/scala/org/apache/linkis/engineconn/callback/service/EngineConnCallback.scala:
##########
@@ -36,15 +36,15 @@ abstract class AbstractEngineConnStartUpCallback() extends EngineConnCallback wi
     protocol match {
       case protocol: EngineConnStatusCallback =>
         if (protocol.getStatus().equals(NodeStatus.Failed)) {
-          logger.error(s"protocol will send to lm: ${protocol}")
+          logger.error(s"EngineConn Start Failed protocol will send to lm: ${protocol}")
         } else {

Review Comment:
   yes



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224303604


##########
linkis-computation-governance/linkis-engineconn/linkis-engineconn-executor/accessible-executor/src/main/scala/org/apache/linkis/engineconn/callback/service/EngineConnCallback.scala:
##########
@@ -36,15 +36,15 @@ abstract class AbstractEngineConnStartUpCallback() extends EngineConnCallback wi
     protocol match {
       case protocol: EngineConnStatusCallback =>
         if (protocol.getStatus().equals(NodeStatus.Failed)) {
-          logger.error(s"protocol will send to lm: ${protocol}")
+          logger.error(s"EngineConn Start Failed protocol will send to lm: ${protocol}")
         } else {

Review Comment:
   lm => LM or LinkisManager  
   for easy to understand



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224299612


##########
linkis-computation-governance/linkis-manager/linkis-manager-common/src/main/java/org/apache/linkis/manager/common/entity/persistence/ECResourceInfoRecord.java:
##########
@@ -87,6 +89,14 @@ public String getLabelValue() {
     return labelValue;
   }
 
+  public String getEngineType() {
+    if (StringUtils.isNotBlank(labelValue)) {
+      return labelValue.split(",")[1].split("-")[0];

Review Comment:
   may add  annotation for labelValue eg "hadoop-IDE,spark-2.4.3 "



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone commented on a diff in pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone commented on code in PR #4608:
URL: https://github.com/apache/linkis/pull/4608#discussion_r1224306125


##########
linkis-computation-governance/linkis-manager/linkis-application-manager/src/main/java/org/apache/linkis/manager/am/service/em/DefaultEMEngineService.java:
##########
@@ -116,11 +114,18 @@ public void stopEngine(EngineNode engineNode, EMNode emNode) {
     engineStopRequest.setIdentifierType(engineNode.getMark());
     engineStopRequest.setIdentifier(engineNode.getIdentifier());
 
-    ECResourceInfoRecord ecResourceInfo =
-        ecResourceInfoService.getECResourceInfoRecordByInstance(
-            engineNode.getServiceInstance().getInstance());
+    ECResourceInfoRecord ecResourceInfo = null;
+    if (StringUtils.isNotBlank(engineNode.getTicketId())) {
+      ecResourceInfo = ecResourceInfoService.getECResourceInfoRecord(engineNode.getTicketId());
+    } else {
+      ecResourceInfo =
+          ecResourceInfoService.getECResourceInfoRecordByInstance(
+              engineNode.getServiceInstance().getInstance());
+    }
+    ecResourceInfoService.getECResourceInfoRecordByInstance(
+        engineNode.getServiceInstance().getInstance());

Review Comment:
   duplicate code?



-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org


[GitHub] [linkis] casionone merged pull request #4608: ECM stateless feature optimization

Posted by "casionone (via GitHub)" <gi...@apache.org>.
casionone merged PR #4608:
URL: https://github.com/apache/linkis/pull/4608


-- 
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: notifications-unsubscribe@linkis.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@linkis.apache.org
For additional commands, e-mail: notifications-help@linkis.apache.org