BIGTOP-4341: Only require changed configs to reduce request body size (…

…#161)
apache · Jan 26, 2025 · 796b42f · 796b42f
1 parent 6b4d151
commit 796b42f
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 30 deletions.
diff --git a/...agent/src/main/java/org/apache/bigtop/manager/agent/grpc/interceptor/TaskInterceptor.java b/...agent/src/main/java/org/apache/bigtop/manager/agent/grpc/interceptor/TaskInterceptor.java
@@ -50,9 +50,7 @@ public void onMessage(ReqT message) {
                 if (isTaskRequest(message)) {
                     try {
                         Method method = message.getClass().getDeclaredMethod("getTaskId");
-                        method.setAccessible(true);
                         Long taskId = (Long) method.invoke(message);
-                        method.setAccessible(false);
                         truncateLogFile(taskId);
                         MDC.put("taskId", String.valueOf(taskId));
                         Caches.RUNNING_TASK = taskId;

diff --git a/bigtop-manager-grpc/src/main/resources/proto/component_command.proto b/bigtop-manager-grpc/src/main/resources/proto/component_command.proto
@@ -18,8 +18,6 @@
  */
 syntax = "proto3";
 
-import "google/protobuf/struct.proto";
-
 option java_multiple_files = true;
 option java_package = "org.apache.bigtop.manager.grpc.generated";
 option java_outer_classname = "ComponentCommandProto";

diff --git a/bigtop-manager-grpc/src/main/resources/proto/job_cache.proto b/bigtop-manager-grpc/src/main/resources/proto/job_cache.proto
@@ -18,8 +18,6 @@
  */
 syntax = "proto3";
 
-import "google/protobuf/struct.proto";
-
 option java_multiple_files = true;
 option java_package = "org.apache.bigtop.manager.grpc.generated";
 option java_outer_classname = "JobCacheProto";

diff --git a/...ver/src/main/java/org/apache/bigtop/manager/server/command/job/service/ServiceAddJob.java b/...ver/src/main/java/org/apache/bigtop/manager/server/command/job/service/ServiceAddJob.java
@@ -38,6 +38,7 @@
 import org.apache.bigtop.manager.server.model.dto.ServiceDTO;
 import org.apache.bigtop.manager.server.model.dto.StackDTO;
 import org.apache.bigtop.manager.server.model.dto.command.ServiceCommandDTO;
+import org.apache.bigtop.manager.server.utils.StackConfigUtils;
 import org.apache.bigtop.manager.server.utils.StackUtils;
 
 import java.util.ArrayList;
@@ -169,8 +170,10 @@ private void saveService(ServiceCommandDTO serviceCommand) {
 
         // Persist current configs
         Map<String, String> confMap = new HashMap<>();
-        List<ServiceConfigDTO> configs = serviceCommand.getConfigs();
-        List<ServiceConfigPO> serviceConfigPOList = ServiceConfigConverter.INSTANCE.fromDTO2PO(configs);
+        List<ServiceConfigDTO> oriConfigs = StackUtils.SERVICE_CONFIG_MAP.get(serviceName);
+        List<ServiceConfigDTO> newConfigs = serviceCommand.getConfigs();
+        List<ServiceConfigDTO> mergedConfigs = StackConfigUtils.mergeServiceConfigs(oriConfigs, newConfigs);
+        List<ServiceConfigPO> serviceConfigPOList = ServiceConfigConverter.INSTANCE.fromDTO2PO(mergedConfigs);
         for (ServiceConfigPO serviceConfigPO : serviceConfigPOList) {
             serviceConfigPO.setClusterId(clusterId);
             serviceConfigPO.setServiceId(servicePO.getId());

diff --git a/...rc/main/java/org/apache/bigtop/manager/server/model/converter/ServiceConfigConverter.java b/...rc/main/java/org/apache/bigtop/manager/server/model/converter/ServiceConfigConverter.java
@@ -21,6 +21,7 @@
 import org.apache.bigtop.manager.dao.po.ServiceConfigPO;
 import org.apache.bigtop.manager.server.config.MapStructSharedConfig;
 import org.apache.bigtop.manager.server.model.dto.ServiceConfigDTO;
+import org.apache.bigtop.manager.server.model.req.ServiceConfigReq;
 import org.apache.bigtop.manager.server.model.vo.ServiceConfigVO;
 
 import org.mapstruct.Mapper;
@@ -36,6 +37,10 @@ public interface ServiceConfigConverter {
 
     ServiceConfigConverter INSTANCE = Mappers.getMapper(ServiceConfigConverter.class);
 
+    ServiceConfigDTO fromReq2DTO(ServiceConfigReq serviceConfigReq);
+
+    List<ServiceConfigDTO> fromReq2DTO(List<ServiceConfigReq> serviceConfigReqList);
+
     @Mapping(target = "properties", source = "propertiesJson", qualifiedByName = "json2PropertyDTOList")
     ServiceConfigDTO fromPO2DTO(ServiceConfigPO serviceConfigPO);
 

diff --git a/...erver/src/main/java/org/apache/bigtop/manager/server/service/impl/ServiceServiceImpl.java b/...erver/src/main/java/org/apache/bigtop/manager/server/service/impl/ServiceServiceImpl.java
@@ -34,6 +34,7 @@
 import org.apache.bigtop.manager.server.model.converter.ServiceConfigConverter;
 import org.apache.bigtop.manager.server.model.converter.ServiceConfigSnapshotConverter;
 import org.apache.bigtop.manager.server.model.converter.ServiceConverter;
+import org.apache.bigtop.manager.server.model.dto.ServiceConfigDTO;
 import org.apache.bigtop.manager.server.model.query.PageQuery;
 import org.apache.bigtop.manager.server.model.req.ServiceConfigReq;
 import org.apache.bigtop.manager.server.model.req.ServiceConfigSnapshotReq;
@@ -43,6 +44,8 @@
 import org.apache.bigtop.manager.server.model.vo.ServiceVO;
 import org.apache.bigtop.manager.server.service.ServiceService;
 import org.apache.bigtop.manager.server.utils.PageUtils;
+import org.apache.bigtop.manager.server.utils.StackConfigUtils;
+import org.apache.bigtop.manager.server.utils.StackUtils;
 
 import org.apache.commons.collections4.CollectionUtils;
 
@@ -54,7 +57,6 @@
 import lombok.extern.slf4j.Slf4j;
 
 import jakarta.annotation.Resource;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -116,15 +118,26 @@ public List<ServiceConfigVO> listConf(Long clusterId, Long serviceId) {
 
     @Override
     public List<ServiceConfigVO> updateConf(Long clusterId, Long serviceId, List<ServiceConfigReq> reqs) {
-        List<ServiceConfigPO> list = new ArrayList<>();
-        for (ServiceConfigReq req : reqs) {
-            ServiceConfigPO serviceConfigPO = new ServiceConfigPO();
-            serviceConfigPO.setId(req.getId());
-            serviceConfigPO.setPropertiesJson(JsonUtils.writeAsString(req.getProperties()));
-            list.add(serviceConfigPO);
-        }
-
-        serviceConfigDao.partialUpdateByIds(list);
+        ServicePO servicePO = serviceDao.findById(serviceId);
+        List<ServiceConfigPO> configs = serviceConfigDao.findByServiceId(serviceId);
+
+        List<ServiceConfigDTO> oriConfigs;
+        List<ServiceConfigDTO> newConfigs;
+        List<ServiceConfigDTO> mergedConfigs;
+
+        // Merge stack config with existing config first, in case new property has been added to stack config.
+        oriConfigs = StackUtils.SERVICE_CONFIG_MAP.get(servicePO.getName());
+        newConfigs = ServiceConfigConverter.INSTANCE.fromPO2DTO(configs);
+        mergedConfigs = StackConfigUtils.mergeServiceConfigs(oriConfigs, newConfigs);
+
+        // Merge existing config with new config in request object
+        oriConfigs = mergedConfigs;
+        newConfigs = ServiceConfigConverter.INSTANCE.fromReq2DTO(reqs);
+        mergedConfigs = StackConfigUtils.mergeServiceConfigs(oriConfigs, newConfigs);
+
+        // Save merged config
+        List<ServiceConfigPO> serviceConfigPOList = ServiceConfigConverter.INSTANCE.fromDTO2PO(mergedConfigs);
+        serviceConfigDao.partialUpdateByIds(serviceConfigPOList);
         return listConf(clusterId, serviceId);
     }
 

diff --git a/...manager-server/src/main/java/org/apache/bigtop/manager/server/utils/StackConfigUtils.java b/...manager-server/src/main/java/org/apache/bigtop/manager/server/utils/StackConfigUtils.java
@@ -20,12 +20,17 @@
 
 import org.apache.bigtop.manager.server.model.dto.AttrsDTO;
 import org.apache.bigtop.manager.server.model.dto.PropertyDTO;
+import org.apache.bigtop.manager.server.model.dto.ServiceConfigDTO;
 import org.apache.bigtop.manager.server.stack.model.AttrsModel;
 import org.apache.bigtop.manager.server.stack.model.PropertyModel;
 import org.apache.bigtop.manager.server.stack.xml.ConfigurationXml;
 
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
 import org.apache.commons.lang3.StringUtils;
 
+import org.springframework.beans.BeanUtils;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -68,23 +73,81 @@ private static PropertyDTO getPropertyDTO(PropertyModel propertyModel) {
         return propertyDTO;
     }
 
+    public static List<ServiceConfigDTO> mergeServiceConfigs(
+            List<ServiceConfigDTO> oriConfigs, List<ServiceConfigDTO> overrideConfigs) {
+        // To avoid to change the original configs, we use cloned object
+        List<ServiceConfigDTO> mergedConfigs = new ArrayList<>();
+        for (ServiceConfigDTO oriConfig : oriConfigs) {
+            ServiceConfigDTO mergedConfig = new ServiceConfigDTO();
+            BeanUtils.copyProperties(oriConfig, mergedConfig);
+            mergedConfigs.add(mergedConfig);
+        }
+
+        if (CollectionUtils.isEmpty(overrideConfigs)) {
+            return mergedConfigs;
+        }
+
+        // Assign id for each service config
+        for (ServiceConfigDTO config : mergedConfigs) {
+            config.setId(overrideConfigs.stream()
+                    .filter(x -> x.getName().equals(config.getName()))
+                    .findFirst()
+                    .map(ServiceConfigDTO::getId)
+                    .orElse(null));
+        }
+
+        Map<String, Map<String, String>> overrideConfigsMap = serviceConfig2Map(overrideConfigs);
+        for (ServiceConfigDTO mergedConfig : mergedConfigs) {
+            String configName = mergedConfig.getName();
+            if (!overrideConfigsMap.containsKey(configName)) {
+                continue;
+            }
+
+            // Override existing properties
+            Map<String, String> overridePropertiesMap = overrideConfigsMap.get(configName);
+            for (PropertyDTO property : mergedConfig.getProperties()) {
+                String propertyName = property.getName();
+                String value = overridePropertiesMap.remove(propertyName);
+                if (value != null) {
+                    property.setValue(value);
+                }
+            }
+
+            // We may still have some properties added by user manually
+            if (MapUtils.isNotEmpty(overridePropertiesMap)) {
+                for (Map.Entry<String, String> entry : overridePropertiesMap.entrySet()) {
+                    PropertyDTO property = new PropertyDTO();
+                    property.setName(entry.getKey());
+                    property.setValue(entry.getValue());
+                    mergedConfig.getProperties().add(property);
+                }
+            }
+        }
+
+        return mergedConfigs;
+    }
+
     /**
-     * extract config from List<Map<String,Object>> to Map<String,Object>
+     * extract config from List<ServiceConfigDTO> to Map<String, Map<String, String>>
      *
-     * @param list List<PropertyModel>
-     * @return Map<String, Object>
+     * @param configs List<ServiceConfigDTO>
+     * @return Map<String, Map<String, String>>
      */
-    public static Map<String, Object> extractConfigMap(List<PropertyDTO> list) {
-        if (list == null) {
-            return null;
+    private static Map<String, Map<String, String>> serviceConfig2Map(List<ServiceConfigDTO> configs) {
+        Map<String, Map<String, String>> outerMap = new HashMap<>();
+        if (CollectionUtils.isEmpty(configs)) {
+            return outerMap;
         }
 
-        Map<String, Object> hashMap = new HashMap<>();
-        for (PropertyDTO property : list) {
-            String key = property.getName();
-            Object value = property.getValue();
-            hashMap.put(key, value);
+        for (ServiceConfigDTO config : configs) {
+            Map<String, String> innerMap = new HashMap<>();
+            for (PropertyDTO property : config.getProperties()) {
+                innerMap.put(property.getName(), property.getValue());
+            }
+
+            outerMap.put(config.getName(), innerMap);
         }
-        return hashMap;
+
+        return outerMap;
     }
 }