This is an automated email from the ASF dual-hosted git repository. shaofengshi pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kylin.git
The following commit(s) were added to refs/heads/master by this push: new 6283046 KYLIN-3597 improve code smell 6283046 is described below commit 62830469424c3f2dcb79f35d1f66e1698768428f Author: Lijun Cao <641507...@qq.com> AuthorDate: Thu Dec 13 15:28:01 2018 +0800 KYLIN-3597 improve code smell --- .../org/apache/kylin/common/KylinConfigBase.java | 26 ++++++++++++---------- .../kylin/common/persistence/ContentWriter.java | 10 ++------- .../persistence/ExponentialBackoffRetry.java | 3 ++- .../common/persistence/FileResourceStore.java | 11 +++------ .../common/persistence/HDFSResourceStore.java | 14 ++++++------ .../common/persistence/JDBCResourceStore.java | 6 ++--- 6 files changed, 31 insertions(+), 39 deletions(-) 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 018552c..f67f6b3 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 @@ -62,6 +62,8 @@ abstract public class KylinConfigBase implements Serializable { private static final String DEFAULT = "default"; private static final String KYLIN_ENGINE_MR_JOB_JAR = "kylin.engine.mr.job-jar"; private static final String KYLIN_STORAGE_HBASE_COPROCESSOR_LOCAL_JAR = "kylin.storage.hbase.coprocessor-local-jar"; + private static final String FILE_SCHEME = "file:"; + private static final String MAPRFS_SCHEME = "maprfs:"; /* * DON'T DEFINE CONSTANTS FOR PROPERTY KEYS! @@ -264,10 +266,10 @@ abstract public class KylinConfigBase implements Serializable { root += "/"; cachedHdfsWorkingDirectory = root; - if (cachedHdfsWorkingDirectory.startsWith("file:")) { - cachedHdfsWorkingDirectory = cachedHdfsWorkingDirectory.replace("file:", "file://"); - } else if (cachedHdfsWorkingDirectory.startsWith("maprfs:")) { - cachedHdfsWorkingDirectory = cachedHdfsWorkingDirectory.replace("maprfs:", "maprfs://"); + if (cachedHdfsWorkingDirectory.startsWith(FILE_SCHEME)) { + cachedHdfsWorkingDirectory = cachedHdfsWorkingDirectory.replace(FILE_SCHEME, "file://"); + } else if (cachedHdfsWorkingDirectory.startsWith(MAPRFS_SCHEME)) { + cachedHdfsWorkingDirectory = cachedHdfsWorkingDirectory.replace(MAPRFS_SCHEME, "maprfs://"); } return cachedHdfsWorkingDirectory; } @@ -302,10 +304,10 @@ abstract public class KylinConfigBase implements Serializable { root += "/"; cachedBigCellDirectory = root; - if (cachedBigCellDirectory.startsWith("file:")) { - cachedBigCellDirectory = cachedBigCellDirectory.replace("file:", "file://"); - } else if (cachedBigCellDirectory.startsWith("maprfs:")) { - cachedBigCellDirectory = cachedBigCellDirectory.replace("maprfs:", "maprfs://"); + if (cachedBigCellDirectory.startsWith(FILE_SCHEME)) { + cachedBigCellDirectory = cachedBigCellDirectory.replace(FILE_SCHEME, "file://"); + } else if (cachedBigCellDirectory.startsWith(MAPRFS_SCHEME)) { + cachedBigCellDirectory = cachedBigCellDirectory.replace(MAPRFS_SCHEME, "maprfs://"); } return cachedBigCellDirectory; @@ -411,7 +413,7 @@ abstract public class KylinConfigBase implements Serializable { } public boolean isResourceStoreReconnectEnabled() { - return Boolean.parseBoolean(getOptional("kylin.resourcestore.reconnect-enabled", "false")); + return Boolean.parseBoolean(getOptional("kylin.resourcestore.reconnect-enabled", FALSE)); } public int getResourceStoreReconnectBaseMs() { @@ -1445,7 +1447,7 @@ abstract public class KylinConfigBase implements Serializable { } public boolean isLazyQueryEnabled() { - return Boolean.parseBoolean(getOptional("kylin.query.lazy-query-enabled", "false")); + return Boolean.parseBoolean(getOptional("kylin.query.lazy-query-enabled", FALSE)); } public long getLazyQueryWaitingTimeoutMilliSeconds() { @@ -1543,7 +1545,7 @@ abstract public class KylinConfigBase implements Serializable { } public boolean isQuerySegmentCacheEnabled() { - return Boolean.parseBoolean(getOptional("kylin.query.segment-cache-enabled", "false")); + return Boolean.parseBoolean(getOptional("kylin.query.segment-cache-enabled", FALSE)); } public int getQuerySegmentCacheTimeout() { @@ -1665,7 +1667,7 @@ abstract public class KylinConfigBase implements Serializable { } public boolean isQueryCacheSignatureEnabled() { - return Boolean.parseBoolean(this.getOptional("kylin.query.cache-signature-enabled", "false")); + return Boolean.parseBoolean(this.getOptional("kylin.query.cache-signature-enabled", FALSE)); } // ============================================================================ diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ContentWriter.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ContentWriter.java index 25420a4..c7d963d 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/ContentWriter.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ContentWriter.java @@ -71,17 +71,11 @@ abstract public class ContentWriter { } public byte[] extractAllBytes() throws IOException { - DataOutputStream dout = null; - ByteArrayOutputStream bout = null; - try { - bout = new ByteArrayOutputStream(); - dout = new DataOutputStream(bout); + try (ByteArrayOutputStream bout = new ByteArrayOutputStream(); + DataOutputStream dout = new DataOutputStream(bout)) { write(dout); dout.flush(); return bout.toByteArray(); - } finally { - IOUtils.closeQuietly(dout); - IOUtils.closeQuietly(bout); } } } diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ExponentialBackoffRetry.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ExponentialBackoffRetry.java index 2cd699b..315c51e 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/ExponentialBackoffRetry.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ExponentialBackoffRetry.java @@ -83,7 +83,8 @@ public class ExponentialBackoffRetry { } long waitMs = getSleepTimeMs(); - logger.info("Will try to re-connect after " + waitMs / 1000 + " seconds."); + long seconds = waitMs / 1000; + logger.info("Will try to re-connect after {} seconds.", seconds); try { Thread.sleep(waitMs); } catch (InterruptedException e) { diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java index 9d9cf58..bccb7a3 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java @@ -27,7 +27,6 @@ import java.io.IOException; import java.util.Collection; import org.apache.commons.io.FileUtils; -import org.apache.commons.io.IOUtils; import org.apache.kylin.common.KylinConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -106,7 +105,7 @@ public class FileResourceStore extends ResourceStore { File f = file(resPath); if (f.exists() && f.isFile()) { if (f.length() == 0) { - logger.warn("Zero length file: " + f.getAbsolutePath()); + logger.warn("Zero length file: {}. ", f.getAbsolutePath()); } return new RawResource(resPath, f.lastModified(), new FileInputStream(f)); @@ -133,14 +132,10 @@ public class FileResourceStore extends ResourceStore { File tmp = File.createTempFile("kylin-fileresource-", ".tmp"); try { - FileOutputStream out = new FileOutputStream(tmp); - DataOutputStream dout = new DataOutputStream(out); - try { + + try (FileOutputStream out = new FileOutputStream(tmp); DataOutputStream dout = new DataOutputStream(out)) { content.write(dout); dout.flush(); - } finally { - IOUtils.closeQuietly(dout); - IOUtils.closeQuietly(out); } File f = file(resPath); diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java index c41892b..c38a182 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java @@ -60,18 +60,18 @@ public class HDFSResourceStore extends ResourceStore { if (path == null) { // missing path is not expected, but don't fail it path = kylinConfig.getHdfsWorkingDirectory(null) + "tmp_metadata"; - logger.warn("Missing path, fall back to " + path); + logger.warn("Missing path, fall back to {}. ", path); } fs = HadoopUtil.getFileSystem(path); Path metadataPath = new Path(path); if (fs.exists(metadataPath) == false) { - logger.warn("Path not exist in HDFS, create it: " + path); + logger.warn("Path not exist in HDFS, create it: {}. ", path); createMetaFolder(metadataPath); } hdfsMetaPath = metadataPath; - logger.info("hdfs meta path : " + hdfsMetaPath.toString()); + logger.info("hdfs meta path : {}", hdfsMetaPath); } @@ -81,7 +81,7 @@ public class HDFSResourceStore extends ResourceStore { fs.mkdirs(metaDirName); } - logger.info("hdfs meta path created: " + metaDirName.toString()); + logger.info("hdfs meta path created: {}", metaDirName); } @Override @@ -185,7 +185,7 @@ public class HDFSResourceStore extends ResourceStore { if (fs.exists(p) && fs.isFile(p)) { FileStatus fileStatus = fs.getFileStatus(p); if (fileStatus.getLen() == 0) { - logger.warn("Zero length file: " + p.toString()); + logger.warn("Zero length file: {}. ", p); } FSDataInputStream in = fs.open(p); long ts = fileStatus.getModificationTime(); @@ -211,9 +211,9 @@ public class HDFSResourceStore extends ResourceStore { @Override protected void putResourceImpl(String resPath, ContentWriter content, long ts) throws IOException { - logger.trace("res path : " + resPath); + logger.trace("res path : {}. ", resPath); Path p = getRealHDFSPath(resPath); - logger.trace("put resource : " + p.toUri()); + logger.trace("put resource : {}. ", p.toUri()); FSDataOutputStream out = null; try { out = fs.create(p, true); diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java index c51d9b6..dc3a45b 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java @@ -117,14 +117,14 @@ public class JDBCResourceStore extends PushdownResourceStore { } String createIfNeededSql = sqls.getCreateIfNeededSql(tableName); - logger.info("Creating table: " + createIfNeededSql); + logger.info("Creating table: {}", createIfNeededSql); pstat = connection.prepareStatement(createIfNeededSql); pstat.executeUpdate(); try { String indexName = "IDX_" + META_TABLE_TS; String createIndexSql = sqls.getCreateIndexSql(indexName, tableName, META_TABLE_TS); - logger.info("Creating index: " + createIndexSql); + logger.info("Creating index: {}", createIndexSql); pstat = connection.prepareStatement(createIndexSql); pstat.executeUpdate(); } catch (SQLException ex) { @@ -587,7 +587,7 @@ public class JDBCResourceStore extends PushdownResourceStore { t = t.getCause(); } - logger.trace("Not an unreachable exception with causes " + exceptionList.toString()); + logger.trace("Not an unreachable exception with causes {}", exceptionList); return false; }