[GitHub] [linkis] peacewong commented on a diff in pull request #4452: feat: do not kill ec when ecm restart part-1 (#4184)

["peacewong (via GitHub)" <gi...@apache.org>]

peacewong commented on code in PR #4452:
URL: https://github.com/apache/linkis/pull/4452#discussion_r1169745205


##########
linkis-computation-governance/linkis-engineconn/linkis-computation-engineconn/src/test/scala/org/apache/linkis/engineconn/computation/executor/upstream/access/ECTaskEntranceInfoAccessHelper.scala:
##########
@@ -53,7 +53,10 @@ object ECTaskEntranceInfoAccessHelper {
     val host = CommonVars(Environment.ECM_HOST.toString, "127.0.0.1").getValue
     val port = CommonVars(Environment.ECM_PORT.toString, "80").getValue
     engineCreationContext.setEMInstance(

Review Comment:
   can remove this file



##########
linkis-computation-governance/linkis-computation-governance-common/src/main/scala/org/apache/linkis/governance/common/conf/GovernanceCommonConf.scala:
##########
@@ -43,6 +43,9 @@ object GovernanceCommonConf {
   val ENGINE_CONN_MANAGER_SPRING_NAME =
     CommonVars("wds.linkis.engineconn.manager.name", "linkis-cg-engineconnmanager")
 
+  val ENGINE_APPLICATION_MANAGER_SPRING_NAME =

Review Comment:
   MANAGER_SERVICE_NAME should also be changed to this



##########
linkis-computation-governance/linkis-manager/linkis-application-manager/src/main/scala/org/apache/linkis/manager/am/manager/DefaultEngineNodeManager.scala:
##########
@@ -105,7 +110,11 @@ class DefaultEngineNodeManager extends EngineNodeManager with Logging {
     val heartMsg = engine.getNodeHeartbeatMsg()
     engineNode.setNodeHealthyInfo(heartMsg.getHealthyInfo)
     engineNode.setNodeOverLoadInfo(heartMsg.getOverLoadInfo)
-    engineNode.setNodeResource(heartMsg.getNodeResource)
+
+    // get node resource from DB
+    val rmNodes: util.List[RMNode] =
+      resourceManager.getResourceInfo(Array(engineNode.getServiceInstance)).resourceInfo
+    engineNode.setNodeResource(rmNodes.get(0).getNodeResource)

Review Comment:
   can remove node resource，not used



##########
linkis-computation-governance/linkis-engineconn-manager/linkis-engineconn-manager-server/src/main/scala/org/apache/linkis/ecm/server/service/impl/DefaultEngineConnKillService.java:
##########
@@ -220,6 +187,20 @@ private boolean isProcessAlive(String pid) {
         }
     }
 
+    private String findProcessId(String processPort){
+        String findCmd = "lsof -t -i:" + processPort;
+        List<String> cmdList = new ArrayList<>();

Review Comment:
   need to use sudo and move this method to GovernanceUtils



##########
linkis-computation-governance/linkis-engineconn/linkis-engineconn-core/src/main/scala/org/apache/linkis/engineconn/launch/EngineConnServer.scala:
##########
@@ -122,11 +122,6 @@ object EngineConnServer extends Logging {
     val engineConf = arguments.getEngineConnConfMap
     this.engineCreationContext.setUser(engineConf.getOrElse("user", Utils.getJvmUser))
     this.engineCreationContext.setTicketId(engineConf.getOrElse(ECConstants.EC_TICKET_ID_KEY, ""))
-    val host = CommonVars(Environment.ECM_HOST.toString, "127.0.0.1").getValue

Review Comment:
   Recommended to keep



##########
linkis-computation-governance/linkis-manager/linkis-application-manager/src/main/scala/org/apache/linkis/manager/am/service/em/DefaultEMEngineService.scala:
##########
@@ -95,6 +100,17 @@ class DefaultEMEngineService extends EMEngineService with Logging {
     )
     val engineStopRequest = new EngineStopRequest
     engineStopRequest.setServiceInstance(engineNode.getServiceInstance)
+
+    val ecInfo: ECResourceInfoRecord =
+      ecResourceInfoService.getECResourceInfoRecordByInstance(
+        engineNode.getServiceInstance.getInstance
+      )
+    // append engineType and logDirSuffix
+    if (ecInfo != null) {

Review Comment:
   if ec Info is null should to add EngineType and logSuffix 
   
   1. can add labels in DefaultEngineStopService:node.setLabels(nodeLabelService.getNodeLabels(engineStopRequest.getServiceInstance)) and to get EngintTypeLabel 
   2. get log suffix can to invoke org.apache.linkis.manager.rm.service.impl.ResourceLogService#getECLogDirSuffix (add to utils)



-- 
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