You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by sh...@apache.org on 2018/10/09 01:49:06 UTC
[kylin] branch master updated: KYLIN-3597 fix code smells
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 46c9f55 KYLIN-3597 fix code smells
46c9f55 is described below
commit 46c9f55b98fa9ea1230f62137fb53264e8980343
Author: shaofengshi <sh...@apache.org>
AuthorDate: Mon Oct 8 19:29:52 2018 +0800
KYLIN-3597 fix code smells
---
.../kylin/common/persistence/BrokenEntity.java | 2 +-
.../common/persistence/FileResourceStore.java | 1 +
.../common/persistence/HDFSResourceStore.java | 30 +++++++---------------
.../org/apache/kylin/common/util/CheckUtil.java | 5 ++--
.../apache/kylin/dict/AppendTrieDictionary.java | 2 +-
.../dict/lookup/cache/RocksDBLookupTable.java | 9 +++----
.../kylin/storage/gtrecord/CubeSegmentScanner.java | 5 +---
.../storage/gtrecord/GTCubeStorageQueryBase.java | 6 ++---
.../hbase/util/ZookeeperDistributedLock.java | 23 +++++++++--------
9 files changed, 35 insertions(+), 48 deletions(-)
diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/BrokenEntity.java b/core-common/src/main/java/org/apache/kylin/common/persistence/BrokenEntity.java
index 6e1b4c2..b86b0d9 100644
--- a/core-common/src/main/java/org/apache/kylin/common/persistence/BrokenEntity.java
+++ b/core-common/src/main/java/org/apache/kylin/common/persistence/BrokenEntity.java
@@ -22,7 +22,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
public class BrokenEntity extends RootPersistentEntity {
- public static final byte[] MAGIC = new byte[]{'B', 'R', 'O', 'K', 'E', 'N'};
+ protected static final byte[] MAGIC = new byte[]{'B', 'R', 'O', 'K', 'E', 'N'};
@JsonProperty("resPath")
private String resPath;
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 18ffd47..99e85dc 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
@@ -161,6 +161,7 @@ public class FileResourceStore extends ResourceStore {
IOUtils.copy(content, out);
} finally {
IOUtils.closeQuietly(out);
+ IOUtils.closeQuietly(content);
}
f.setLastModified(ts);
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 e5bef40..c8819de 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
@@ -65,28 +65,28 @@ public class HDFSResourceStore extends ResourceStore {
if (path == null) {
// missing path is not expected, but don't fail it
path = kylinConfig.getHdfsWorkingDirectory() + "tmp_metadata";
- logger.warn("Missing path, fall back to {0}", path);
+ logger.warn("Missing path, fall back to %s", path);
}
fs = HadoopUtil.getFileSystem(path);
Path metadataPath = new Path(path);
if (fs.exists(metadataPath) == false) {
- logger.warn("Path not exist in HDFS, create it: {0}", path);
+ logger.warn("Path not exist in HDFS, create it: %s", path);
createMetaFolder(metadataPath);
}
hdfsMetaPath = metadataPath;
- logger.info("hdfs meta path : {0}", hdfsMetaPath.toString());
+ logger.info("hdfs meta path : %s", hdfsMetaPath);
}
- private void createMetaFolder(Path metaDirName) throws Exception {
+ private void createMetaFolder(Path metaDirName) throws IOException {
//create hdfs meta path
if (!fs.exists(metaDirName)) {
fs.mkdirs(metaDirName);
}
- logger.info("hdfs meta path created: {0}", metaDirName.toString());
+ logger.info("hdfs meta path created: %s", metaDirName);
}
@Override
@@ -175,17 +175,10 @@ public class HDFSResourceStore extends ResourceStore {
if (!fs.exists(p) || !fs.isFile(p)) {
return 0;
}
- FSDataInputStream in = null;
- try {
- in = fs.open(p);
+ try (FSDataInputStream in = fs.open(p)) {
long t = in.readLong();
return t;
- } catch (Exception e) {
- throw new IOException("Put resource fail", e);
- } finally {
- IOUtils.closeQuietly(in);
}
-
}
@Override
@@ -193,16 +186,11 @@ public class HDFSResourceStore extends ResourceStore {
logger.trace("res path : {0}", resPath);
Path p = getRealHDFSPath(resPath);
logger.trace("put resource : {0}", p.toUri());
- FSDataOutputStream out = null;
- try {
- out = fs.create(p, true);
+ try (FSDataOutputStream out = fs.create(p, true)) {
out.writeLong(ts);
IOUtils.copy(content, out);
-
- } catch (Exception e) {
- throw new IOException("Put resource fail", e);
} finally {
- IOUtils.closeQuietly(out);
+ IOUtils.closeQuietly(content);
}
}
@@ -245,7 +233,7 @@ public class HDFSResourceStore extends ResourceStore {
if (resourcePath.equals("/"))
return this.hdfsMetaPath;
if (resourcePath.startsWith("/") && resourcePath.length() > 1)
- resourcePath = resourcePath.substring(1, resourcePath.length());
+ resourcePath = resourcePath.substring(1);
return new Path(this.hdfsMetaPath, resourcePath);
}
}
diff --git a/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java b/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java
index fced09d..f727566 100644
--- a/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java
+++ b/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java
@@ -27,6 +27,7 @@ import org.slf4j.LoggerFactory;
public class CheckUtil {
public static final Logger logger = LoggerFactory.getLogger(CheckUtil.class);
+ private static final Random rand = new Random();
private CheckUtil(){
throw new IllegalStateException("Class CheckUtil is an utility class !");
@@ -42,13 +43,12 @@ public class CheckUtil {
}
public static int randomAvailablePort(int minPort, int maxPort) {
- Random rand = new Random();
for (int i = 0; i < 100; i++) {
int p = minPort + rand.nextInt(maxPort - minPort);
if (checkPortAvailable(p))
return p;
}
- throw new RuntimeException("Failed to get random available port between [" + minPort + "," + maxPort + ")");
+ throw new IllegalArgumentException("Failed to get random available port between [" + minPort + "," + maxPort + ")");
}
/**
@@ -65,6 +65,7 @@ public class CheckUtil {
ds.setReuseAddress(true);
return true;
} catch (IOException e) {
+ logger.error("Exception in checking port, should be ignored.");
}
return false;
diff --git a/core-dictionary/src/main/java/org/apache/kylin/dict/AppendTrieDictionary.java b/core-dictionary/src/main/java/org/apache/kylin/dict/AppendTrieDictionary.java
index bcead85..3a55961 100644
--- a/core-dictionary/src/main/java/org/apache/kylin/dict/AppendTrieDictionary.java
+++ b/core-dictionary/src/main/java/org/apache/kylin/dict/AppendTrieDictionary.java
@@ -117,7 +117,7 @@ public class AppendTrieDictionary<T> extends CacheDictionary<T> {
try {
slice = dictCache.get(sliceKey);
} catch (ExecutionException e) {
- throw new RuntimeException("Failed to load slice with key " + sliceKey, e.getCause());
+ throw new IllegalStateException("Failed to load slice with key " + sliceKey, e.getCause());
}
return slice.getIdFromValueBytesImpl(value, offset, len, roundingFlag);
}
diff --git a/core-dictionary/src/main/java/org/apache/kylin/dict/lookup/cache/RocksDBLookupTable.java b/core-dictionary/src/main/java/org/apache/kylin/dict/lookup/cache/RocksDBLookupTable.java
index 4f0c816..1ca83bc 100644
--- a/core-dictionary/src/main/java/org/apache/kylin/dict/lookup/cache/RocksDBLookupTable.java
+++ b/core-dictionary/src/main/java/org/apache/kylin/dict/lookup/cache/RocksDBLookupTable.java
@@ -38,20 +38,18 @@ public class RocksDBLookupTable implements ILookupTable {
RocksDB.loadLibrary();
}
- private TableDesc tableDesc;
private RocksDB rocksDB;
private Options options;
private RocksDBLookupRowEncoder rowEncoder;
public RocksDBLookupTable(TableDesc tableDesc, String[] keyColumns, String dbPath) {
- this.tableDesc = tableDesc;
this.options = new Options();
this.rowEncoder = new RocksDBLookupRowEncoder(tableDesc, keyColumns);
try {
this.rocksDB = RocksDB.openReadOnly(options, dbPath);
} catch (RocksDBException e) {
- throw new RuntimeException("cannot open rocks db in path:" + dbPath, e);
+ throw new IllegalStateException("cannot open rocks db in path:" + dbPath, e);
}
}
@@ -65,7 +63,7 @@ public class RocksDBLookupTable implements ILookupTable {
}
return rowEncoder.decode(new KV(encodeKey, value));
} catch (RocksDBException e) {
- throw new RuntimeException("error when get key from rocksdb", e);
+ throw new IllegalStateException("error when get key from rocksdb", e);
}
}
@@ -76,6 +74,7 @@ public class RocksDBLookupTable implements ILookupTable {
return new Iterator<String[]>() {
int counter;
+
@Override
public boolean hasNext() {
boolean valid = rocksIterator.isValid();
@@ -87,7 +86,7 @@ public class RocksDBLookupTable implements ILookupTable {
@Override
public String[] next() {
- counter ++;
+ counter++;
if (counter % 100000 == 0) {
logger.info("scanned {} rows from rocksDB", counter);
}
diff --git a/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/CubeSegmentScanner.java b/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/CubeSegmentScanner.java
index 95ffa35..3adbb8e 100644
--- a/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/CubeSegmentScanner.java
+++ b/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/CubeSegmentScanner.java
@@ -30,7 +30,6 @@ import org.apache.kylin.dict.BuiltInFunctionTransformer;
import org.apache.kylin.gridtable.GTInfo;
import org.apache.kylin.gridtable.GTRecord;
import org.apache.kylin.gridtable.GTScanRequest;
-import org.apache.kylin.gridtable.IGTScanner;
import org.apache.kylin.metadata.expression.TupleExpression;
import org.apache.kylin.metadata.filter.ITupleFilterTransformer;
import org.apache.kylin.metadata.filter.StringCodeSystem;
@@ -43,7 +42,7 @@ import org.apache.kylin.storage.StorageContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class CubeSegmentScanner implements IGTScanner {
+public class CubeSegmentScanner implements Iterable<GTRecord> {
private static final Logger logger = LoggerFactory.getLogger(CubeSegmentScanner.class);
@@ -98,12 +97,10 @@ public class CubeSegmentScanner implements IGTScanner {
return scanner.iterator();
}
- @Override
public void close() throws IOException {
scanner.close();
}
- @Override
public GTInfo getInfo() {
return scanRequest == null ? null : scanRequest.getInfo();
}
diff --git a/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/GTCubeStorageQueryBase.java b/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/GTCubeStorageQueryBase.java
index 5f4d4be..07d241f 100644
--- a/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/GTCubeStorageQueryBase.java
+++ b/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/GTCubeStorageQueryBase.java
@@ -306,8 +306,8 @@ public abstract class GTCubeStorageQueryBase implements IStorageQuery {
private long getQueryFilterMask(Set<TblColRef> filterColumnD) {
long filterMask = 0;
- logger.info("Filter column set for query: {0}", filterColumnD.toString());
- if (filterColumnD.size() != 0) {
+ logger.info("Filter column set for query: %s", filterColumnD);
+ if (filterColumnD.isEmpty() == false) {
RowKeyColDesc[] allColumns = cubeDesc.getRowkey().getRowKeyColumns();
for (int i = 0; i < allColumns.length; i++) {
if (filterColumnD.contains(allColumns[i].getColRef())) {
@@ -600,7 +600,7 @@ public abstract class GTCubeStorageQueryBase implements IStorageQuery {
if (partDesc.isPartitioned()) {
TblColRef col = partDesc.getPartitionDateColumnRef();
if (!groups.contains(col) && !singleValuesD.contains(col)) {
- logger.info("exactAggregation is false because cube is partitioned and " + col + " is not on group by");
+ logger.info("exactAggregation is false because cube is partitioned and %s is not on group by", col);
return false;
}
}
diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java
index b746501..ae146d3 100644
--- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java
+++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java
@@ -19,7 +19,7 @@
package org.apache.kylin.storage.hbase.util;
import java.io.Closeable;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
import java.util.Random;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@@ -48,6 +48,7 @@ import org.slf4j.LoggerFactory;
*/
public class ZookeeperDistributedLock implements DistributedLock, JobLock {
private static Logger logger = LoggerFactory.getLogger(ZookeeperDistributedLock.class);
+ private static final Random random = new Random();
public static class Factory extends DistributedLockFactory {
@@ -128,7 +129,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock {
this.curator = curator;
this.zkPathBase = zkPathBase;
this.client = client;
- this.clientBytes = client.getBytes(Charset.forName("UTF-8"));
+ this.clientBytes = client.getBytes(StandardCharsets.UTF_8);
}
@Override
@@ -147,7 +148,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock {
} catch (KeeperException.NodeExistsException ex) {
logger.debug("{} see {} is already locked", client, lockPath);
} catch (Exception ex) {
- throw new RuntimeException("Error while " + client + " trying to lock " + lockPath, ex);
+ throw new IllegalStateException("Error while " + client + " trying to lock " + lockPath, ex);
}
String lockOwner = peekLock(lockPath);
@@ -172,13 +173,13 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock {
logger.debug("{} will wait for lock path {}", client, lockPath);
long waitStart = System.currentTimeMillis();
- Random random = new Random();
- long sleep = 10 * 1000; // 10 seconds
+ long sleep = 10 * 1000L; // 10 seconds
while (System.currentTimeMillis() - waitStart <= timeout) {
try {
Thread.sleep((long) (1000 + sleep * random.nextDouble()));
} catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
return false;
}
@@ -198,11 +199,11 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock {
try {
byte[] bytes = curator.getData().forPath(lockPath);
- return new String(bytes, Charset.forName("UTF-8"));
+ return new String(bytes, StandardCharsets.UTF_8);
} catch (KeeperException.NoNodeException ex) {
return null;
} catch (Exception ex) {
- throw new RuntimeException("Error while peeking at " + lockPath, ex);
+ throw new IllegalStateException("Error while peeking at " + lockPath, ex);
}
}
@@ -234,7 +235,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock {
logger.info("{} released lock at {}", client, lockPath);
} catch (Exception ex) {
- throw new RuntimeException("Error while " + client + " trying to unlock " + lockPath, ex);
+ throw new IllegalStateException("Error while " + client + " trying to unlock " + lockPath, ex);
}
}
@@ -248,7 +249,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock {
logger.info("{} purged all locks under {}", client, lockPathRoot);
} catch (Exception ex) {
- throw new RuntimeException("Error while " + client + " trying to purge " + lockPathRoot, ex);
+ throw new IllegalStateException("Error while " + client + " trying to purge " + lockPathRoot, ex);
}
}
@@ -264,10 +265,10 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock {
public void childEvent(CuratorFramework client, PathChildrenCacheEvent event) throws Exception {
switch (event.getType()) {
case CHILD_ADDED:
- watcher.onLock(event.getData().getPath(), new String(event.getData().getData(), Charset.forName("UTF-8")));
+ watcher.onLock(event.getData().getPath(), new String(event.getData().getData(), StandardCharsets.UTF_8));
break;
case CHILD_REMOVED:
- watcher.onUnlock(event.getData().getPath(), new String(event.getData().getData(), Charset.forName("UTF-8")));
+ watcher.onUnlock(event.getData().getPath(), new String(event.getData().getData(), StandardCharsets.UTF_8));
break;
default:
break;