You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by xx...@apache.org on 2022/04/29 02:33:07 UTC

[kylin] branch main updated: KYLIN-5179, fix StorageCleanupJob remove invalid segement

This is an automated email from the ASF dual-hosted git repository.

xxyu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/kylin.git


The following commit(s) were added to refs/heads/main by this push:
     new a6941e35bd KYLIN-5179, fix StorageCleanupJob remove invalid segement
a6941e35bd is described below

commit a6941e35bda6a4d819b966c0c040b0931c84630f
Author: Mukvin <bo...@163.com>
AuthorDate: Wed Apr 27 20:07:29 2022 +0800

    KYLIN-5179, fix StorageCleanupJob remove invalid segement
---
 .../java/org/apache/kylin/common/KylinConfig.java  |  7 +++--
 .../org/apache/kylin/common/KylinConfigBase.java   | 34 +++++++++++++++++-----
 .../kylin/common/util/AbstractApplication.java     |  9 ++++--
 .../apache/kylin/common/util/OptionsHelper.java    |  8 +++--
 docker/dockerfile/standalone/conf/bin/kylin.sh     |  4 +--
 .../apache/kylin/rest/job/StorageCleanupJob.java   |  8 +++++
 6 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java b/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java
index a0dce5b047..de5c16bfee 100644
--- a/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java
+++ b/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java
@@ -354,7 +354,7 @@ public class KylinConfig extends KylinConfigBase {
 
     // should be private; package visible for test only
     static File getSitePropertiesFile() {
-        String kylinConfHome = System.getProperty(KYLIN_CONF);
+        String kylinConfHome = getKylinConfHome();
         if (!StringUtils.isEmpty(kylinConfHome)) {
             logger.info("Use KYLIN_CONF={}", kylinConfHome);
             return existFile(kylinConfHome);
@@ -363,8 +363,9 @@ public class KylinConfig extends KylinConfigBase {
         logger.debug("KYLIN_CONF property was not set, will seek KYLIN_HOME env variable");
 
         String kylinHome = getKylinHome();
-        if (StringUtils.isEmpty(kylinHome))
+        if (StringUtils.isEmpty(kylinHome)) {
             throw new KylinConfigCannotInitException("Didn't find KYLIN_CONF or KYLIN_HOME, please set one of them");
+        }
 
         logger.info("Use KYLIN_HOME={}", kylinHome);
         String path = kylinHome + File.separator + "conf";
@@ -553,7 +554,7 @@ public class KylinConfig extends KylinConfigBase {
                 orderedProperties.setProperty(sysProp, sysPropValue);
             }
         }
-        
+
         final StringBuilder sb = new StringBuilder();
         for (Map.Entry<String, String> entry : orderedProperties.entrySet()) {
             sb.append(entry.getKey() + "=" + entry.getValue()).append('\n');
diff --git a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
index 22e2d883e6..710f8e18e4 100644
--- a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
+++ b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
@@ -103,6 +103,22 @@ public abstract class KylinConfigBase implements Serializable {
         this.properties = force ? props : BCC.check(props);
     }
 
+    public static String getKylinConfHome() {
+        String kylinConfHome = getKylinConfHomeWithoutWarn();
+        if (StringUtils.isEmpty(kylinConfHome)) {
+            logger.warn("KYLIN_CONF was not set");
+        }
+        return kylinConfHome;
+    }
+
+    public static String getKylinConfHomeWithoutWarn() {
+        String kylinConfHome = System.getenv("KYLIN_CONF");
+        if (StringUtils.isEmpty(kylinConfHome)) {
+            kylinConfHome = System.getProperty("KYLIN_CONF");
+        }
+        return kylinConfHome;
+    }
+
     public static String getKylinHome() {
         String kylinHome = getKylinHomeWithoutWarn();
         if (StringUtils.isEmpty(kylinHome)) {
@@ -279,6 +295,7 @@ public abstract class KylinConfigBase implements Serializable {
         return result;
     }
 
+    @Override
     public String toString() {
         return getMetadataUrl().toString();
     }
@@ -298,8 +315,9 @@ public abstract class KylinConfigBase implements Serializable {
     }
 
     public String getHdfsWorkingDirectory() {
-        if (cachedHdfsWorkingDirectory != null)
+        if (cachedHdfsWorkingDirectory != null) {
             return cachedHdfsWorkingDirectory;
+        }
 
         String root = getOptional("kylin.env.hdfs-working-dir", "/kylin");
 
@@ -324,8 +342,9 @@ public abstract class KylinConfigBase implements Serializable {
 
         root = new Path(path, metaId).toString();
 
-        if (!root.endsWith("/"))
+        if (!root.endsWith("/")) {
             root += "/";
+        }
 
         cachedHdfsWorkingDirectory = root;
         if (cachedHdfsWorkingDirectory.startsWith(FILE_SCHEME)) {
@@ -348,9 +367,10 @@ public abstract class KylinConfigBase implements Serializable {
         }
 
         Path path = new Path(root);
-        if (!path.isAbsolute())
+        if (!path.isAbsolute()) {
             throw new IllegalArgumentException(
                     "kylin.env.hdfs-metastore-bigcell-dir must be absolute, but got " + root);
+        }
 
         // make sure path is qualified
         try {
@@ -693,7 +713,7 @@ public abstract class KylinConfigBase implements Serializable {
     public boolean isRowKeyEncodingAutoConvert() {
         return Boolean.parseBoolean(getOptional("kylin.cube.kylin.cube.rowkey-encoding-auto-convert", "true"));
     }
-    
+
     public String getSegmentAdvisor() {
         return getOptional("kylin.cube.segment-advisor", "org.apache.kylin.cube.CubeSegmentAdvisor");
     }
@@ -2340,7 +2360,7 @@ public abstract class KylinConfigBase implements Serializable {
     public String getKylinMetricsEventTimeZone() {
         return getOptional("kylin.metrics.event-time-zone", getTimeZone()).toUpperCase(Locale.ROOT);
     }
-    
+
     public boolean isKylinMetricsMonitorEnabled() {
         return Boolean.parseBoolean(getOptional("kylin.metrics.monitor-enabled", FALSE));
     }
@@ -3136,10 +3156,10 @@ public abstract class KylinConfigBase implements Serializable {
     private String getLogPropertyFile(String filename) {
         if (isDevEnv()) {
             return Paths.get(getKylinHomeWithoutWarn(),
-                    "build", "conf").toString() + File.separator + filename;
+                    "build", "conf") + File.separator + filename;
         } else {
             return Paths.get(getKylinHomeWithoutWarn(),
-                    "conf").toString() + File.separator + filename;
+                    "conf") + File.separator + filename;
         }
     }
 
diff --git a/core-common/src/main/java/org/apache/kylin/common/util/AbstractApplication.java b/core-common/src/main/java/org/apache/kylin/common/util/AbstractApplication.java
index 7275e10da3..75dce7ca0d 100644
--- a/core-common/src/main/java/org/apache/kylin/common/util/AbstractApplication.java
+++ b/core-common/src/main/java/org/apache/kylin/common/util/AbstractApplication.java
@@ -6,9 +6,9 @@
  * 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.
@@ -34,6 +34,11 @@ public abstract class AbstractApplication {
         System.out.println("Running " + this.getClass().getName() + " " + StringUtils.join(args, " "));
         try {
             optionsHelper.parseOptions(getOptions(), args);
+            if (optionsHelper.isHelpOption()) {
+                optionsHelper.printUsage(this.getClass().getName(), getOptions());
+                return;
+            }
+
             execute(optionsHelper);
         } catch (ParseException e) {
             optionsHelper.printUsage(this.getClass().getName(), getOptions());
diff --git a/core-common/src/main/java/org/apache/kylin/common/util/OptionsHelper.java b/core-common/src/main/java/org/apache/kylin/common/util/OptionsHelper.java
index 2e20ef6305..ecf8aa23ea 100644
--- a/core-common/src/main/java/org/apache/kylin/common/util/OptionsHelper.java
+++ b/core-common/src/main/java/org/apache/kylin/common/util/OptionsHelper.java
@@ -6,9 +6,9 @@
  * 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.
@@ -68,6 +68,10 @@ public class OptionsHelper {
         formatter.printHelp(programName, options);
     }
 
+    public boolean isHelpOption() throws ParseException {
+        return commandLine.hasOption("help");
+    }
+
     public static String convertToFileURL(String path) {
         if (File.separatorChar != '/') {
             path = path.replace(File.separatorChar, '/');
diff --git a/docker/dockerfile/standalone/conf/bin/kylin.sh b/docker/dockerfile/standalone/conf/bin/kylin.sh
index a110c8c234..3afbff546f 100755
--- a/docker/dockerfile/standalone/conf/bin/kylin.sh
+++ b/docker/dockerfile/standalone/conf/bin/kylin.sh
@@ -152,10 +152,10 @@ function retrieveDependency() {
 
     # compose KYLIN_TOMCAT_CLASSPATH
     tomcat_classpath=${tomcat_root}/bin/bootstrap.jar:${tomcat_root}/bin/tomcat-juli.jar:${tomcat_root}/lib/*
-    export KYLIN_TOMCAT_CLASSPATH=${tomcat_classpath}:${KYLIN_HOME}/conf:${KYLIN_HOME}/lib/*:${KYLIN_HOME}/ext/*:${hadoop_dependencies}:${flink_dependency}
+    export KYLIN_TOMCAT_CLASSPATH=${tomcat_classpath}:${KYLIN_HOME}/conf:${KYLIN_HOME}/lib/*:${KYLIN_HOME}/ext/*:${hadoop_dependencies}
 
     # compose KYLIN_TOOL_CLASSPATH
-    export KYLIN_TOOL_CLASSPATH=${KYLIN_HOME}/conf:${KYLIN_HOME}/tool/*:${KYLIN_HOME}/ext/*:${hadoop_dependencies}
+    export KYLIN_TOOL_CLASSPATH=${KYLIN_HOME}/conf:${KYLIN_HOME}/hadoop_conf:${KYLIN_HOME}/tool/*:${KYLIN_HOME}/ext/*:${hadoop_dependencies}
 
     # compose kylin_common_opts
     kylin_common_opts="-Dkylin.hive.dependency=${hive_dependency} \
diff --git a/server-base/src/main/java/org/apache/kylin/rest/job/StorageCleanupJob.java b/server-base/src/main/java/org/apache/kylin/rest/job/StorageCleanupJob.java
index 95a7a48944..d8d40fca17 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/job/StorageCleanupJob.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/job/StorageCleanupJob.java
@@ -66,6 +66,13 @@ public class StorageCleanupJob extends AbstractApplication {
     private static final String GLOBAL_DICT_PREFIX = "/dict/global_dict/";
     private static final String TABLE_SNAPSHOT_PREFIX = "/table_snapshot/";
 
+    @SuppressWarnings("static-access")
+    protected static final Option OPTION_HELP = OptionBuilder
+            .hasArg(false)
+            .isRequired(false)
+            .withDescription("Print supported operations.")
+            .create("help");
+
     @SuppressWarnings("static-access")
     protected static final Option OPTION_DELETE = OptionBuilder.withArgName("delete")
             .hasArg().isRequired(false)
@@ -130,6 +137,7 @@ public class StorageCleanupJob extends AbstractApplication {
     @Override
     protected Options getOptions() {
         Options options = new Options();
+        options.addOption(OPTION_HELP);
         options.addOption(OPTION_DELETE);
         options.addOption(OPTION_CLEANUP_GLOBAL_DICT);
         options.addOption(OPTION_CLEANUP_TABLE_SNAPSHOT);