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/11 09:26:01 UTC
[kylin] branch master updated: KYLIN-3597 fix sonar reported issues
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 97e9520 KYLIN-3597 fix sonar reported issues
97e9520 is described below
commit 97e95207bb14db2399f85e22c0d001d95a4ff187
Author: shaofengshi <sh...@apache.org>
AuthorDate: Wed Oct 10 19:21:32 2018 +0800
KYLIN-3597 fix sonar reported issues
---
.../common/persistence/HDFSResourceStore.java | 50 +++++++++++++---------
.../kylin/common/persistence/ResourceStore.java | 24 +++++++----
.../kylin/common/persistence/ResourceTool.java | 7 ++-
.../hbase/util/GridTableHBaseBenchmark.java | 25 +++++------
4 files changed, 63 insertions(+), 43 deletions(-)
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 c8819de..60a6477 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
@@ -43,6 +43,10 @@ import org.slf4j.LoggerFactory;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+/**
+ * A ResourceStore implementation with HDFS as the storage.
+ * The typical scenario is as read-only or single thread temporary storage for metadata.
+ */
public class HDFSResourceStore extends ResourceStore {
private static final Logger logger = LoggerFactory.getLogger(HDFSResourceStore.class);
@@ -53,30 +57,30 @@ public class HDFSResourceStore extends ResourceStore {
private static final String HDFS_SCHEME = "hdfs";
- public HDFSResourceStore(KylinConfig kylinConfig) throws Exception {
+ public HDFSResourceStore(KylinConfig kylinConfig) throws IOException {
this(kylinConfig, kylinConfig.getMetadataUrl());
}
-
- public HDFSResourceStore(KylinConfig kylinConfig, StorageURL metadataUrl) throws Exception {
+
+ public HDFSResourceStore(KylinConfig kylinConfig, StorageURL metadataUrl) throws IOException {
super(kylinConfig);
Preconditions.checkState(HDFS_SCHEME.equals(metadataUrl.getScheme()));
-
+
String path = metadataUrl.getParameter("path");
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 %s", 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: %s", path);
+ if (!fs.exists(metadataPath)) {
+ logger.warn("Path not exist in HDFS, create it: {}", path);
createMetaFolder(metadataPath);
}
hdfsMetaPath = metadataPath;
- logger.info("hdfs meta path : %s", hdfsMetaPath);
+ logger.info("hdfs meta path : {}", hdfsMetaPath);
}
@@ -86,7 +90,7 @@ public class HDFSResourceStore extends ResourceStore {
fs.mkdirs(metaDirName);
}
- logger.info("hdfs meta path created: %s", metaDirName);
+ logger.info("hdfs meta path created: {}", metaDirName);
}
@Override
@@ -131,7 +135,8 @@ public class HDFSResourceStore extends ResourceStore {
}
@Override
- protected List<RawResource> getAllResourcesImpl(String folderPath, long timeStart, long timeEndExclusive) throws IOException {
+ protected List<RawResource> getAllResourcesImpl(String folderPath, long timeStart, long timeEndExclusive)
+ throws IOException {
NavigableSet<String> resources = listResources(folderPath);
if (resources == null)
return Collections.emptyList();
@@ -159,9 +164,9 @@ public class HDFSResourceStore extends ResourceStore {
Path p = getRealHDFSPath(resPath);
if (fs.exists(p) && fs.isFile(p)) {
if (fs.getFileStatus(p).getLen() == 0) {
- logger.warn("Zero length file: {0}", p.toString());
+ logger.warn("Zero length file: {}", p);
}
- FSDataInputStream in = fs.open(p);
+ FSDataInputStream in = getHDFSFileInputStream(fs, p);
long t = in.readLong();
return new RawResource(in, t);
} else {
@@ -169,6 +174,10 @@ public class HDFSResourceStore extends ResourceStore {
}
}
+ private FSDataInputStream getHDFSFileInputStream(FileSystem fs, Path path) throws IOException {
+ return fs.open(path);
+ }
+
@Override
protected long getResourceTimestampImpl(String resPath) throws IOException {
Path p = getRealHDFSPath(resPath);
@@ -176,16 +185,15 @@ public class HDFSResourceStore extends ResourceStore {
return 0;
}
try (FSDataInputStream in = fs.open(p)) {
- long t = in.readLong();
- return t;
+ return in.readLong();
}
}
@Override
protected void putResourceImpl(String resPath, InputStream content, long ts) throws IOException {
- logger.trace("res path : {0}", resPath);
+ logger.trace("res path : {}", resPath);
Path p = getRealHDFSPath(resPath);
- logger.trace("put resource : {0}", p.toUri());
+ logger.trace("put resource : {}", p.toUri());
try (FSDataOutputStream out = fs.create(p, true)) {
out.writeLong(ts);
IOUtils.copy(content, out);
@@ -195,17 +203,19 @@ public class HDFSResourceStore extends ResourceStore {
}
@Override
- protected long checkAndPutResourceImpl(String resPath, byte[] content, long oldTS, long newTS) throws IOException, WriteConflictException {
+ protected long checkAndPutResourceImpl(String resPath, byte[] content, long oldTS, long newTS) throws IOException {
Path p = getRealHDFSPath(resPath);
if (!fs.exists(p)) {
if (oldTS != 0) {
- throw new IllegalStateException("For not exist file. OldTS have to be 0. but Actual oldTS is : " + oldTS);
+ throw new IllegalStateException(
+ "For not exist file. OldTS have to be 0. but Actual oldTS is : " + oldTS);
}
} else {
long realLastModify = getResourceTimestamp(resPath);
if (realLastModify != oldTS) {
- throw new WriteConflictException("Overwriting conflict " + resPath + ", expect old TS " + oldTS + ", but found " + realLastModify);
+ throw new WriteConflictException("Overwriting conflict " + resPath + ", expect old TS " + oldTS
+ + ", but found " + realLastModify);
}
}
putResourceImpl(resPath, new ByteArrayInputStream(content), newTS);
diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java
index 1262680..9643e8d 100644
--- a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java
+++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java
@@ -398,17 +398,20 @@ abstract public class ResourceStore {
origResData.put(resPath, null);
origResTimestamp.put(resPath, null);
} else {
- origResData.put(resPath, readAll(raw.inputStream));
- origResTimestamp.put(resPath, raw.timestamp);
+ try {
+ origResData.put(resPath, readAll(raw.inputStream));
+ origResTimestamp.put(resPath, raw.timestamp);
+ } finally {
+ IOUtils.closeQuietly(raw.inputStream);
+ }
}
}
private byte[] readAll(InputStream inputStream) throws IOException {
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- IOUtils.copy(inputStream, out);
- inputStream.close();
- out.close();
- return out.toByteArray();
+ try (ByteArrayOutputStream out = new ByteArrayOutputStream();) {
+ IOUtils.copy(inputStream, out);
+ return out.toByteArray();
+ }
}
public void rollback() {
@@ -493,8 +496,11 @@ abstract public class ResourceStore {
RawResource res = from.getResource(path);
if (res == null)
throw new IllegalStateException("No resource found at -- " + path);
- to.putResource(path, res.inputStream, res.timestamp);
- res.inputStream.close();
+ try {
+ to.putResource(path, res.inputStream, res.timestamp);
+ } finally {
+ IOUtils.closeQuietly(res.inputStream);
+ }
}
String metaDirURI = OptionsHelper.convertToFileURL(metaDir.getAbsolutePath());
diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java
index ca38091..29650c4 100644
--- a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java
+++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java
@@ -222,8 +222,11 @@ public class ResourceTool {
try {
RawResource res = src.getResource(path);
if (res != null) {
- dst.putResource(path, res.inputStream, res.timestamp);
- res.inputStream.close();
+ try {
+ dst.putResource(path, res.inputStream, res.timestamp);
+ } finally {
+ IOUtils.closeQuietly(res.inputStream);
+ }
} else {
System.out.println("Resource not exist for " + path);
}
diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/GridTableHBaseBenchmark.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/GridTableHBaseBenchmark.java
index b7e97a1..20b67c1 100644
--- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/GridTableHBaseBenchmark.java
+++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/GridTableHBaseBenchmark.java
@@ -43,6 +43,8 @@ import org.apache.kylin.common.util.Pair;
import org.apache.kylin.storage.hbase.HBaseConnection;
import com.google.common.collect.Lists;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class GridTableHBaseBenchmark {
@@ -54,6 +56,7 @@ public class GridTableHBaseBenchmark {
private static final double DFT_HIT_RATIO = 0.3;
private static final double DFT_INDEX_RATIO = 0.1;
private static final int ROUND = 3;
+ protected static final Logger logger = LoggerFactory.getLogger(GridTableHBaseBenchmark.class);
public static void main(String[] args) throws IOException {
double hitRatio = DFT_HIT_RATIO;
@@ -74,7 +77,7 @@ public class GridTableHBaseBenchmark {
}
public static void testGridTable(double hitRatio, double indexRatio) throws IOException {
- System.out.println("Testing grid table scanning, hit ratio " + hitRatio + ", index ratio " + indexRatio);
+ logger.info("Testing grid table scanning, hit ratio " + hitRatio + ", index ratio " + indexRatio);
StorageURL hbaseUrl = StorageURL.valueOf("default@hbase"); // use hbase-site.xml on classpath
Connection conn = HBaseConnection.get(hbaseUrl);
@@ -84,7 +87,7 @@ public class GridTableHBaseBenchmark {
Hits hits = new Hits(N_ROWS, hitRatio, indexRatio);
for (int i = 0; i < ROUND; i++) {
- System.out.println("==================================== ROUND " + (i + 1)
+ logger.info("==================================== ROUND " + (i + 1)
+ " ========================================");
testRowScanWithIndex(conn, hits.getHitsForRowScanWithIndex());
testRowScanNoIndexFullScan(conn, hits.getHitsForRowScanNoIndex());
@@ -222,14 +225,14 @@ public class GridTableHBaseBenchmark {
}
if (nRows > 0) {
- System.out.println(nRows + " existing rows");
+ logger.info(nRows + " existing rows");
if (nRows != N_ROWS)
throw new IOException("Expect " + N_ROWS + " rows but it is not");
return;
}
// insert rows into empty table
- System.out.println("Writing " + N_ROWS + " rows to " + TEST_TABLE);
+ logger.info("Writing " + N_ROWS + " rows to " + TEST_TABLE);
long nBytes = 0;
for (int i = 0; i < N_ROWS; i++) {
byte[] rowkey = Bytes.toBytes(i);
@@ -240,8 +243,7 @@ public class GridTableHBaseBenchmark {
nBytes += cell.length;
dot(i, N_ROWS);
}
- System.out.println();
- System.out.println("Written " + N_ROWS + " rows, " + nBytes + " bytes");
+ logger.info("Written " + N_ROWS + " rows, " + nBytes + " bytes");
} finally {
IOUtils.closeQuietly(table);
@@ -274,11 +276,11 @@ public class GridTableHBaseBenchmark {
}
if (tableExist) {
- System.out.println("HTable '" + tableName + "' already exists");
+ logger.info("HTable '" + tableName + "' already exists");
return;
}
- System.out.println("Creating HTable '" + tableName + "'");
+ logger.info("Creating HTable '" + tableName + "'");
HTableDescriptor desc = new HTableDescriptor(TableName.valueOf(tableName));
@@ -287,7 +289,7 @@ public class GridTableHBaseBenchmark {
desc.addFamily(fd);
hbase.createTable(desc);
- System.out.println("HTable '" + tableName + "' created");
+ logger.info("HTable '" + tableName + "' created");
} finally {
hbase.close();
}
@@ -381,14 +383,13 @@ public class GridTableHBaseBenchmark {
}
public void markStart() {
- System.out.println(name + " starts");
+ logger.info(name + " starts");
startTime = System.currentTimeMillis();
}
public void markEnd() {
endTime = System.currentTimeMillis();
- System.out.println();
- System.out.println(name + " ends, " + (endTime - startTime) + " ms, " + rowsRead + " rows read, "
+ logger.info(name + " ends, " + (endTime - startTime) + " ms, " + rowsRead + " rows read, "
+ bytesRead + " bytes read");
}
}