From dev-return-14413-archive-asf-public=cust-asf.ponee.io@kylin.apache.org Thu Oct 11 11:26:01 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 502A518067E for ; Thu, 11 Oct 2018 11:26:00 +0200 (CEST) Received: (qmail 70167 invoked by uid 500); 11 Oct 2018 09:25:59 -0000 Mailing-List: contact dev-help@kylin.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kylin.apache.org Delivered-To: mailing list dev@kylin.apache.org Received: (qmail 70156 invoked by uid 99); 11 Oct 2018 09:25:59 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 11 Oct 2018 09:25:59 +0000 From: GitBox To: dev@kylin.apache.org Subject: [GitHub] shaofengshi closed pull request #285: KYLIN-3597 fix sonar reported issues Message-ID: <153924995871.31476.9978714775521352772.gitbox@gitbox.apache.org> Date: Thu, 11 Oct 2018 09:25:58 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 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 getAllResourcesImpl(String folderPath, long timeStart, long timeEndExclusive) throws IOException { + protected List getAllResourcesImpl(String folderPath, long timeStart, long timeEndExclusive) + throws IOException { NavigableSet 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 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