From 5efea6662e1ba541603507cac29290c2d40ab73f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=BF=9F=E6=88=90?= Date: Mon, 6 Sep 2021 20:05:12 +0800 Subject: [PATCH] #6508 custom config handler --- .../org/apache/doris/common/ConfigBase.java | 204 ++++++++---------- .../doris/http/rest/SetConfigAction.java | 50 ++--- .../doris/httpv2/rest/SetConfigAction.java | 68 ++---- .../doris/httpv2/rest/manager/NodeAction.java | 33 ++- .../org/apache/doris/qe/ShowExecutor.java | 24 +-- .../analysis/AdminSetConfigStmtTest.java | 2 +- 6 files changed, 137 insertions(+), 244 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/ConfigBase.java b/fe/fe-core/src/main/java/org/apache/doris/common/ConfigBase.java index b7298aa7dfdddc..1287c19d2bcd0d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/ConfigBase.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/ConfigBase.java @@ -17,6 +17,7 @@ package org.apache.doris.common; +import org.apache.doris.catalog.Catalog; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -39,21 +40,36 @@ import java.util.Properties; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class ConfigBase { private static final Logger LOG = LogManager.getLogger(ConfigBase.class); @Retention(RetentionPolicy.RUNTIME) - public static @interface ConfField { + public @interface ConfField { String value() default ""; boolean mutable() default false; boolean masterOnly() default false; String comment() default ""; + Class callback() default DefaultConfHandler.class; + } + + public interface ConfHandler { + void handle(Field field, String confVal) throws Exception; + } + + static class DefaultConfHandler implements ConfHandler { + @Override + public void handle(Field field, String confVal) throws Exception{ + setConfigField(field, confVal); + } } private static String confFile; private static String customConfFile; public static Class confClass; + public static Map confFields; private static String ldapConfFile; private static String ldapCustomConfFile; @@ -66,6 +82,15 @@ public void init(String configFile) throws Exception { if (!isLdapConfig) { confClass = this.getClass(); confFile = configFile; + confFields = Maps.newHashMap(); + for (Field field : confClass.getFields()) { + ConfField confField = field.getAnnotation(ConfField.class); + if (confField == null) { + continue; + } + confFields.put(confField.value().equals("") ? field.getName() : confField.value(), field); + } + initConf(confFile); } else { ldapConfClass = this.getClass(); @@ -90,42 +115,48 @@ private void initConf(String confFile) throws Exception { replacedByEnv(props); setFields(props, isLdapConfig); } - - public static HashMap dump() throws Exception { - HashMap map = new HashMap(); - Field[] fields = confClass.getFields(); + + public static HashMap dump() { + HashMap map = new HashMap<>(); + Field[] fields = confClass.getFields(); for (Field f : fields) { - if (f.getAnnotation(ConfField.class) == null) { - continue; + ConfField anno = f.getAnnotation(ConfField.class); + if (anno != null) { + map.put(anno.value().isEmpty() ? f.getName() : anno.value(), getConfValue(f)); } - if (f.getType().isArray()) { - switch (f.getType().getSimpleName()) { + } + return map; + } + + public static String getConfValue(Field field) { + try { + if (field.getType().isArray()) { + switch (field.getType().getSimpleName()) { + case "boolean[]": + return Arrays.toString((boolean[]) field.get(null)); + case "char[]": + return Arrays.toString((char[]) field.get(null)); + case "byte[]": + return Arrays.toString((byte[]) field.get(null)); case "short[]": - map.put(f.getName(), Arrays.toString((short[]) f.get(null))); - break; + return Arrays.toString((short[]) field.get(null)); case "int[]": - map.put(f.getName(), Arrays.toString((int[]) f.get(null))); - break; + return Arrays.toString((int[]) field.get(null)); case "long[]": - map.put(f.getName(), Arrays.toString((long[]) f.get(null))); - break; + return Arrays.toString((long[]) field.get(null)); + case "float[]": + return Arrays.toString((float[]) field.get(null)); case "double[]": - map.put(f.getName(), Arrays.toString((double[]) f.get(null))); - break; - case "boolean[]": - map.put(f.getName(), Arrays.toString((boolean[]) f.get(null))); - break; - case "String[]": - map.put(f.getName(), Arrays.toString((String[]) f.get(null))); - break; + return Arrays.toString((double[]) field.get(null)); default: - throw new Exception("unknown type: " + f.getType().getSimpleName()); - } + return Arrays.toString((Object[]) field.get(null)); + } } else { - map.put(f.getName(), f.get(null).toString()); + return String.valueOf(field.get(null)); } + } catch (Exception e) { + return String.format("Failed to get config %s: %s", field.getName(), e.getMessage()); } - return map; } // there is some config in fe.conf like: @@ -176,7 +207,7 @@ private static void setFields(Properties props, boolean isLdapConfig) throws Exc } } - public static void setConfigField(Field f, String confVal) throws IllegalAccessException, Exception { + public static void setConfigField(Field f, String confVal) throws Exception { confVal = confVal.trim(); String[] sa = confVal.split(","); @@ -258,114 +289,57 @@ private static boolean isBoolean(String s) { throw new IllegalArgumentException("type mismatch"); } - public static Map getAllMutableConfigs() { - Map mutableConfigs = Maps.newHashMap(); - Field fields[] = ConfigBase.confClass.getFields(); - for (Field field : fields) { - ConfField confField = field.getAnnotation(ConfField.class); - if (confField == null) { - continue; - } - if (!confField.mutable()) { - continue; - } - mutableConfigs.put(confField.value().equals("") ? field.getName() : confField.value(), field); - } - - return mutableConfigs; - } - public synchronized static void setMutableConfig(String key, String value) throws DdlException { - Map mutableConfigs = getAllMutableConfigs(); - Field field = mutableConfigs.get(key); + Field field = confFields.get(key); if (field == null) { - throw new DdlException("Config '" + key + "' does not exist or is not mutable"); + throw new DdlException("Config '" + key + "' does not exist"); + } + + ConfField anno = field.getAnnotation(ConfField.class); + if (!anno.mutable()) { + throw new DdlException("Config '" + key + "' is not mutable"); + } + if (anno.masterOnly() && !Catalog.getCurrentCatalog().isMaster()){ + throw new DdlException("Config '" + key + "' is master only"); } try { - ConfigBase.setConfigField(field, value); + anno.callback().newInstance().handle(field, value); } catch (Exception e) { throw new DdlException("Failed to set config '" + key + "'. err: " + e.getMessage()); } - + LOG.info("set config {} to {}", key, value); } - public synchronized static List> getConfigInfo(PatternMatcher matcher) throws DdlException { - List> configs = Lists.newArrayList(); - Field[] fields = confClass.getFields(); - for (Field f : fields) { - List config = Lists.newArrayList(); + public synchronized static List> getConfigInfo(PatternMatcher matcher) { + return confFields.entrySet().stream().sorted(Map.Entry.comparingByKey()).flatMap(e -> { + String confKey = e.getKey(); + Field f = e.getValue(); ConfField anno = f.getAnnotation(ConfField.class); - if (anno == null) { - continue; - } - - String confKey = anno.value().equals("") ? f.getName() : anno.value(); - if (matcher != null && !matcher.match(confKey)) { - continue; - } - String confVal; - try { - switch (f.getType().getSimpleName()) { - case "short": - case "int": - case "long": - case "double": - case "boolean": - case "String": - confVal = String.valueOf(f.get(null)); - break; - case "short[]": - confVal = Arrays.toString((short[])f.get(null)); - break; - case "int[]": - confVal = Arrays.toString((int[])f.get(null)); - break; - case "long[]": - confVal = Arrays.toString((long[])f.get(null)); - break; - case "double[]": - confVal = Arrays.toString((double[])f.get(null)); - break; - case "boolean[]": - confVal = Arrays.toString((boolean[])f.get(null)); - break; - case "String[]": - confVal = Arrays.toString((String[])f.get(null)); - break; - default: - throw new DdlException("unknown type: " + f.getType().getSimpleName()); - } - } catch (IllegalArgumentException | IllegalAccessException e) { - throw new DdlException("Failed to get config '" + confKey + "'. err: " + e.getMessage()); + if (matcher == null || matcher.match(confKey)) { + List config = Lists.newArrayList(); + config.add(confKey); + config.add(getConfValue(f)); + config.add(f.getType().getSimpleName()); + config.add(String.valueOf(anno.mutable())); + config.add(String.valueOf(anno.masterOnly())); + config.add(anno.comment()); + return Stream.of(config); + } else { + return Stream.empty(); } - - config.add(confKey); - config.add(Strings.nullToEmpty(confVal)); - config.add(f.getType().getSimpleName()); - config.add(String.valueOf(anno.mutable())); - config.add(String.valueOf(anno.masterOnly())); - config.add(anno.comment()); - configs.add(config); - } - - return configs; + }).collect(Collectors.toList()); } public synchronized static boolean checkIsMasterOnly(String key) { - Map mutableConfigs = getAllMutableConfigs(); - Field f = mutableConfigs.get(key); + Field f = confFields.get(key); if (f == null) { return false; } ConfField anno = f.getAnnotation(ConfField.class); - if (anno == null) { - return false; - } - - return anno.masterOnly(); + return anno != null && anno.mutable() && anno.masterOnly(); } // use synchronized to make sure only one thread modify this file diff --git a/fe/fe-core/src/main/java/org/apache/doris/http/rest/SetConfigAction.java b/fe/fe-core/src/main/java/org/apache/doris/http/rest/SetConfigAction.java index a79a2beaf0eec9..f096b10bf7da93 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/http/rest/SetConfigAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/http/rest/SetConfigAction.java @@ -19,9 +19,7 @@ import io.netty.handler.codec.http.HttpMethod; -import org.apache.doris.catalog.Catalog; import org.apache.doris.common.ConfigBase; -import org.apache.doris.common.ConfigBase.ConfField; import org.apache.doris.common.DdlException; import org.apache.doris.http.ActionController; import org.apache.doris.http.BaseRequest; @@ -37,7 +35,6 @@ import org.codehaus.jackson.map.ObjectMapper; import java.io.IOException; -import java.lang.reflect.Field; import java.util.List; import java.util.Map; @@ -86,37 +83,20 @@ protected void executeWithoutPassword(BaseRequest request, BaseResponse response LOG.debug("get config from url: {}, need persist: {}", configs, needPersist); - Field[] fields = ConfigBase.confClass.getFields(); - for (Field f : fields) { - // ensure that field has "@ConfField" annotation - ConfField anno = f.getAnnotation(ConfField.class); - if (anno == null || !anno.mutable()) { - continue; - } - - if (anno.masterOnly() && !Catalog.getCurrentCatalog().isMaster()) { - continue; - } - - // ensure that field has property string - String confKey = anno.value().equals("") ? f.getName() : anno.value(); - List confVals = configs.get(confKey); - if (confVals == null || confVals.isEmpty()) { - continue; - } - - if (confVals.size() > 1) { - continue; - } - + for (Map.Entry> config : configs.entrySet()) { + String confKey = config.getKey(); + List confValue = config.getValue(); try { - ConfigBase.setConfigField(f, confVals.get(0)); - } catch (Exception e) { - LOG.warn("failed to set config {}:{}", confKey, confVals.get(0), e); - continue; + if (confValue != null && confValue.size() == 1) { + ConfigBase.setMutableConfig(confKey, confValue.get(0)); + setConfigs.put(confKey, confValue.get(0)); + } else { + throw new DdlException("conf value size != 1"); + } + } catch (DdlException e) { + LOG.warn("failed to set config {}:{}", confKey, confValue, e); + errConfigs.put(confKey, String.valueOf(confValue)); } - - setConfigs.put(confKey, confVals.get(0)); } String persistMsg = ""; @@ -130,12 +110,6 @@ protected void executeWithoutPassword(BaseRequest request, BaseResponse response } } - for (String key : configs.keySet()) { - if (!setConfigs.containsKey(key)) { - errConfigs.put(key, configs.get(key).toString()); - } - } - Map resultMap = Maps.newHashMap(); resultMap.put("set", setConfigs); resultMap.put("err", errConfigs); diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/SetConfigAction.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/SetConfigAction.java index 7124c338e387ff..014efcf904c57a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/SetConfigAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/SetConfigAction.java @@ -20,9 +20,8 @@ import lombok.AllArgsConstructor; import lombok.Getter; import lombok.Setter; -import org.apache.doris.catalog.Catalog; import org.apache.doris.common.ConfigBase; -import org.apache.doris.common.ConfigBase.ConfField; +import org.apache.doris.common.DdlException; import org.apache.doris.httpv2.entity.ResponseEntityBuilder; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.qe.ConnectContext; @@ -39,10 +38,9 @@ import org.springframework.web.bind.annotation.RestController; import java.io.IOException; -import java.lang.reflect.Field; +import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -88,49 +86,20 @@ protected Object set_config(HttpServletRequest request, HttpServletResponse resp LOG.debug("get config from url: {}, need persist: {}", configs, needPersist); - Field[] fields = ConfigBase.confClass.getFields(); - for (Field f : fields) { - // ensure that field has "@ConfField" annotation - ConfField anno = f.getAnnotation(ConfField.class); - - if (anno == null) { - continue; - } - - // ensure that field has property string - String confKey = anno.value().equals("") ? f.getName() : anno.value(); - String[] confVals = configs.get(confKey); - if (confVals == null) { - continue; - } - - if (confVals.length != 1) { - errConfigs.add(new ErrConfig(confKey, "", "No or multiple configuration values.")); - continue; - } - - if (!anno.mutable()) { - errConfigs.add(new ErrConfig(confKey, confVals[0], "Not support dynamic modification.")); - continue; - } - - if (anno.masterOnly() && !Catalog.getCurrentCatalog().isMaster()) { - errConfigs.add(new ErrConfig(confKey, confVals[0], "Not support modification on non-master")); - continue; - } - + for (Map.Entry config : configs.entrySet()) { + String confKey = config.getKey(); + String[] confValue = config.getValue(); try { - ConfigBase.setConfigField(f, confVals[0]); - } catch (IllegalArgumentException e){ - errConfigs.add(new ErrConfig(confKey, confVals[0], "Unsupported configuration value type.")); - continue; - } catch (Exception e) { - LOG.warn("failed to set config {}:{}, {}", confKey, confVals[0], e.getMessage()); - errConfigs.add(new ErrConfig(confKey, confVals[0], e.getMessage())); - continue; + if (confValue != null && confValue.length == 1) { + ConfigBase.setMutableConfig(confKey, confValue[0]); + setConfigs.put(confKey, confValue[0]); + } else { + throw new DdlException("conf value size != 1"); + } + } catch (DdlException e) { + LOG.warn("failed to set config {}:{}", confKey, Arrays.toString(confValue), e); + errConfigs.add(new ErrConfig(confKey, Arrays.toString(confValue), e.getMessage())); } - - setConfigs.put(confKey, confVals[0]); } String persistMsg = ""; @@ -144,15 +113,6 @@ protected Object set_config(HttpServletRequest request, HttpServletResponse resp } } - List errConfigNames = errConfigs.stream().map(ErrConfig::getConfigName).collect(Collectors.toList()); - for (String key : configs.keySet()) { - if (!setConfigs.containsKey(key) && !errConfigNames.contains(key)) { - String[] confVals = configs.get(key); - String confVal = confVals.length == 1 ? confVals[0] : "invalid value"; - errConfigs.add(new ErrConfig(key, confVal, "invalid config")); - } - } - return ResponseEntityBuilder.ok(new SetConfigEntity(setConfigs, errConfigs, persistMsg)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java index fa05289ce24a8c..9bbb3c46c756e7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java @@ -23,7 +23,6 @@ import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ConfigBase; -import org.apache.doris.common.DdlException; import org.apache.doris.common.MarkedCountDownLatch; import org.apache.doris.common.Pair; import org.apache.doris.common.ThreadPoolManager; @@ -57,7 +56,6 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; -import java.util.Collections; import java.util.Comparator; import java.util.Iterator; import java.util.List; @@ -251,25 +249,20 @@ public Object config(HttpServletRequest request, HttpServletResponse response) { executeCheckPassword(request, response); checkGlobalAuth(ConnectContext.get().getCurrentUserIdentity(), PrivPredicate.ADMIN); + List> configs = ConfigBase.getConfigInfo(null); + // Sort all configs by config key. + configs.sort(Comparator.comparing(o -> o.get(0))); + + // reorder the fields List> results = Lists.newArrayList(); - try { - List> configs = ConfigBase.getConfigInfo(null); - // Sort all configs by config key. - Collections.sort(configs, Comparator.comparing(o -> o.get(0))); - - // reorder the fields - for (List config : configs) { - List list = Lists.newArrayList(); - list.add(config.get(0)); - list.add(config.get(2)); - list.add(config.get(4)); - list.add(config.get(1)); - list.add(config.get(3)); - results.add(list); - } - } catch (DdlException e) { - LOG.warn(e); - return ResponseEntityBuilder.internalError(e.getMessage()); + for (List config : configs) { + List list = Lists.newArrayList(); + list.add(config.get(0)); + list.add(config.get(2)); + list.add(config.get(4)); + list.add(config.get(1)); + list.add(config.get(3)); + results.add(list); } return results; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java index 8778104433c616..3346338c8a5c37 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java @@ -1754,23 +1754,15 @@ private void handleAdminShowTabletDistribution() throws AnalysisException { private void handleAdminShowConfig() throws AnalysisException { AdminShowConfigStmt showStmt = (AdminShowConfigStmt) stmt; List> results; - try { - PatternMatcher matcher = null; - if (showStmt.getPattern() != null) { - matcher = PatternMatcher.createMysqlPattern(showStmt.getPattern(), - CaseSensibility.CONFIG.getCaseSensibility()); - } - results = ConfigBase.getConfigInfo(matcher); - // Sort all configs by config key. - Collections.sort(results, new Comparator>() { - @Override - public int compare(List o1, List o2) { - return o1.get(0).compareTo(o2.get(0)); - } - }); - } catch (DdlException e) { - throw new AnalysisException(e.getMessage()); + + PatternMatcher matcher = null; + if (showStmt.getPattern() != null) { + matcher = PatternMatcher.createMysqlPattern(showStmt.getPattern(), + CaseSensibility.CONFIG.getCaseSensibility()); } + results = ConfigBase.getConfigInfo(matcher); + // Sort all configs by config key. + results.sort(Comparator.comparing(o -> o.get(0))); resultSet = new ShowResultSet(showStmt.getMetaData(), results); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/AdminSetConfigStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/AdminSetConfigStmtTest.java index f6ce7409ac2a70..52b6758e5bb5fb 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/AdminSetConfigStmtTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/AdminSetConfigStmtTest.java @@ -58,7 +58,7 @@ public void testUnknownConfig() throws Exception { String stmt = "admin set frontend config(\"unknown_config\" = \"unknown\");"; AdminSetConfigStmt adminSetConfigStmt = (AdminSetConfigStmt) UtFrameUtils.parseAndAnalyzeStmt(stmt, connectContext); expectedEx.expect(DdlException.class); - expectedEx.expectMessage("errCode = 2, detailMessage = Config 'unknown_config' does not exist or is not mutable"); + expectedEx.expectMessage("errCode = 2, detailMessage = Config 'unknown_config' does not exist"); Catalog.getCurrentCatalog().setConfig(adminSetConfigStmt); }