shaofengshi closed pull request #393: KYLIN-3597 improve code smell
URL: https://github.com/apache/kylin/pull/393
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 018552caf1..f67f6b3479 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 @@
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 @@ public String getHdfsWorkingDirectory() {
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 @@ public String getMetastoreBigCellHdfsDirectory() {
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 @@ public String getMetadataUrlPrefix() {
}
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 @@ public int getScanThreshold() {
}
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 @@ public String getMemCachedHosts() {
}
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 @@ public String getSQLResponseSignatureClass() {
}
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 25420a4d7e..c7d963d40c 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 @@ public long bytesWritten() {
}
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 2cd699ba8b..315c51efcb 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 @@ private boolean checkIfAllowRetry(Throwable ex) {
}
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 9d9cf58e9a..bccb7a369e 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.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 @@ protected RawResource getResourceImpl(String resPath)
throws IOException {
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 @@ protected void putResourceImpl(String resPath,
ContentWriter content, long ts) t
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 c41892b5b6..c38a182f0e 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 HDFSResourceStore(KylinConfig kylinConfig,
StorageURL metadataUrl) throws
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 @@ private void createMetaFolder(Path metaDirName) throws
Exception {
fs.mkdirs(metaDirName);
}
- logger.info("hdfs meta path created: " + metaDirName.toString());
+ logger.info("hdfs meta path created: {}", metaDirName);
}
@Override
@@ -185,7 +185,7 @@ protected RawResource getResourceImpl(String resPath)
throws IOException {
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 @@ protected long getResourceTimestampImpl(String resPath)
throws IOException {
@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 c51d9b6d81..dc3a45b00f 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 void execute(Connection connection) throws
SQLException {
}
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 @@ protected boolean isUnreachableException(Throwable ex) {
t = t.getCause();
}
- logger.trace("Not an unreachable exception with causes " +
exceptionList.toString());
+ logger.trace("Not an unreachable exception with causes {}",
exceptionList);
return false;
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
