[
https://issues.apache.org/jira/browse/KYLIN-3597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16642685#comment-16642685
]
ASF GitHub Bot commented on KYLIN-3597:
---------------------------------------
shaofengshi closed pull request #283: KYLIN-3597 fix code smells
URL: https://github.com/apache/kylin/pull/283
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/persistence/BrokenEntity.java
b/core-common/src/main/java/org/apache/kylin/common/persistence/BrokenEntity.java
index 6e1b4c28e6..b86b0d984d 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 @@
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 18ffd473d0..99e85dc315 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 @@ protected void putResourceImpl(String resPath, InputStream
content, long ts) thr
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 e5bef40e77..c8819de17f 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 HDFSResourceStore(KylinConfig kylinConfig,
StorageURL metadataUrl) throws
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 @@ protected long getResourceTimestampImpl(String resPath)
throws IOException {
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 @@ protected void putResourceImpl(String resPath,
InputStream content, long ts) thr
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 @@ private Path getRealHDFSPath(String resourcePath) {
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 fced09d711..f727566fa4 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 @@
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 static boolean checkCondition(boolean condition,
String message, Object..
}
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 static boolean checkPortAvailable(int port) {
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 bcead85217..3a559616b9 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 int getIdFromValueBytesWithoutCache(byte[] value,
int offset, int len, in
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 4f0c8166da..1ca83bc7c9 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 @@
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 RocksDBLookupTable(TableDesc tableDesc, String[]
keyColumns, String dbPat
}
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 RocksDBLookupTable(TableDesc tableDesc, String[]
keyColumns, String dbPat
return new Iterator<String[]>() {
int counter;
+
@Override
public boolean hasNext() {
boolean valid = rocksIterator.isValid();
@@ -87,7 +86,7 @@ public boolean hasNext() {
@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 95ffa35335..3adbb8ef51 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.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.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 boolean isSegmentSkipped() {
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 5f4d4be9b9..07d241fe23 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 @@ private FunctionDesc findAggrFuncFromCubeDesc(FunctionDesc
aggrFunc) {
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 @@ private boolean isExactAggregation(StorageContext context,
Cuboid cuboid, Collec
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 b746501539..ae146d3966 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 @@
*/
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 @@ private ZookeeperDistributedLock(CuratorFramework curator,
String zkPathBase, St
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 boolean lock(String lockPath) {
} 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 boolean lock(String lockPath, long timeout) {
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 String peekLock(String lockPath) {
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 void unlock(String lockPath) {
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 void purgeLocks(String lockPathRoot) {
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 Closeable watchLocks(String lockPathRoot, Executor
executor, final Watche
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;
----------------------------------------------------------------
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
> Fix sonar reported static code issues
> -------------------------------------
>
> Key: KYLIN-3597
> URL: https://issues.apache.org/jira/browse/KYLIN-3597
> Project: Kylin
> Issue Type: Improvement
> Components: Others
> Reporter: Shaofeng SHI
> Priority: Major
> Fix For: v2.6.0
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
