[ 
https://issues.apache.org/jira/browse/KYLIN-3597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16713507#comment-16713507
 ] 

ASF GitHub Bot commented on KYLIN-3597:
---------------------------------------

hit-lacus closed pull request #374: KYLIN-3597 Improve code smell
URL: https://github.com/apache/kylin/pull/374
 
 
   

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-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java 
b/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
index 4fb9522c96..a9af3b7cfe 100644
--- a/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
@@ -21,6 +21,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 import org.apache.commons.lang3.StringUtils;
@@ -60,12 +61,15 @@
 
     private static final Logger logger = 
LoggerFactory.getLogger(CubeDescManager.class);
 
-    public static final Serializer<CubeDesc> CUBE_DESC_SERIALIZER = new 
JsonSerializer<CubeDesc>(CubeDesc.class);
+    public static final Serializer<CubeDesc> CUBE_DESC_SERIALIZER = new 
JsonSerializer<>(CubeDesc.class);
     
     public static CubeDescManager getInstance(KylinConfig config) {
         return config.getManager(CubeDescManager.class);
     }
 
+    static final String BROKEN_CUBE_MSG = "Broken cube desc %s";
+    static final String CUBE_SHOULD_NOT_BE_DRAFT_MSG = "CubeDesc '%s' must not 
be a draft";
+
     // called by reflection
     static CubeDescManager newInstance(KylinConfig config) throws IOException {
         return new CubeDescManager(config);
@@ -84,20 +88,20 @@ static CubeDescManager newInstance(KylinConfig config) 
throws IOException {
     private AutoReadWriteLock descMapLock = new AutoReadWriteLock();
 
     private CubeDescManager(KylinConfig cfg) throws IOException {
-        logger.info("Initializing CubeDescManager with config " + cfg);
+        logger.info("Initializing CubeDescManager with config {}", cfg);
         this.config = cfg;
-        this.cubeDescMap = new CaseInsensitiveStringCache<CubeDesc>(config, 
"cube_desc");
+        this.cubeDescMap = new CaseInsensitiveStringCache<>(config, 
"cube_desc");
         this.crud = new CachedCrudAssist<CubeDesc>(getStore(), 
ResourceStore.CUBE_DESC_RESOURCE_ROOT, CubeDesc.class,
                 cubeDescMap) {
             @Override
             protected CubeDesc initEntityAfterReload(CubeDesc cubeDesc, String 
resourceName) {
                 if (cubeDesc.isDraft())
-                    throw new IllegalArgumentException("CubeDesc '" + 
cubeDesc.getName() + "' must not be a draft");
+                    throw new 
IllegalArgumentException(String.format(Locale.ROOT, 
CUBE_SHOULD_NOT_BE_DRAFT_MSG, cubeDesc.getName()));
 
                 try {
                     cubeDesc.init(config);
                 } catch (Exception e) {
-                    logger.warn("Broken cube desc " + cubeDesc.resourceName(), 
e);
+                    logger.warn(String.format(Locale.ROOT, 
CUBE_SHOULD_NOT_BE_DRAFT_MSG, cubeDesc.resourceName()), e);
                     cubeDesc.addError(e.toString());
                 }
                 return cubeDesc;
@@ -147,7 +151,7 @@ public CubeDesc getCubeDesc(String name) {
 
     public List<CubeDesc> listAllDesc() {
         try (AutoLock lock = descMapLock.lockForRead()) {
-            return new ArrayList<CubeDesc>(cubeDescMap.values());
+            return new ArrayList<>(cubeDescMap.values());
         }
     }
     
@@ -175,10 +179,6 @@ public CubeDesc reloadCubeDescLocal(String name) throws 
IOException {
 
     /**
      * Create a new CubeDesc
-     * 
-     * @param cubeDesc
-     * @return
-     * @throws IOException
      */
     public CubeDesc createCubeDesc(CubeDesc cubeDesc) throws IOException {
         try (AutoLock lock = descMapLock.lockForWrite()) {
@@ -187,7 +187,7 @@ public CubeDesc createCubeDesc(CubeDesc cubeDesc) throws 
IOException {
             if (cubeDescMap.containsKey(cubeDesc.getName()))
                 throw new IllegalArgumentException("CubeDesc '" + 
cubeDesc.getName() + "' already exists");
             if (cubeDesc.isDraft())
-                throw new IllegalArgumentException("CubeDesc '" + 
cubeDesc.getName() + "' must not be a draft");
+                throw new IllegalArgumentException(String.format(Locale.ROOT, 
CUBE_SHOULD_NOT_BE_DRAFT_MSG, cubeDesc.getName()));
 
             try {
                 cubeDesc.init(config);
@@ -219,10 +219,6 @@ public CubeDesc createCubeDesc(CubeDesc cubeDesc) throws 
IOException {
 
     /**
      * Update CubeDesc with the input. Broadcast the event into cluster
-     * 
-     * @param desc
-     * @return
-     * @throws IOException
      */
     public CubeDesc updateCubeDesc(CubeDesc desc) throws IOException {
         try (AutoLock lock = descMapLock.lockForWrite()) {
@@ -233,7 +229,7 @@ public CubeDesc updateCubeDesc(CubeDesc desc) throws 
IOException {
             if (!cubeDescMap.containsKey(name))
                 throw new IllegalArgumentException("CubeDesc '" + name + "' 
does not exist.");
             if (desc.isDraft())
-                throw new IllegalArgumentException("CubeDesc '" + 
desc.getName() + "' must not be a draft");
+                throw new IllegalArgumentException(String.format(Locale.ROOT, 
CUBE_SHOULD_NOT_BE_DRAFT_MSG, desc.getName()));
 
             try {
                 desc.init(config);
diff --git 
a/core-cube/src/main/java/org/apache/kylin/cube/cli/DictionaryGeneratorCLI.java 
b/core-cube/src/main/java/org/apache/kylin/cube/cli/DictionaryGeneratorCLI.java
index 1b5cf635d8..8039c338e1 100644
--- 
a/core-cube/src/main/java/org/apache/kylin/cube/cli/DictionaryGeneratorCLI.java
+++ 
b/core-cube/src/main/java/org/apache/kylin/cube/cli/DictionaryGeneratorCLI.java
@@ -42,6 +42,8 @@
 
 public class DictionaryGeneratorCLI {
 
+    private DictionaryGeneratorCLI(){}
+
     private static final Logger logger = 
LoggerFactory.getLogger(DictionaryGeneratorCLI.class);
 
     public static void processSegment(KylinConfig config, String cubeName, 
String segmentID, String uuid,
@@ -58,7 +60,7 @@ private static void processSegment(KylinConfig config, 
CubeSegment cubeSeg, Stri
 
         // dictionary
         for (TblColRef col : 
cubeSeg.getCubeDesc().getAllColumnsNeedDictionaryBuilt()) {
-            logger.info("Building dictionary for " + col);
+            logger.info("Building dictionary for {}", col);
             IReadableTable inpTable = 
factTableValueProvider.getDistinctValuesFor(col);
 
             Dictionary<String> preBuiltDict = null;
@@ -67,10 +69,10 @@ private static void processSegment(KylinConfig config, 
CubeSegment cubeSeg, Stri
             }
 
             if (preBuiltDict != null) {
-                logger.debug("Dict for '" + col.getName() + "' has already 
been built, save it");
+                logger.debug("Dict for '{}' has already been built, save it", 
col.getName());
                 cubeMgr.saveDictionary(cubeSeg, col, inpTable, preBuiltDict);
             } else {
-                logger.debug("Dict for '" + col.getName() + "' not pre-built, 
build it from " + inpTable.toString());
+                logger.debug("Dict for '{}' not pre-built, build it from {}", 
col.getName(), inpTable);
                 cubeMgr.buildDictionary(cubeSeg, col, inpTable);
             }
         }
@@ -90,22 +92,22 @@ private static void processSegment(KylinConfig config, 
CubeSegment cubeSeg, Stri
         }
 
         for (String tableIdentity : toSnapshot) {
-            logger.info("Building snapshot of " + tableIdentity);
+            logger.info("Building snapshot of {}", tableIdentity);
             cubeMgr.buildSnapshotTable(cubeSeg, tableIdentity, uuid);
         }
 
         CubeInstance updatedCube = 
cubeMgr.getCube(cubeSeg.getCubeInstance().getName());
         cubeSeg = updatedCube.getSegmentById(cubeSeg.getUuid());
         for (TableRef lookup : toCheckLookup) {
-            logger.info("Checking snapshot of " + lookup);
+            logger.info("Checking snapshot of {}", lookup);
             try {
                 JoinDesc join = 
cubeSeg.getModel().getJoinsTree().getJoinByPKSide(lookup);
                 ILookupTable table = cubeMgr.getLookupTable(cubeSeg, join);
                 if (table != null) {
                     IOUtils.closeStream(table);
                 }
-            } catch (Throwable th) {
-                throw new RuntimeException("Checking snapshot of " + lookup + 
" failed.", th);
+            } catch (Exception th) {
+                throw new IllegalStateException("Checking snapshot of " + 
lookup + " failed.", th);
             }
         }
     }
diff --git 
a/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/DoggedCubeBuilder.java
 
b/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/DoggedCubeBuilder.java
index 06e4a5d582..d7579e1e69 100644
--- 
a/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/DoggedCubeBuilder.java
+++ 
b/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/DoggedCubeBuilder.java
@@ -93,7 +93,7 @@ public DoggedCubeBuilder(CuboidScheduler cuboidScheduler, 
IJoinedFlatTableDesc f
                     splits.add(last);
 
                     last.start();
-                    logger.info("Split #" + splits.size() + " kickoff");
+                    logger.info("Split #{} kickoff", splits.size());
 
                     // Build splits sequentially
                     last.join();
@@ -101,7 +101,7 @@ public DoggedCubeBuilder(CuboidScheduler cuboidScheduler, 
IJoinedFlatTableDesc f
                     checkException(splits);
                 }
 
-                logger.info("Dogged Cube Build splits complete, took " + 
(System.currentTimeMillis() - start) + " ms");
+                logger.info("Dogged Cube Build splits complete, took {} ms", 
(System.currentTimeMillis() - start));
 
                 merger.mergeAndOutput(splits, output);
 
@@ -116,7 +116,7 @@ else if (e instanceof RuntimeException)
             } finally {
                 output.close();
                 closeGirdTables(splits);
-                logger.info("Dogged Cube Build end, totally took " + 
(System.currentTimeMillis() - start) + " ms");
+                logger.info("Dogged Cube Build end, totally took {} ms", 
(System.currentTimeMillis() - start));
                 ensureExit(splits);
                 logger.info("Dogged Cube Build return");
             }
@@ -128,7 +128,7 @@ private void closeGirdTables(List<SplitThread> splits) {
                     for (CuboidResult r : split.buildResult.values()) {
                         try {
                             r.table.close();
-                        } catch (Throwable e) {
+                        } catch (Exception e) {
                             logger.error("Error closing grid table " + 
r.table, e);
                         }
                     }
@@ -144,7 +144,7 @@ private void ensureExit(List<SplitThread> splits) throws 
IOException {
                         abort(splits);
                     }
                 }
-            } catch (Throwable e) {
+            } catch (Exception e) {
                 logger.error("Dogged Cube Build error", e);
             }
         }
@@ -173,20 +173,7 @@ private void abort(List<SplitThread> splits) throws 
IOException {
                 if (split.exception != null)
                     errors.add(split.exception);
             }
-
-            if (errors.isEmpty()) {
-                return;
-            } else if (errors.size() == 1) {
-                Throwable t = errors.get(0);
-                if (t instanceof IOException)
-                    throw (IOException) t;
-                else
-                    throw new IOException(t);
-            } else {
-                for (Throwable t : errors)
-                    logger.error("Exception during in-mem cube build", t);
-                throw new IOException(errors.size() + " exceptions during 
in-mem cube build, cause set to the first, check log for more", errors.get(0));
-            }
+            InMemCubeBuilder.processErrors(errors);
         }
     }
 
diff --git 
a/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/InMemCubeBuilder.java
 
b/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/InMemCubeBuilder.java
index ef61ce9a5a..da393b62fe 100644
--- 
a/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/InMemCubeBuilder.java
+++ 
b/core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/InMemCubeBuilder.java
@@ -111,9 +111,6 @@ private GridTable newGridTableByCuboidID(long cuboidID) 
throws IOException {
                 new CubeDimEncMap(cubeDesc, dictionaryMap)
         );
 
-        // Below several store implementation are very similar in performance. 
The ConcurrentDiskStore is the simplest.
-        // MemDiskStore store = new MemDiskStore(info, memBudget == null ? 
MemoryBudgetController.ZERO_BUDGET : memBudget);
-        // MemDiskStore store = new MemDiskStore(info, 
MemoryBudgetController.ZERO_BUDGET);
         ConcurrentDiskStore store = new ConcurrentDiskStore(info);
 
         GridTable gridTable = new GridTable(info, store);
@@ -137,15 +134,13 @@ private GridTable newGridTableByCuboidID(long cuboidID) 
throws IOException {
 
     public <T> ConcurrentNavigableMap<Long, CuboidResult> 
build(RecordConsumeBlockingQueueController<T> input)
             throws IOException {
-        final ConcurrentNavigableMap<Long, CuboidResult> result = new 
ConcurrentSkipListMap<Long, CuboidResult>();
-        build(input, new ICuboidCollector() {
-            @Override
-            public void collect(CuboidResult cuboidResult) {
-                logger.info("collecting CuboidResult cuboid id:" + 
cuboidResult.cuboidId);
+        final ConcurrentNavigableMap<Long, CuboidResult> result = new 
ConcurrentSkipListMap<>();
+        build(input, cuboidResult -> {
+                logger.info("collecting CuboidResult cuboid id:{}", 
cuboidResult.cuboidId);
                 result.put(cuboidResult.cuboidId, cuboidResult);
             }
-        });
-        logger.info("total CuboidResult count:" + result.size());
+        );
+        logger.info("total CuboidResult count:{}", result.size());
         return result;
     }
 
@@ -156,13 +151,13 @@ public void collect(CuboidResult cuboidResult) {
     private <T> void build(RecordConsumeBlockingQueueController<T> input, 
ICuboidCollector collector)
             throws IOException {
         long startTime = System.currentTimeMillis();
-        logger.info("In Mem Cube Build start, " + cubeDesc.getName());
+        logger.info("In Mem Cube Build start, {}", cubeDesc.getName());
 
         baseCuboidMemTracker = new MemoryWaterLevel();
         baseCuboidMemTracker.markLow();
 
         // multiple threads to compute cuboid in parallel
-        taskPending = new TreeSet<CuboidTask>();
+        taskPending = new TreeSet<>();
         taskCuboidCompleted.set(0);
         taskThreads = prepareTaskThreads();
         taskThreadExceptions = new Throwable[taskThreadCount];
@@ -186,7 +181,7 @@ public void collect(CuboidResult cuboidResult) {
         join(taskThreads);
 
         long endTime = System.currentTimeMillis();
-        logger.info("In Mem Cube Build end, " + cubeDesc.getName() + ", takes 
" + (endTime - startTime) + " ms");
+        logger.info("In Mem Cube Build end, {}, takes {} ms", 
cubeDesc.getName(), (endTime - startTime));
 
         throwExceptionIfAny();
     }
@@ -216,15 +211,17 @@ private void join(Thread... threads) throws IOException {
     }
 
     private void throwExceptionIfAny() throws IOException {
-        ArrayList<Throwable> errors = new ArrayList<Throwable>();
+        ArrayList<Throwable> errors = new ArrayList<>();
         for (int i = 0; i < taskThreadCount; i++) {
             Throwable t = taskThreadExceptions[i];
             if (t != null)
                 errors.add(t);
         }
-        if (errors.isEmpty()) {
-            return;
-        } else if (errors.size() == 1) {
+        processErrors(errors);
+    }
+
+    public static void processErrors(List<Throwable> errors) throws 
IOException{
+        if (errors.size() == 1) {
             Throwable t = errors.get(0);
             if (t instanceof IOException)
                 throw (IOException) t;
@@ -283,7 +280,7 @@ public void run() {
                         }
                     }
                 }
-            } catch (Throwable ex) {
+            } catch (Exception ex) {
                 if (!isAllCuboidDone()) {
                     logger.error("task thread exception", ex);
                     taskThreadExceptions[id] = ex;
@@ -313,26 +310,26 @@ private void addChildTasks(CuboidResult parent) {
 
     private void makeMemoryBudget() {
         baseResult.aggrCacheMB = 
Math.max(baseCuboidMemTracker.getEstimateMB(), 10); // 10 MB at minimal
-        logger.debug("Base cuboid aggr cache is " + baseResult.aggrCacheMB + " 
MB");
+        logger.debug("Base cuboid aggr cache is {} MB", 
baseResult.aggrCacheMB);
         int systemAvailMB = MemoryBudgetController.gcAndGetSystemAvailMB();
-        logger.debug("System avail " + systemAvailMB + " MB");
+        logger.debug("System avail {} MB", systemAvailMB);
         int reserve = reserveMemoryMB;
-        logger.debug("Reserve " + reserve + " MB for system basics");
+        logger.debug("Reserve {} MB for system basics", reserve);
 
         int budget = systemAvailMB - reserve;
         if (budget < baseResult.aggrCacheMB) {
             // make sure we have base aggr cache as minimal
             budget = baseResult.aggrCacheMB;
-            logger.warn("System avail memory (" + systemAvailMB + " MB) is 
less than base aggr cache (" + baseResult.aggrCacheMB + " MB) + minimal 
reservation (" + reserve + " MB), consider increase JVM heap -Xmx");
+            logger.warn("System avail memory ({} MB) is less than base aggr 
cache ({} MB) + minimal reservation ({} MB), consider increase JVM heap -Xmx", 
systemAvailMB, baseResult.aggrCacheMB, reserve);
         }
 
-        logger.debug("Memory Budget is " + budget + " MB");
+        logger.debug("Memory Budget is {} MB", budget);
         memBudget = new MemoryBudgetController(budget);
     }
 
     private <T> CuboidResult 
createBaseCuboid(RecordConsumeBlockingQueueController<T> input) throws 
IOException {
         long startTime = System.currentTimeMillis();
-        logger.info("Calculating base cuboid " + baseCuboidId);
+        logger.info("Calculating base cuboid {}", baseCuboidId);
 
         GridTable baseCuboid = newGridTableByCuboidID(baseCuboidId);
         GTBuilder baseBuilder = baseCuboid.rebuild();
@@ -358,10 +355,10 @@ private void makeMemoryBudget() {
         }
 
         long timeSpent = System.currentTimeMillis() - startTime;
-        logger.info("Cuboid " + baseCuboidId + " has " + count + " rows, build 
takes " + timeSpent + "ms");
+        logger.info("Cuboid {} has {} rows, build takes {}ms", baseCuboidId, 
count, timeSpent);
 
         int mbEstimateBaseAggrCache = (int) 
(aggregationScanner.getEstimateSizeOfAggrCache() / 
MemoryBudgetController.ONE_MB);
-        logger.info("Wild estimate of base aggr cache is " + 
mbEstimateBaseAggrCache + " MB");
+        logger.info("Wild estimate of base aggr cache is {} MB", 
mbEstimateBaseAggrCache);
 
         return updateCuboidResult(baseCuboidId, baseCuboid, count, timeSpent, 
0, input.inputConverterUnit.ifChange());
     }
@@ -427,7 +424,7 @@ private GTAggregateScanner 
prepareGTAggregationScanner(GridTable gridTable, long
                 aggrMask[i] = 
!measureDescs[i].getFunction().getMeasureType().onlyAggrInBaseCuboid();
 
                 if (!aggrMask[i]) {
-                    logger.info(measureDescs[i].toString() + " doesn't need 
aggregation.");
+                    logger.info("{} doesn't need aggregation.", 
measureDescs[i]);
                 }
             }
             scanner.setAggrMask(aggrMask);
@@ -438,7 +435,7 @@ private GTAggregateScanner 
prepareGTAggregationScanner(GridTable gridTable, long
 
     private CuboidResult scanAndAggregateGridTable(GridTable gridTable, long 
parentId, long cuboidId, ImmutableBitSet aggregationColumns, ImmutableBitSet 
measureColumns) throws IOException {
         long startTime = System.currentTimeMillis();
-        logger.info("Calculating cuboid " + cuboidId);
+        logger.info("Calculating cuboid {}", cuboidId);
 
         GTAggregateScanner scanner = prepareGTAggregationScanner(gridTable, 
parentId, cuboidId, aggregationColumns, measureColumns);
         GridTable newGridTable = newGridTableByCuboidID(cuboidId);
@@ -458,16 +455,13 @@ private CuboidResult scanAndAggregateGridTable(GridTable 
gridTable, long parentI
                 builder.write(newRecord);
             }
 
-            //long t = System.currentTimeMillis();
-            //sanityCheck(parentId, cuboidId, 
scanner.getTotalSumForSanityCheck());
-            //logger.info("sanity check for Cuboid " + cuboidId + " cost " + 
(System.currentTimeMillis() - t) + "ms");
         } finally {
             scanner.close();
             builder.close();
         }
 
         long timeSpent = System.currentTimeMillis() - startTime;
-        logger.info("Cuboid " + cuboidId + " has " + count + " rows, build 
takes " + timeSpent + "ms");
+        logger.info("Cuboid {} has {} rows, build takes {}ms", cuboidId, 
count, timeSpent);
 
         return updateCuboidResult(cuboidId, newGridTable, count, timeSpent, 0);
     }
@@ -498,9 +492,11 @@ private void sanityCheck(long parentId, long cuboidId, 
Object[] totalSum) {
             return;
         }
         if (Arrays.equals(totalSumForSanityCheck, totalSum) == false) {
-            logger.info("sanityCheck failed when calculate " + cuboidId + " 
from parent " + parentId);
-            logger.info("Expected: " + 
Arrays.toString(totalSumForSanityCheck));
-            logger.info("Actually: " + Arrays.toString(totalSum));
+            logger.info("sanityCheck failed when calculate{} from parent {}", 
cuboidId, parentId);
+            if(logger.isInfoEnabled()){
+                logger.info("Expected: {}", 
Arrays.toString(totalSumForSanityCheck));
+                logger.info("Actually: {}", Arrays.toString(totalSum));
+            }
             throw new IllegalStateException();
         }
     }
@@ -519,7 +515,13 @@ private void sanityCheck(long parentId, long cuboidId, 
Object[] totalSum) {
         @Override
         public int compareTo(CuboidTask o) {
             long comp = this.childCuboidId - o.childCuboidId;
-            return comp < 0 ? -1 : (comp > 0 ? 1 : 0);
+            if(comp < 0){
+                return -1;
+            }else if(comp > 0){
+                return 1;
+            }else {
+                return 0;
+            }
         }
 
         @Override
diff --git 
a/core-metadata/src/main/java/org/apache/kylin/measure/percentile/PercentileSerializer.java
 
b/core-metadata/src/main/java/org/apache/kylin/measure/percentile/PercentileSerializer.java
index 203a975d63..8e89b6218b 100644
--- 
a/core-metadata/src/main/java/org/apache/kylin/measure/percentile/PercentileSerializer.java
+++ 
b/core-metadata/src/main/java/org/apache/kylin/measure/percentile/PercentileSerializer.java
@@ -25,7 +25,7 @@
 
 public class PercentileSerializer extends 
DataTypeSerializer<PercentileCounter> {
     // be thread-safe and avoid repeated obj creation
-    private transient ThreadLocal<PercentileCounter> current = null;
+    private transient ThreadLocal<PercentileCounter> counterThreadLocal = null;
 
     private double compression;
 
@@ -48,19 +48,20 @@ public int getStorageBytesEstimate() {
         return current().getBytesEstimate();
     }
 
+    @Override
     protected double getStorageBytesEstimate(double count) {
         return current().getBytesEstimate(count);
     }
 
     private PercentileCounter current() {
-        if (current == null) {
-            current = new ThreadLocal<>();
+        if (counterThreadLocal == null) {
+            counterThreadLocal = new ThreadLocal<>();
         }
 
-        PercentileCounter counter = current.get();
+        PercentileCounter counter = counterThreadLocal.get();
         if (counter == null) {
             counter = new PercentileCounter(compression);
-            current.set(counter);
+            counterThreadLocal.set(counter);
         }
         return counter;
     }
diff --git 
a/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java
 
b/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java
index bf52f91117..b0da934933 100644
--- 
a/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java
+++ 
b/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java
@@ -52,7 +52,6 @@
 import org.apache.kylin.metadata.model.TblColRef;
 import org.apache.kylin.query.relnode.ColumnRowType;
 
-import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 
@@ -130,10 +129,10 @@ public TupleFilter visitCall(RexCall call) {
                 // is a trivial expr
                 return f;
             }
-            //else go to default
-        default:
             filter = new 
UnsupportedTupleFilter(TupleFilter.FilterOperatorEnum.UNSUPPORTED);
             break;
+        default:
+            filter = new 
UnsupportedTupleFilter(TupleFilter.FilterOperatorEnum.UNSUPPORTED);
         }
 
         for (RexNode operand : call.operands) {
@@ -145,12 +144,12 @@ public TupleFilter visitCall(RexCall call) {
             }
         }
 
-        if (op.getKind() == SqlKind.OR) {
+        if (op.getKind() == SqlKind.OR && filter != null) {
             CompareTupleFilter inFilter = mergeToInClause(filter);
             if (inFilter != null) {
                 filter = inFilter;
             }
-        } else if (op.getKind() == SqlKind.NOT) {
+        } else if (op.getKind() == SqlKind.NOT && filter != null) {
             assert (filter.getChildren().size() == 1);
             filter = filter.getChildren().get(0).reverse();
         }
@@ -183,8 +182,9 @@ private TupleFilter dealWithTrivialExpr(RexCall call) {
             }
         }
 
-        Preconditions.checkNotNull(left);
-        Preconditions.checkNotNull(right);
+        if(left == null || right == null){
+            throw new NullPointerException();
+        }
 
         switch (call.op.getKind()) {
         case PLUS:
@@ -281,7 +281,6 @@ public TupleFilter visitLiteral(RexLiteral literal) {
             strValue = ((NlsString) literalValue).getValue();
         } else if (literalValue instanceof GregorianCalendar) {
             GregorianCalendar g = (GregorianCalendar) literalValue;
-            //strValue = "" + g.get(Calendar.YEAR) + "-" + 
normToTwoDigits(g.get(Calendar.MONTH) + 1) + "-" + 
normToTwoDigits(g.get(Calendar.DAY_OF_MONTH));
             strValue = Long.toString(g.getTimeInMillis());
         } else if (literalValue instanceof TimeUnitRange) {
             // Extract(x from y) in where clause
diff --git a/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java 
b/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java
index 5aad5e0e26..649a1b33e1 100644
--- a/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java
+++ b/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java
@@ -126,7 +126,7 @@ private static void initQueryTransformers() {
                 IQueryTransformer t = (IQueryTransformer) 
ClassUtil.newInstance(clz);
                 transformers.add(t);
             } catch (Exception e) {
-                throw new RuntimeException("Failed to init query transformer", 
e);
+                throw new IllegalStateException("Failed to init query 
transformer", e);
             }
         }
 
@@ -178,8 +178,9 @@ public static boolean isSelectStatement(String sql) {
         String sql1 = sql.toLowerCase(Locale.ROOT);
         sql1 = removeCommentInSql(sql1);
         sql1 = sql1.trim();
-        return sql1.startsWith("select") || (sql1.startsWith("with") && 
sql1.contains("select"))
-                || (sql1.startsWith("explain") && sql1.contains("select"));
+        String select = "select";
+        return sql1.startsWith(select) || (sql1.startsWith("with") && 
sql1.contains(select))
+                || (sql1.startsWith("explain") && sql1.contains(select));
     }
 
     public static String removeCommentInSql(String sql1) {
diff --git 
a/source-kafka/src/main/java/org/apache/kylin/source/kafka/util/KafkaSampleProducer.java
 
b/source-kafka/src/main/java/org/apache/kylin/source/kafka/util/KafkaSampleProducer.java
index 5899c7c7fb..51e8ff3d08 100644
--- 
a/source-kafka/src/main/java/org/apache/kylin/source/kafka/util/KafkaSampleProducer.java
+++ 
b/source-kafka/src/main/java/org/apache/kylin/source/kafka/util/KafkaSampleProducer.java
@@ -56,7 +56,7 @@
     private static final ObjectMapper mapper = new ObjectMapper();
 
     public static void main(String[] args) throws Exception {
-        logger.info("args: {}", Arrays.toString(args));
+        if(logger.isInfoEnabled()) logger.info("args: {}", 
Arrays.toString(args));
         OptionsHelper optionsHelper = new OptionsHelper();
         Options options = new Options();
         options.addOption(OPTION_TOPIC);
@@ -109,12 +109,12 @@ public static void main(String[] args) throws Exception {
         props.put("buffer.memory", 33554432);
         props.put("key.serializer", 
"org.apache.kafka.common.serialization.StringSerializer");
         props.put("value.serializer", 
"org.apache.kafka.common.serialization.StringSerializer");
-
+        long startTime = System.currentTimeMillis();
         try (Producer<String, String> producer = new KafkaProducer<>(props)) {
             boolean alive = true;
             Random rnd = new Random();
             Map<String, Object> record = new HashMap<>();
-            while (alive == true) {
+            while (alive) {
                 //add normal record
                 record.put("order_time", (new Date().getTime()));
                 record.put("country", 
countries.get(rnd.nextInt(countries.size())));
@@ -132,9 +132,12 @@ public static void main(String[] args) throws Exception {
                 record.put("user", user);
                 //send message
                 ProducerRecord<String, String> data = new 
ProducerRecord<>(topic, System.currentTimeMillis() + "", 
mapper.writeValueAsString(record));
-                System.out.println("Sending 1 message: " + 
JsonUtil.writeValueAsString(record));
+                if(logger.isInfoEnabled()) logger.info("Sending 1 message: 
{}", JsonUtil.writeValueAsString(record));
                 producer.send(data);
                 Thread.sleep(interval);
+                if(System.currentTimeMillis() - startTime <= 7 * 24 * 3600 * 
1000){
+                    alive = false;
+                }
             }
         }
     }
diff --git 
a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/RowCounterCLI.java
 
b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/RowCounterCLI.java
index d6367e5a09..1a659cba56 100644
--- 
a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/RowCounterCLI.java
+++ 
b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/RowCounterCLI.java
@@ -59,9 +59,10 @@ public static void main(String[] args) throws IOException {
         } else {
             logger.info("startkey lenght: {}", startKey.length);
         }
-
-        logger.info("start key in binary: {}", Bytes.toStringBinary(startKey));
-        logger.info("end key in binary: {}", Bytes.toStringBinary(endKey));
+        if(logger.isInfoEnabled()){
+            logger.info("start key in binary: {}", 
Bytes.toStringBinary(startKey));
+            logger.info("end key in binary: {}", Bytes.toStringBinary(endKey));
+        }
 
         Configuration conf = HBaseConnection.getCurrentHBaseConfiguration();
 


 

----------------------------------------------------------------
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


> Fix sonar reported static code issues
> -------------------------------------
>
>                 Key: KYLIN-3597
>                 URL: https://issues.apache.org/jira/browse/KYLIN-3597
>             Project: Kylin
>          Issue Type: Improvement
>          Components: Others
>            Reporter: Shaofeng SHI
>            Priority: Major
>             Fix For: v2.6.0
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to