shaofengshi closed pull request #285: KYLIN-3597 fix sonar reported issues
URL: https://github.com/apache/kylin/pull/285
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/HDFSResourceStore.java
b/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java
index c8819de17f..60a647716c 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 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 @@
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 @@ private void createMetaFolder(Path metaDirName) throws
IOException {
fs.mkdirs(metaDirName);
}
- logger.info("hdfs meta path created: %s", metaDirName);
+ logger.info("hdfs meta path created: {}", metaDirName);
}
@Override
@@ -131,7 +135,8 @@ protected boolean existsImpl(String resPath) throws
IOException {
}
@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 @@ protected RawResource getResourceImpl(String resPath)
throws IOException {
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 @@ protected RawResource getResourceImpl(String resPath)
throws IOException {
}
}
+ 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 @@ protected long getResourceTimestampImpl(String resPath)
throws IOException {
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 @@ protected void putResourceImpl(String resPath,
InputStream content, long ts) thr
}
@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 1262680d9b..9643e8dfe6 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 @@ private void beforeChange(String resPath) throws
IOException {
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 @@ public static String dumpResources(KylinConfig
kylinConfig, Collection<String> d
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 ca3809114f..29650c4077 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 static void copyR(ResourceStore src, ResourceStore
dst, String path, Tree
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 b7e97a1d8f..20b67c1727 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.storage.hbase.HBaseConnection;
import com.google.common.collect.Lists;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class GridTableHBaseBenchmark {
@@ -54,6 +56,7 @@
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 static void main(String[] args) throws IOException {
}
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 static void testGridTable(double hitRatio, double
indexRatio) throws IOEx
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 @@ private static void prepareData(Connection conn) throws
IOException {
}
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 @@ private static void prepareData(Connection conn) throws
IOException {
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 @@ private static void createHTableIfNeeded(Connection conn,
String tableName) thro
}
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 @@ private static void createHTableIfNeeded(Connection conn,
String tableName) thro
desc.addFamily(fd);
hbase.createTable(desc);
- System.out.println("HTable '" + tableName + "' created");
+ logger.info("HTable '" + tableName + "' created");
} finally {
hbase.close();
}
@@ -381,14 +383,13 @@ private void discard(byte n) {
}
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");
}
}
----------------------------------------------------------------
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
