kylin-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] shaofengshi closed pull request #285: KYLIN-3597 fix sonar reported issues
Date Thu, 11 Oct 2018 09:25:58 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message