This is an automated email from the ASF dual-hosted git repository. billyliu 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 23e4ffa KYLIN-3597 Fix sonar reported static code issues 23e4ffa is described below commit 23e4ffa795baede6a772091fe823d5d4bf3048ee Author: chao long <wayn...@qq.com> AuthorDate: Thu Sep 27 14:34:05 2018 +0800 KYLIN-3597 Fix sonar reported static code issues --- .../org/apache/kylin/metadata/model/TableDesc.java | 11 ++-- .../kylin/engine/spark/SparkCubingMerge.java | 7 ++- .../kylin/engine/spark/SparkFactDistinct.java | 59 +++++++++++----------- .../kylin/engine/spark/SparkMergingDictionary.java | 31 +++++------- .../kylin/storage/hbase/steps/SparkCubeHFile.java | 12 ++--- .../storage/hbase/util/UpdateHTableHostCLI.java | 6 +-- 6 files changed, 59 insertions(+), 67 deletions(-) diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java index d8e3b02..63a78f8 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java @@ -74,7 +74,8 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { if (cut >= 0) path = path.substring(cut + 1); - String table, prj; + String table; + String prj; int dash = path.indexOf("--"); if (dash >= 0) { table = path.substring(0, dash); @@ -153,9 +154,10 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { if (existingColumns[i].getName().equalsIgnoreCase(computedColumns[j].getName())) { // if we're adding a computed column twice, it should be allowed without producing duplicates if (!existingColumns[i].isComputedColumn()) { - throw new IllegalArgumentException(String.format(Locale.ROOT, + String errorMsg = String.format(Locale.ROOT, "There is already a column named %s on table %s, please change your computed column name", - new Object[] { computedColumns[j].getName(), this.getIdentity() })); + computedColumns[j].getName(), this.getIdentity()); + throw new IllegalArgumentException(errorMsg); } else { isFreshCC = false; } @@ -178,7 +180,7 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { public ColumnDesc findColumnByName(String name) { //ignore the db name and table name if exists - int lastIndexOfDot = name.lastIndexOf("."); + int lastIndexOfDot = name.lastIndexOf('.'); if (lastIndexOfDot >= 0) { name = name.substring(lastIndexOfDot + 1); } @@ -204,6 +206,7 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { * @deprecated this is for compatible with data model v1; * @return */ + @Deprecated public String getResourcePathV1() { return concatResourcePath(name, null); } diff --git a/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkCubingMerge.java b/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkCubingMerge.java index 991c31e..0b03f70 100644 --- a/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkCubingMerge.java +++ b/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkCubingMerge.java @@ -143,7 +143,7 @@ public class SparkCubingMerge extends AbstractApplication implements Serializabl }; final PairFunction convertTextFunction = new PairFunction<Tuple2<Text, Object[]>, org.apache.hadoop.io.Text, org.apache.hadoop.io.Text>() { - private volatile transient boolean initialized = false; + private transient volatile boolean initialized = false; BufferedMeasureCodec codec; @Override @@ -231,12 +231,11 @@ public class SparkCubingMerge extends AbstractApplication implements Serializabl } // output the data size to console, job engine will parse and save the metric // please note: this mechanism won't work when spark.submit.deployMode=cluster - logger.info("HDFS: Number of bytes written=" + jobListener.metrics.getBytesWritten()); - // HadoopUtil.deleteHDFSMeta(metaUrl); + logger.info("HDFS: Number of bytes written={}", jobListener.metrics.getBytesWritten()); } static class ReEncodCuboidFunction implements PairFunction<Tuple2<Text, Text>, Text, Object[]> { - private volatile transient boolean initialized = false; + private transient volatile boolean initialized = false; private String cubeName; private String sourceSegmentId; private String mergedSegmentId; diff --git a/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkFactDistinct.java b/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkFactDistinct.java index 5a59167..043f479 100644 --- a/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkFactDistinct.java +++ b/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkFactDistinct.java @@ -234,19 +234,19 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab } static class FlatOutputFucntion implements PairFlatMapFunction<Iterator<String[]>, SelfDefineSortableKey, Text> { - private volatile transient boolean initialized = false; + private transient volatile boolean initialized = false; private String cubeName; private String segmentId; private String metaUrl; private SerializableConfiguration conf; private int samplingPercent; - private CuboidStatCalculator cuboidStatCalculator; - private FactDistinctColumnsReducerMapping reducerMapping; + private transient CuboidStatCalculator cuboidStatCalculator; + private transient FactDistinctColumnsReducerMapping reducerMapping; private List<TblColRef> allCols; private int[] columnIndex; - private DictColDeduper dictColDeduper; + private transient DictColDeduper dictColDeduper; private Map<Integer, DimensionRangeInfo> dimensionRangeInfoMap; - private ByteBuffer tmpbuf; + private transient ByteBuffer tmpbuf; private LongAccumulator bytesWritten; public FlatOutputFucntion(String cubeName, String segmentId, String metaurl, SerializableConfiguration conf, @@ -374,8 +374,9 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab result.add(new Tuple2<SelfDefineSortableKey, Text>(sortableKey, outputValue)); } - for (Integer colIndex : dimensionRangeInfoMap.keySet()) { - DimensionRangeInfo rangeInfo = dimensionRangeInfoMap.get(colIndex); + for (Map.Entry<Integer, DimensionRangeInfo> entry : dimensionRangeInfoMap.entrySet()) { + int colIndex = entry.getKey(); + DimensionRangeInfo rangeInfo = entry.getValue(); DataType dataType = allCols.get(colIndex).getType(); addFieldValue(dataType, colIndex, rangeInfo.getMin(), result); addFieldValue(dataType, colIndex, rangeInfo.getMax(), result); @@ -458,7 +459,7 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab // log a few rows for troubleshooting if (result.size() < 10) { - logger.info("Sample output: " + allCols.get(colIndex) + " '" + value + "' => reducer " + reducerIndex); + logger.info("Sample output: {} '{}' => reducer {}", allCols.get(colIndex), value, reducerIndex); } } @@ -573,12 +574,12 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab } static class FactDistinctPartitioner extends Partitioner { - private volatile transient boolean initialized = false; + private transient volatile boolean initialized = false; private String cubeName; private String metaUrl; private SerializableConfiguration conf; private int totalReducerNum; - private FactDistinctColumnsReducerMapping reducerMapping; + private transient FactDistinctColumnsReducerMapping reducerMapping; public FactDistinctPartitioner(String cubeName, String metaUrl, SerializableConfiguration conf, int totalReducerNum) { @@ -626,14 +627,14 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab static class MultiOutputFunction implements PairFlatMapFunction<Iterator<Tuple2<SelfDefineSortableKey, Iterable<Text>>>, String, Tuple3<Writable, Writable, String>> { - private volatile transient boolean initialized = false; + private transient volatile boolean initialized = false; private String DICT_FILE_POSTFIX = ".rldict"; private String DIMENSION_COL_INFO_FILE_POSTFIX = ".dci"; private String cubeName; private String metaUrl; private SerializableConfiguration conf; private int samplingPercent; - private FactDistinctColumnsReducerMapping reducerMapping; + private transient FactDistinctColumnsReducerMapping reducerMapping; private int taskId; private boolean isStatistics = false; private long baseCuboidId; @@ -641,7 +642,7 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab private Map<Long, HLLCounter> cuboidHLLMap; private TblColRef col; private boolean buildDictInReducer; - private IDictionaryBuilder builder; + private transient IDictionaryBuilder builder; private int rowCount = 0; private long totalRowsBeforeMerge = 0; private KylinConfig cubeConfig; @@ -677,7 +678,7 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab baseCuboidRowCountInMappers = Lists.newArrayList(); cuboidHLLMap = Maps.newHashMap(); - logger.info("Partition " + taskId + " handling stats"); + logger.info("Partition {} handling stats", taskId); } else { // normal col col = reducerMapping.getColForReducer(taskId); @@ -697,8 +698,7 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab builder = DictionaryGenerator.newDictionaryBuilder(col.getType()); builder.init(null, 0, null); } - logger.info("Partition " + taskId + " handling column " + col + ", buildDictInReducer=" - + buildDictInReducer); + logger.info("Partition {} handling column {}, buildDictInReducer={}", taskId, col, buildDictInReducer); } initialized = true; @@ -707,7 +707,7 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab private void logAFewRows(String value) { if (rowCount < 10) { - logger.info("Received value: " + value); + logger.info("Received value: {}", value); } } @@ -781,7 +781,7 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab } if (isStatistics) { - //output the hll info; + //output the hll info List<Long> allCuboids = Lists.newArrayList(); allCuboids.addAll(cuboidHLLMap.keySet()); Collections.sort(allCuboids); @@ -803,26 +803,26 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab return result.iterator(); } - private void logMapperAndCuboidStatistics(List<Long> allCuboids) throws IOException { - logger.info("Cuboid number for task: " + taskId + "\t" + allCuboids.size()); - logger.info("Samping percentage: \t" + samplingPercent); + private void logMapperAndCuboidStatistics(List<Long> allCuboids) { + logger.info("Cuboid number for task: {}\t{}", taskId, allCuboids.size()); + logger.info("Samping percentage: \t{}", samplingPercent); logger.info("The following statistics are collected based on sampling data. "); - logger.info("Number of Mappers: " + baseCuboidRowCountInMappers.size()); + logger.info("Number of Mappers: {}", baseCuboidRowCountInMappers.size()); for (int i = 0; i < baseCuboidRowCountInMappers.size(); i++) { if (baseCuboidRowCountInMappers.get(i) > 0) { - logger.info("Base Cuboid in Mapper " + i + " row count: \t " + baseCuboidRowCountInMappers.get(i)); + logger.info("Base Cuboid in Mapper {} row count: \t {}", i, baseCuboidRowCountInMappers.get(i)); } } long grantTotal = 0; for (long i : allCuboids) { grantTotal += cuboidHLLMap.get(i).getCountEstimate(); - logger.info("Cuboid " + i + " row count is: \t " + cuboidHLLMap.get(i).getCountEstimate()); + logger.info("Cuboid {} row count is: \t {}", i, cuboidHLLMap.get(i).getCountEstimate()); } - logger.info("Sum of row counts (before merge) is: \t " + totalRowsBeforeMerge); - logger.info("After merge, the row count: \t " + grantTotal); + logger.info("Sum of row counts (before merge) is: \t {}", totalRowsBeforeMerge); + logger.info("After merge, the row count: \t {}", grantTotal); } private void outputDimRangeInfo(List<Tuple2<String, Tuple3<Writable, Writable, String>>> result) { @@ -836,14 +836,13 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab result.add(new Tuple2<String, Tuple3<Writable, Writable, String>>(BatchConstants.CFG_OUTPUT_PARTITION, new Tuple3<Writable, Writable, String>(NullWritable.get(), new Text(maxValue.getBytes(StandardCharsets.UTF_8)), dimRangeFileName))); - logger.info("write dimension range info for col : " + col.getName() + " minValue:" + minValue - + " maxValue:" + maxValue); + logger.info("write dimension range info for col : {} minValue:{} maxValue:{}", col.getName(), minValue, maxValue); } } private void outputDict(TblColRef col, Dictionary<String> dict, List<Tuple2<String, Tuple3<Writable, Writable, String>>> result) - throws IOException, InterruptedException { + throws IOException { // output written to baseDir/colName/colName.rldict-r-00000 (etc) String dictFileName = col.getIdentity() + "/" + col.getName() + DICT_FILE_POSTFIX; @@ -860,7 +859,7 @@ public class SparkFactDistinct extends AbstractApplication implements Serializab private void outputStatistics(List<Long> allCuboids, List<Tuple2<String, Tuple3<Writable, Writable, String>>> result) - throws IOException, InterruptedException { + throws IOException { // output written to baseDir/statistics/statistics-r-00000 (etc) String statisticsFileName = BatchConstants.CFG_OUTPUT_STATISTICS + "/" + BatchConstants.CFG_OUTPUT_STATISTICS; diff --git a/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkMergingDictionary.java b/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkMergingDictionary.java index 4d4346f..37f957f 100644 --- a/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkMergingDictionary.java +++ b/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkMergingDictionary.java @@ -156,8 +156,8 @@ public class SparkMergingDictionary extends AbstractApplication implements Seria colToDictPathRDD.coalesce(1, false).saveAsNewAPIHadoopFile(dictOutputPath, Text.class, Text.class, SequenceFileOutputFormat.class); } - static public class MergeDictAndStatsFunction implements PairFunction<Integer, Text, Text> { - private volatile transient boolean initialized = false; + public static class MergeDictAndStatsFunction implements PairFunction<Integer, Text, Text> { + private transient volatile boolean initialized = false; private String cubeName; private String metaUrl; private String segmentId; @@ -165,7 +165,7 @@ public class SparkMergingDictionary extends AbstractApplication implements Seria private String statOutputPath; private TblColRef[] tblColRefs; private SerializableConfiguration conf; - private DictionaryManager dictMgr; + private transient DictionaryManager dictMgr; private KylinConfig kylinConfig; private List<CubeSegment> mergingSegments; @@ -236,32 +236,27 @@ public class SparkMergingDictionary extends AbstractApplication implements Seria for (CubeSegment cubeSegment : mergingSegments) { String filePath = cubeSegment.getStatisticsResourcePath(); - InputStream is = rs.getResource(filePath).inputStream; - File tempFile; - FileOutputStream tempFileStream = null; - try { - tempFile = File.createTempFile(segmentId, ".seq"); - tempFileStream = new FileOutputStream(tempFile); + File tempFile = File.createTempFile(segmentId, ".seq"); + + try(InputStream is = rs.getResource(filePath).inputStream; + FileOutputStream tempFileStream = new FileOutputStream(tempFile)) { + org.apache.commons.io.IOUtils.copy(is, tempFileStream); - } finally { - IOUtils.closeStream(is); - IOUtils.closeStream(tempFileStream); } FileSystem fs = HadoopUtil.getFileSystem("file:///" + tempFile.getAbsolutePath()); - SequenceFile.Reader reader = null; - try { - conf = HadoopUtil.getCurrentConfiguration(); + conf = HadoopUtil.getCurrentConfiguration(); + + try(SequenceFile.Reader reader = new SequenceFile.Reader(fs, new Path(tempFile.getAbsolutePath()), conf)) { //noinspection deprecation - reader = new SequenceFile.Reader(fs, new Path(tempFile.getAbsolutePath()), conf); LongWritable key = (LongWritable) ReflectionUtils.newInstance(reader.getKeyClass(), conf); BytesWritable value = (BytesWritable) ReflectionUtils.newInstance(reader.getValueClass(), conf); while (reader.next(key, value)) { if (key.get() == 0L) { - // sampling percentage; + // sampling percentage averageSamplingPercentage += Bytes.toInt(value.getBytes()); } else if (key.get() > 0) { HLLCounter hll = new HLLCounter(kylinConfig.getCubeStatsHLLPrecision()); @@ -275,8 +270,6 @@ public class SparkMergingDictionary extends AbstractApplication implements Seria } } } - } finally { - IOUtils.closeStream(reader); } } diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/steps/SparkCubeHFile.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/steps/SparkCubeHFile.java index 539f03b..e2d43ba 100644 --- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/steps/SparkCubeHFile.java +++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/steps/SparkCubeHFile.java @@ -171,21 +171,21 @@ public class SparkCubeHFile extends AbstractApplication implements Serializable Writable value = NullWritable.get(); while (reader.next(key, value)) { keys.add(key); - logger.info(" ------- split key: " + key); + logger.info(" ------- split key: {}", key); key = new RowKeyWritable(); // important, new an object! } } - logger.info("There are " + keys.size() + " split keys, totally " + (keys.size() + 1) + " hfiles"); + logger.info("There are {} split keys, totally {} hfiles", keys.size(), (keys.size() + 1)); //HBase conf - logger.info("Loading HBase configuration from:" + hbaseConfFile); + logger.info("Loading HBase configuration from:{}", hbaseConfFile); FSDataInputStream confInput = fs.open(new Path(hbaseConfFile)); Configuration hbaseJobConf = new Configuration(); hbaseJobConf.addResource(confInput); hbaseJobConf.set("spark.hadoop.dfs.replication", "3"); // HFile, replication=3 - Job job = new Job(hbaseJobConf, cubeSegment.getStorageLocationIdentifier()); + Job job = Job.getInstance(hbaseJobConf, cubeSegment.getStorageLocationIdentifier()); FileOutputFormat.setOutputPath(job, new Path(outputPath)); @@ -233,15 +233,13 @@ public class SparkCubeHFile extends AbstractApplication implements Serializable } }).saveAsNewAPIHadoopDataset(job.getConfiguration()); - logger.info("HDFS: Number of bytes written=" + jobListener.metrics.getBytesWritten()); + logger.info("HDFS: Number of bytes written={}", jobListener.metrics.getBytesWritten()); Map<String, String> counterMap = Maps.newHashMap(); counterMap.put(ExecutableConstants.HDFS_BYTES_WRITTEN, String.valueOf(jobListener.metrics.getBytesWritten())); // save counter to hdfs HadoopUtil.writeToSequenceFile(sc.hadoopConfiguration(), counterPath, counterMap); - - //HadoopUtil.deleteHDFSMeta(metaUrl); } static class HFilePartitioner extends Partitioner { diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/UpdateHTableHostCLI.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/UpdateHTableHostCLI.java index 3f290ac..bf5c4e8 100644 --- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/UpdateHTableHostCLI.java +++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/UpdateHTableHostCLI.java @@ -82,7 +82,7 @@ public class UpdateHTableHostCLI { } else if (!filterType.equals("-all")) { printUsageAndExit(); } - logger.info("These htables are needed to be updated: " + StringUtils.join(tableNames, ",")); + logger.info("These htables are needed to be updated: {}", StringUtils.join(tableNames, ",")); UpdateHTableHostCLI updateHTableHostCLI = new UpdateHTableHostCLI(tableNames, oldHostValue); updateHTableHostCLI.execute(); @@ -119,13 +119,13 @@ public class UpdateHTableHostCLI { private static List<String> getHTableNames(KylinConfig config) { CubeManager cubeMgr = CubeManager.getInstance(config); - ArrayList<String> result = new ArrayList<String>(); + ArrayList<String> result = new ArrayList<>(); for (CubeInstance cube : cubeMgr.listAllCubes()) { for (CubeSegment seg : cube.getSegments(SegmentStatusEnum.READY)) { String tableName = seg.getStorageLocationIdentifier(); if (!StringUtils.isBlank(tableName)) { result.add(tableName); - System.out.println("added new table: " + tableName); + logger.info("added new table: {}", tableName); } } }