From ae902e2308b60f57010918bbce67703def10a02f Mon Sep 17 00:00:00 2001 From: BoYiZhang <39816903+BoYiZhang@users.noreply.github.com> Date: Thu, 9 Jul 2020 17:59:20 +0800 Subject: [PATCH 1/8] fix flink web ui bug (#3079) * fix bug Delete invalid field: executorcores Modify verification prompt * fix bug Delete invalid field: executorcores Modify verification prompt * fix bug Delete invalid field: executorcores Modify verification prompt * dag add close button * reset last version * reset last version * Update dag.vue Co-authored-by: dailidong --- .../dag/_source/formModel/tasks/flink.vue | 37 +++++-------------- .../src/js/module/i18n/locale/en_US.js | 4 ++ .../src/js/module/i18n/locale/zh_CN.js | 4 ++ 3 files changed, 17 insertions(+), 28 deletions(-) diff --git a/dolphinscheduler-ui/src/js/conf/home/pages/dag/_source/formModel/tasks/flink.vue b/dolphinscheduler-ui/src/js/conf/home/pages/dag/_source/formModel/tasks/flink.vue index 2e87721341..d1488df6d0 100644 --- a/dolphinscheduler-ui/src/js/conf/home/pages/dag/_source/formModel/tasks/flink.vue +++ b/dolphinscheduler-ui/src/js/conf/home/pages/dag/_source/formModel/tasks/flink.vue @@ -86,7 +86,7 @@ :disabled="isDetails" type="input" v-model="jobManagerMemory" - :placeholder="$t('Please enter the number of Executor')" + :placeholder="$t('Please enter jobManager memory')" style="width: 200px;" autocomplete="off"> @@ -97,7 +97,7 @@ :disabled="isDetails" type="input" v-model="taskManagerMemory" - :placeholder="$t('Please enter the Executor memory')" + :placeholder="$t('Please enter the taskManager memory')" style="width: 186px;" autocomplete="off"> @@ -110,7 +110,7 @@ :disabled="isDetails" type="input" v-model="slot" - :placeholder="$t('Please enter driver core number')" + :placeholder="$t('Please enter solt number')" style="width: 200px;" autocomplete="off"> @@ -122,7 +122,7 @@ :disabled="isDetails" type="input" v-model="taskManager" - :placeholder="$t('Please enter driver memory use')" + :placeholder="$t('Please enter taskManager number')" style="width: 186px;" autocomplete="off"> @@ -210,12 +210,10 @@ slot: 1, // Driver Number of memory taskManager: '2', - // Executor Number + // jobManager Memory jobManagerMemory: '1G', - // Executor Number of memory + // taskManager Memory taskManagerMemory: '2G', - // Executor Number of cores - executorCores: 2, // Command line argument mainArgs: '', // Other parameters @@ -290,22 +288,17 @@ } if (!this.jobManagerMemory) { - this.$message.warning(`${i18n.$t('Please enter the number of Executor')}`) + this.$message.warning(`${i18n.$t('Please enter jobManager memory')}`) return false } if (!Number.isInteger(parseInt(this.jobManagerMemory))) { - this.$message.warning(`${i18n.$t('The number of Executors should be a positive integer')}`) - return false - } - - if (!this.taskManagerMemory) { - this.$message.warning(`${i18n.$t('Please enter the Executor memory')}`) + this.$message.warning(`${i18n.$t('Memory should be a positive integer')}`) return false } if (!this.taskManagerMemory) { - this.$message.warning(`${i18n.$t('Please enter the Executor memory')}`) + this.$message.warning(`${i18n.$t('Please enter the taskManager memory')}`) return false } @@ -314,16 +307,6 @@ return false } - if (!this.executorCores) { - this.$message.warning(`${i18n.$t('Please enter ExecutorPlease enter Executor core number')}`) - return false - } - - if (!Number.isInteger(parseInt(this.executorCores))) { - this.$message.warning(`${i18n.$t('Core number should be positive integer')}`) - return false - } - // noRes if (this.noRes.length>0) { this.$message.warning(`${i18n.$t('Please delete all non-existent resources')}`) @@ -351,7 +334,6 @@ taskManager: this.taskManager, jobManagerMemory: this.jobManagerMemory, taskManagerMemory: this.taskManagerMemory, - executorCores: this.executorCores, mainArgs: this.mainArgs, others: this.others, programType: this.programType @@ -482,7 +464,6 @@ taskManager: this.taskManager, jobManagerMemory: this.jobManagerMemory, taskManagerMemory: this.taskManagerMemory, - executorCores: this.executorCores, mainArgs: this.mainArgs, others: this.others, programType: this.programType diff --git a/dolphinscheduler-ui/src/js/module/i18n/locale/en_US.js b/dolphinscheduler-ui/src/js/module/i18n/locale/en_US.js index c9541c63b4..3e0ed54eb2 100755 --- a/dolphinscheduler-ui/src/js/module/i18n/locale/en_US.js +++ b/dolphinscheduler-ui/src/js/module/i18n/locale/en_US.js @@ -113,6 +113,10 @@ export default { 'Memory should be a positive integer': 'Memory should be a positive integer', 'Please enter ExecutorPlease enter Executor core number': 'Please enter ExecutorPlease enter Executor core number', 'Core number should be positive integer': 'Core number should be positive integer', + 'Please enter jobManager memory': 'Please enter jobManager memory', + 'Please enter the taskManager memory': 'Please enter the taskManager memory', + 'Please enter solt number': 'Please enter solt number', + 'Please enter taskManager number': 'Please enter taskManager number', 'SQL Type': 'SQL Type', Title: 'Title', 'Please enter the title of email': 'Please enter the title of email', diff --git a/dolphinscheduler-ui/src/js/module/i18n/locale/zh_CN.js b/dolphinscheduler-ui/src/js/module/i18n/locale/zh_CN.js index f7a2f9b335..a610d8b03c 100755 --- a/dolphinscheduler-ui/src/js/module/i18n/locale/zh_CN.js +++ b/dolphinscheduler-ui/src/js/module/i18n/locale/zh_CN.js @@ -114,6 +114,10 @@ export default { 'Memory should be a positive integer': '内存数为数字', 'Please enter ExecutorPlease enter Executor core number': '请填写Executor内核数', 'Core number should be positive integer': '内核数为正整数', + 'Please enter jobManager memory': '请输入JobManager内存数', + 'Please enter the taskManager memory': '请输入TaskManager内存数', + 'Please enter solt number': '请输入solt数量', + 'Please enter taskManager number': '请输入taskManager数量', 'SQL Type': 'sql类型', Title: '主题', 'Please enter the title of email': '请输入邮件主题', From a23a3e2d1254acf1d6c194f4996824e3f290cae1 Mon Sep 17 00:00:00 2001 From: felix-thinkingdata <59079269+felix-thinkingdata@users.noreply.github.com> Date: Thu, 9 Jul 2020 18:28:04 +0800 Subject: [PATCH 2/8] fix bug #3165 get resource.storage.type value toUpperCase (#3166) Co-authored-by: dailidong --- .../org/apache/dolphinscheduler/common/utils/PropertyUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java index ba1fcd6926..d0c08b1d33 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java +++ b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java @@ -72,7 +72,7 @@ public class PropertyUtils { * @return judge whether resource upload startup */ public static Boolean getResUploadStartupState(){ - String resUploadStartupType = PropertyUtils.getString(Constants.RESOURCE_STORAGE_TYPE); + String resUploadStartupType = PropertyUtils.getString(Constants.RESOURCE_STORAGE_TYPE).toUpperCase(); ResUploadType resUploadType = ResUploadType.valueOf(resUploadStartupType); return resUploadType == ResUploadType.HDFS || resUploadType == ResUploadType.S3; } From d87d2d80dd88db76aa2d99f4f34ecfeeda584afe Mon Sep 17 00:00:00 2001 From: tswstarplanet Date: Thu, 9 Jul 2020 20:27:41 +0800 Subject: [PATCH 3/8] add maxWaitTime config when build zookeeper client (#3133) * add maxWaitTime config when build zookeeper client * throw exception when connect zk timeout * use dedicated exception instead of a generic one Co-authored-by: dailidong Co-authored-by: qiaozhanwei --- .../dolphinscheduler/service/zk/ZookeeperConfig.java | 11 +++++++++++ .../service/zk/ZookeeperOperator.java | 5 ++++- .../src/main/resources/zookeeper.properties | 3 ++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperConfig.java b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperConfig.java index 5bdc6f8cd7..57ac13e3be 100644 --- a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperConfig.java +++ b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperConfig.java @@ -52,6 +52,9 @@ public class ZookeeperConfig { @Value("${zookeeper.dolphinscheduler.root:/dolphinscheduler}") private String dsRoot; + @Value("${zookeeper.max.wait.time:10000}") + private int maxWaitTime; + public String getServerList() { return serverList; } @@ -115,4 +118,12 @@ public class ZookeeperConfig { public void setDsRoot(String dsRoot) { this.dsRoot = dsRoot; } + + public int getMaxWaitTime() { + return maxWaitTime; + } + + public void setMaxWaitTime(int maxWaitTime) { + this.maxWaitTime = maxWaitTime; + } } \ No newline at end of file diff --git a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperOperator.java b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperOperator.java index cd479d44cc..ba3a3bfecb 100644 --- a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperOperator.java +++ b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/zk/ZookeeperOperator.java @@ -37,6 +37,7 @@ import org.springframework.stereotype.Component; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.concurrent.TimeUnit; import static org.apache.dolphinscheduler.common.utils.Preconditions.checkNotNull; @@ -109,7 +110,9 @@ public class ZookeeperOperator implements InitializingBean { zkClient = builder.build(); zkClient.start(); try { - zkClient.blockUntilConnected(); + if (!zkClient.blockUntilConnected(zookeeperConfig.getMaxWaitTime(), TimeUnit.MILLISECONDS)) { + throw new IllegalStateException("Connect zookeeper expire max wait time"); + } } catch (final Exception ex) { throw new RuntimeException(ex); } diff --git a/dolphinscheduler-service/src/main/resources/zookeeper.properties b/dolphinscheduler-service/src/main/resources/zookeeper.properties index 2204467ac9..31f5f43c42 100644 --- a/dolphinscheduler-service/src/main/resources/zookeeper.properties +++ b/dolphinscheduler-service/src/main/resources/zookeeper.properties @@ -26,4 +26,5 @@ zookeeper.quorum=localhost:2181 #zookeeper.connection.timeout=30000 #zookeeper.retry.base.sleep=100 #zookeeper.retry.max.sleep=30000 -#zookeeper.retry.maxtime=10 \ No newline at end of file +#zookeeper.retry.maxtime=10 +#zookeeper.max.wait.time=10000 \ No newline at end of file From 1eb8fb6db33af4b644492a9fb0b0348d7de14407 Mon Sep 17 00:00:00 2001 From: qiaozhanwei Date: Fri, 10 Jul 2020 11:10:13 +0800 Subject: [PATCH 4/8] NettyDecoder class decode method switch case remove break (#3035) --- .../apache/dolphinscheduler/remote/codec/NettyDecoder.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dolphinscheduler-remote/src/main/java/org/apache/dolphinscheduler/remote/codec/NettyDecoder.java b/dolphinscheduler-remote/src/main/java/org/apache/dolphinscheduler/remote/codec/NettyDecoder.java index 09f97d6cd7..a69022214d 100644 --- a/dolphinscheduler-remote/src/main/java/org/apache/dolphinscheduler/remote/codec/NettyDecoder.java +++ b/dolphinscheduler-remote/src/main/java/org/apache/dolphinscheduler/remote/codec/NettyDecoder.java @@ -55,19 +55,15 @@ public class NettyDecoder extends ReplayingDecoder { case MAGIC: checkMagic(in.readByte()); checkpoint(State.COMMAND); - break; case COMMAND: commandHeader.setType(in.readByte()); checkpoint(State.OPAQUE); - break; case OPAQUE: commandHeader.setOpaque(in.readLong()); checkpoint(State.BODY_LENGTH); - break; case BODY_LENGTH: commandHeader.setBodyLength(in.readInt()); checkpoint(State.BODY); - break; case BODY: byte[] body = new byte[commandHeader.getBodyLength()]; in.readBytes(body); @@ -79,7 +75,6 @@ public class NettyDecoder extends ReplayingDecoder { out.add(packet); // checkpoint(State.MAGIC); - break; default: logger.warn("unknown decoder state {}", state()); } From 1e7582e910c23a42bb4e92cd64fde4df7cbf6b34 Mon Sep 17 00:00:00 2001 From: felix-thinkingdata <59079269+felix-thinkingdata@users.noreply.github.com> Date: Fri, 10 Jul 2020 15:21:42 +0800 Subject: [PATCH 5/8] =?UTF-8?q?#3176=20optimize=20Gets=20the=20value=20of?= =?UTF-8?q?=20this=20property=20=E2=80=9Cresource.storage.type=E2=80=9D?= =?UTF-8?q?=EF=BC=8C=20Comparison=20with=20enumerated=20types=20(#3178)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix bug #3165 get resource.storage.type value toUpperCase * fix bug #3176 optimize Gets the value of this property “resource.storage.type” Co-authored-by: dailidong --- .../dolphinscheduler/common/utils/CommonUtils.java | 2 +- .../dolphinscheduler/common/utils/HadoopUtils.java | 2 +- .../dolphinscheduler/common/utils/PropertyUtils.java | 12 +++++++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CommonUtils.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CommonUtils.java index 2468e62ab0..0c23d5c2ae 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CommonUtils.java +++ b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CommonUtils.java @@ -70,7 +70,7 @@ public class CommonUtils { * @return true if upload resource is HDFS and kerberos startup */ public static boolean getKerberosStartupState(){ - String resUploadStartupType = PropertyUtils.getString(Constants.RESOURCE_STORAGE_TYPE); + String resUploadStartupType = PropertyUtils.getUpperCaseString(Constants.RESOURCE_STORAGE_TYPE); ResUploadType resUploadType = ResUploadType.valueOf(resUploadStartupType); Boolean kerberosStartupState = PropertyUtils.getBoolean(Constants.HADOOP_SECURITY_AUTHENTICATION_STARTUP_STATE,false); return resUploadType == ResUploadType.HDFS && kerberosStartupState; diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java index 33127e1214..a628c6a86e 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java +++ b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java @@ -110,7 +110,7 @@ public class HadoopUtils implements Closeable { try { configuration = new Configuration(); - String resourceStorageType = PropertyUtils.getString(Constants.RESOURCE_STORAGE_TYPE); + String resourceStorageType = PropertyUtils.getUpperCaseString(Constants.RESOURCE_STORAGE_TYPE); ResUploadType resUploadType = ResUploadType.valueOf(resourceStorageType); if (resUploadType == ResUploadType.HDFS) { diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java index d0c08b1d33..8f0ee327c1 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java +++ b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java @@ -72,7 +72,7 @@ public class PropertyUtils { * @return judge whether resource upload startup */ public static Boolean getResUploadStartupState(){ - String resUploadStartupType = PropertyUtils.getString(Constants.RESOURCE_STORAGE_TYPE).toUpperCase(); + String resUploadStartupType = PropertyUtils.getUpperCaseString(Constants.RESOURCE_STORAGE_TYPE); ResUploadType resUploadType = ResUploadType.valueOf(resUploadStartupType); return resUploadType == ResUploadType.HDFS || resUploadType == ResUploadType.S3; } @@ -87,6 +87,16 @@ public class PropertyUtils { return properties.getProperty(key.trim()); } + /** + * get property value with upper case + * + * @param key property name + * @return property value with upper case + */ + public static String getUpperCaseString(String key) { + return properties.getProperty(key.trim()).toUpperCase(); + } + /** * get property value * From dcdd7dedd06454ed468eae86881b75177261c9e2 Mon Sep 17 00:00:00 2001 From: rockxsj Date: Sat, 11 Jul 2020 08:56:38 +0800 Subject: [PATCH 6/8] Modify the AWS S3 request encryption method to V4. (#3182) Co-authored-by: rockxsj --- .../java/org/apache/dolphinscheduler/common/Constants.java | 4 ++++ .../org/apache/dolphinscheduler/common/utils/HadoopUtils.java | 1 + 2 files changed, 5 insertions(+) diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java index a2634bf492..2432b7ee93 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java +++ b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java @@ -786,6 +786,10 @@ public final class Constants { */ public static final String HADOOP_SECURITY_AUTHENTICATION_STARTUP_STATE = "hadoop.security.authentication.startup.state"; + /** + * com.amazonaws.services.s3.enableV4 + */ + public static final String AWS_S3_V4 = "com.amazonaws.services.s3.enableV4"; /** * loginUserFromKeytab user diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java index a628c6a86e..4111ef9714 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java +++ b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java @@ -159,6 +159,7 @@ public class HadoopUtils implements Closeable { } } } else if (resUploadType == ResUploadType.S3) { + System.setProperty(Constants.AWS_S3_V4, Constants.STRING_TRUE); configuration.set(Constants.FS_DEFAULTFS, PropertyUtils.getString(Constants.FS_DEFAULTFS)); configuration.set(Constants.FS_S3A_ENDPOINT, PropertyUtils.getString(Constants.FS_S3A_ENDPOINT)); configuration.set(Constants.FS_S3A_ACCESS_KEY, PropertyUtils.getString(Constants.FS_S3A_ACCESS_KEY)); From aea702e3d4279859ee803c305be78390058e8be3 Mon Sep 17 00:00:00 2001 From: Yichao Yang <1048262223@qq.com> Date: Sun, 12 Jul 2020 08:42:55 +0800 Subject: [PATCH 7/8] [Feature][docs] Add the issue template according to the issue specification (#3185) Co-authored-by: dailidong --- .github/ISSUE_TEMPLATE/bug_report.md | 4 ++-- .github/ISSUE_TEMPLATE/feature_request.md | 7 +++++- .../ISSUE_TEMPLATE/improvement_suggestion.md | 22 +++++++++++++++++++ .github/ISSUE_TEMPLATE/question.md | 8 +++---- .github/ISSUE_TEMPLATE/test.md | 22 +++++++++++++++++++ 5 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/improvement_suggestion.md create mode 100644 .github/ISSUE_TEMPLATE/test.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index dc797cf0a8..11751bd184 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,7 +1,7 @@ --- name: Bug report about: Create a report to help us improve -title: "[BUG] bug title " +title: "[Bug][Module Name] Bug title " labels: bug assignees: '' @@ -32,5 +32,5 @@ If applicable, add screenshots to help explain your problem. **Additional context** Add any other context about the problem here. -**Requirement or improvement +**Requirement or improvement** - Please describe about your requirements or improvement suggestions. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index 8cd481d442..eb71fc3bbd 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -1,12 +1,17 @@ --- name: Feature request about: Suggest an idea for this project -title: "[Feature]" +title: "[Feature][Module Name] Feature title" labels: new feature assignees: '' --- +*For better global communication, please give priority to using English description, thx! * + +**Describe the feature** +A clear and concise description of what the feature is. + **Is your feature request related to a problem? Please describe.** A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] diff --git a/.github/ISSUE_TEMPLATE/improvement_suggestion.md b/.github/ISSUE_TEMPLATE/improvement_suggestion.md new file mode 100644 index 0000000000..44f8f48e8b --- /dev/null +++ b/.github/ISSUE_TEMPLATE/improvement_suggestion.md @@ -0,0 +1,22 @@ +--- +name: Improvement suggestion +about: Improvement suggestion for this project +title: "[Improvement][Module Name] Improvement title" +labels: improvement +assignees: '' + +--- + +*For better global communication, please give priority to using English description, thx! * + +**Describe the question** +A clear and concise description of what the improvement is. + +**What are the current deficiencies and the benefits of improvement** +- A clear and concise description of the current deficiencies and the benefits of this improvement. + +**Which version of DolphinScheduler:** + -[1.1.0-preview] + +**Describe alternatives you've considered** +A clear and concise description of any alternative improvement solutions you've considered. diff --git a/.github/ISSUE_TEMPLATE/question.md b/.github/ISSUE_TEMPLATE/question.md index 59c8ce7716..891aff292b 100644 --- a/.github/ISSUE_TEMPLATE/question.md +++ b/.github/ISSUE_TEMPLATE/question.md @@ -1,7 +1,7 @@ --- -name: question -about: have a question wanted to be help -title: "[QUESTION] question title" +name: Question +about: Have a question wanted to be help +title: "[Question] Question title" labels: question assignees: '' @@ -19,5 +19,5 @@ A clear and concise description of what the question is. **Additional context** Add any other context about the problem here. -**Requirement or improvement +**Requirement or improvement** - Please describe about your requirements or improvement suggestions. diff --git a/.github/ISSUE_TEMPLATE/test.md b/.github/ISSUE_TEMPLATE/test.md new file mode 100644 index 0000000000..59a5b61a64 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/test.md @@ -0,0 +1,22 @@ +--- +name: Test +about: Test to enhance the robustness of this project +title: "[Test][Module Name] Test title" +labels: test +assignees: '' + +--- + +*For better global communication, please give priority to using English description, thx! * + +**Describe the question** +A clear and concise description of what the test part is. + +**What are the current deficiencies and the benefits of changing or adding this test** +- A clear and concise description of the current deficiencies, the benefits of changing or adding this test, and the scope involved. + +**Which version of DolphinScheduler:** + -[1.1.0-preview] + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions you've considered. \ No newline at end of file From 6fa37013966eceb663108bcb72eaeab732a6171d Mon Sep 17 00:00:00 2001 From: samz406 Date: Sun, 12 Jul 2020 09:48:44 +0800 Subject: [PATCH 8/8] optimize SchedulerService.setScheduleState code (#3136) * Optimize SchedulerService.setScheduleState code * modify the test case to PowerMock * modify code smell * modify code smell Co-authored-by: dailidong --- .../api/service/SchedulerService.java | 23 +-- .../api/service/SchedulerServiceTest.java | 174 ++++++++++++++++-- 2 files changed, 166 insertions(+), 31 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java index f926fa9d49..ed856d5094 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/SchedulerService.java @@ -19,6 +19,7 @@ package org.apache.dolphinscheduler.api.service; import org.apache.dolphinscheduler.api.dto.ScheduleParam; import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.exceptions.ServiceException; import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.common.Constants; import org.apache.dolphinscheduler.common.enums.*; @@ -333,10 +334,9 @@ public class SchedulerService extends BaseService { if(scheduleStatus == ReleaseState.ONLINE){ // check process definition release state if(processDefinition.getReleaseState() != ReleaseState.ONLINE){ - ProcessDefinition definition = processDefinitionMapper.selectById(scheduleObj.getProcessDefinitionId()); logger.info("not release process definition id: {} , name : {}", processDefinition.getId(), processDefinition.getName()); - putMsg(result, Status.PROCESS_DEFINE_NOT_RELEASE, definition.getName()); + putMsg(result, Status.PROCESS_DEFINE_NOT_RELEASE, processDefinition.getName()); return result; } // check sub process definition release state @@ -380,7 +380,7 @@ public class SchedulerService extends BaseService { switch (scheduleStatus) { case ONLINE: { logger.info("Call master client set schedule online, project id: {}, flow id: {},host: {}", project.getId(), processDefinition.getId(), masterServers); - setSchedule(project.getId(), id); + setSchedule(project.getId(), scheduleObj); break; } case OFFLINE: { @@ -395,7 +395,7 @@ public class SchedulerService extends BaseService { } } catch (Exception e) { result.put(Constants.MSG, scheduleStatus == ReleaseState.ONLINE ? "set online failure" : "set offline failure"); - throw new RuntimeException(result.get(Constants.MSG).toString()); + throw new ServiceException(result.get(Constants.MSG).toString()); } putMsg(result, Status.SUCCESS); @@ -472,15 +472,10 @@ public class SchedulerService extends BaseService { return result; } - public void setSchedule(int projectId, int scheduleId) throws RuntimeException{ - logger.info("set schedule, project id: {}, scheduleId: {}", projectId, scheduleId); - + public void setSchedule(int projectId, Schedule schedule) { - Schedule schedule = processService.querySchedule(scheduleId); - if (schedule == null) { - logger.warn("process schedule info not exists"); - return; - } + int scheduleId = schedule.getId(); + logger.info("set schedule, project id: {}, scheduleId: {}", projectId, scheduleId); Date startDate = schedule.getStartTime(); Date endDate = schedule.getEndTime(); @@ -502,7 +497,7 @@ public class SchedulerService extends BaseService { * @param scheduleId schedule id * @throws RuntimeException runtime exception */ - public static void deleteSchedule(int projectId, int scheduleId) throws RuntimeException{ + public static void deleteSchedule(int projectId, int scheduleId) { logger.info("delete schedules of project id:{}, schedule id:{}", projectId, scheduleId); String jobName = QuartzExecutors.buildJobName(scheduleId); @@ -510,7 +505,7 @@ public class SchedulerService extends BaseService { if(!QuartzExecutors.getInstance().deleteJob(jobName, jobGroupName)){ logger.warn("set offline failure:projectId:{},scheduleId:{}",projectId,scheduleId); - throw new RuntimeException(String.format("set offline failure")); + throw new ServiceException("set offline failure"); } } diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java index 9f6dca813e..f75d808e56 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SchedulerServiceTest.java @@ -16,43 +16,183 @@ */ package org.apache.dolphinscheduler.api.service; -import org.apache.dolphinscheduler.api.ApiApplicationServer; import org.apache.dolphinscheduler.api.enums.Status; import org.apache.dolphinscheduler.common.Constants; import org.apache.dolphinscheduler.common.enums.ReleaseState; -import org.apache.dolphinscheduler.common.enums.UserType; +import org.apache.dolphinscheduler.common.model.Server; +import org.apache.dolphinscheduler.dao.entity.ProcessDefinition; import org.apache.dolphinscheduler.dao.entity.Project; +import org.apache.dolphinscheduler.dao.entity.Schedule; import org.apache.dolphinscheduler.dao.entity.User; +import org.apache.dolphinscheduler.dao.mapper.ProcessDefinitionMapper; +import org.apache.dolphinscheduler.dao.mapper.ProjectMapper; +import org.apache.dolphinscheduler.dao.mapper.ProjectUserMapper; +import org.apache.dolphinscheduler.dao.mapper.ScheduleMapper; +import org.apache.dolphinscheduler.service.process.ProcessService; +import org.apache.dolphinscheduler.service.quartz.QuartzExecutors; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.quartz.Scheduler; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.test.context.junit4.SpringRunner; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; import java.util.Map; -@RunWith(SpringRunner.class) -@SpringBootTest(classes = ApiApplicationServer.class) +@RunWith(PowerMockRunner.class) +@PrepareForTest(QuartzExecutors.class) + public class SchedulerServiceTest { - private static final Logger logger = LoggerFactory.getLogger(ExecutorServiceTest.class); - @Autowired + + @InjectMocks private SchedulerService schedulerService; + + @Autowired + private ExecutorService executorService; + + @Mock + private MonitorService monitorService; + + @Mock + private ProcessService processService; + + @Mock + private ScheduleMapper scheduleMapper; + + @Mock + private ProjectMapper projectMapper; + @Mock + private ProjectUserMapper projectUserMapper; + @Mock + private ProjectService projectService; + + @Mock + private ProcessDefinitionMapper processDefinitionMapper; + + @Mock + private QuartzExecutors quartzExecutors; + + @Mock + private Scheduler scheduler; + + + @Before + public void setUp() { + + quartzExecutors = PowerMockito.mock(QuartzExecutors.class); + PowerMockito.mockStatic(QuartzExecutors.class); + try { + PowerMockito.doReturn(quartzExecutors).when(QuartzExecutors.class, "getInstance"); + } catch (Exception e) { + e.printStackTrace(); + } + + } + @Test - public void testSetScheduleState(){ + public void testSetScheduleState() { + + + String projectName = "test"; User loginUser = new User(); - loginUser.setId(-1); - loginUser.setUserType(UserType.GENERAL_USER); + loginUser.setId(1); + Map result = new HashMap(); + Project project = getProject(projectName); + + + ProcessDefinition processDefinition = new ProcessDefinition(); + + Schedule schedule = new Schedule(); + schedule.setId(1); + schedule.setProcessDefinitionId(1); + schedule.setReleaseState(ReleaseState.OFFLINE); + + List masterServers = new ArrayList<>(); + masterServers.add(new Server()); + + Mockito.when(scheduleMapper.selectById(1)).thenReturn(schedule); + + Mockito.when(projectMapper.queryByName(projectName)).thenReturn(project); + + Mockito.when(processService.findProcessDefineById(1)).thenReturn(processDefinition); + + //hash no auth + result = schedulerService.setScheduleState(loginUser, projectName, 1, ReleaseState.ONLINE); + + Mockito.when(projectService.hasProjectAndPerm(loginUser, project, result)).thenReturn(true); + //schedule not exists + result = schedulerService.setScheduleState(loginUser, projectName, 2, ReleaseState.ONLINE); + Assert.assertEquals(Status.SCHEDULE_CRON_NOT_EXISTS, result.get(Constants.STATUS)); + + //SCHEDULE_CRON_REALEASE_NEED_NOT_CHANGE + result = schedulerService.setScheduleState(loginUser, projectName, 1, ReleaseState.OFFLINE); + Assert.assertEquals(Status.SCHEDULE_CRON_REALEASE_NEED_NOT_CHANGE, result.get(Constants.STATUS)); + + //PROCESS_DEFINE_NOT_EXIST + schedule.setProcessDefinitionId(2); + result = schedulerService.setScheduleState(loginUser, projectName, 1, ReleaseState.ONLINE); + Assert.assertEquals(Status.PROCESS_DEFINE_NOT_EXIST, result.get(Constants.STATUS)); + schedule.setProcessDefinitionId(1); + + // PROCESS_DEFINE_NOT_RELEASE + result = schedulerService.setScheduleState(loginUser, projectName, 1, ReleaseState.ONLINE); + Assert.assertEquals(Status.PROCESS_DEFINE_NOT_RELEASE, result.get(Constants.STATUS)); + + processDefinition.setReleaseState(ReleaseState.ONLINE); + Mockito.when(processService.findProcessDefineById(1)).thenReturn(processDefinition); + + //MASTER_NOT_EXISTS + result = schedulerService.setScheduleState(loginUser, projectName, 1, ReleaseState.ONLINE); + Assert.assertEquals(Status.MASTER_NOT_EXISTS, result.get(Constants.STATUS)); + + //set master + Mockito.when(monitorService.getServerListFromZK(true)).thenReturn(masterServers); + + //SUCCESS + result = schedulerService.setScheduleState(loginUser, projectName, 1, ReleaseState.ONLINE); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + + //OFFLINE + Mockito.when(quartzExecutors.deleteJob(null, null)).thenReturn(true); + result = schedulerService.setScheduleState(loginUser, projectName, 1, ReleaseState.OFFLINE); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + + } + + @Test + public void testDeleteSchedule() { + + Mockito.when(quartzExecutors.deleteJob("1", "1")).thenReturn(true); + Mockito.when(quartzExecutors.buildJobGroupName(1)).thenReturn("1"); + Mockito.when(quartzExecutors.buildJobName(1)).thenReturn("1"); + boolean flag = true; + try { + schedulerService.deleteSchedule(1, 1); + }catch (Exception e){ + flag = false; + } + Assert.assertTrue(flag); + + } + + private Project getProject(String name) { + Project project = new Project(); - project.setName("project_test1"); - project.setId(-1); + project.setName(name); + project.setUserId(1); - Map map = schedulerService.setScheduleState(loginUser, project.getName(), 44, ReleaseState.ONLINE); - Assert.assertEquals(Status.PROJECT_NOT_FOUNT, map.get(Constants.STATUS)); + return project; } } \ No newline at end of file