This is an automated email from the ASF dual-hosted git repository. shaofengshi 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 24efe15 KYLIN-3597 Fix some issues reported by SonarCloud 24efe15 is described below commit 24efe159dce75255462b1792363937587bdf56fa Author: Lijun Cao <641507...@qq.com> AuthorDate: Wed Sep 26 10:49:58 2018 +0800 KYLIN-3597 Fix some issues reported by SonarCloud --- .../org/apache/kylin/common/util/CheckUtil.java | 25 +++++++------------- .../java/org/apache/kylin/cube/CubeManager.java | 27 +++++++++++----------- .../java/org/apache/kylin/source/jdbc/SqlUtil.java | 4 ++++ .../hbase/util/ZookeeperDistributedLock.java | 18 +++++++-------- 4 files changed, 34 insertions(+), 40 deletions(-) diff --git a/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java b/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java index 0f75ff2..fced09d 100644 --- a/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java +++ b/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java @@ -28,6 +28,10 @@ import org.slf4j.LoggerFactory; public class CheckUtil { public static final Logger logger = LoggerFactory.getLogger(CheckUtil.class); + private CheckUtil(){ + throw new IllegalStateException("Class CheckUtil is an utility class !"); + } + public static boolean checkCondition(boolean condition, String message, Object... args) { if (condition) { return true; @@ -53,27 +57,14 @@ public class CheckUtil { * @param port the port to check for availability */ public static boolean checkPortAvailable(int port) { - ServerSocket ss = null; - DatagramSocket ds = null; - try { - ss = new ServerSocket(port); + + try(ServerSocket ss = new ServerSocket(port); + DatagramSocket ds = new DatagramSocket(port); + ) { ss.setReuseAddress(true); - ds = new DatagramSocket(port); ds.setReuseAddress(true); return true; } catch (IOException e) { - } finally { - if (ds != null) { - ds.close(); - } - - if (ss != null) { - try { - ss.close(); - } catch (IOException e) { - /* should not be thrown */ - } - } } return false; diff --git a/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java b/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java index 0dc825f..1cacd0d 100755 --- a/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java +++ b/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java @@ -125,7 +125,7 @@ public class CubeManager implements IRealizationProvider { private DictionaryAssist dictAssist = new DictionaryAssist(); private CubeManager(KylinConfig cfg) throws IOException { - logger.info("Initializing CubeManager with config " + cfg); + logger.info("Initializing CubeManager with config {}", cfg); this.config = cfg; this.cubeMap = new CaseInsensitiveStringCache<CubeInstance>(config, "cube"); this.crud = new CachedCrudAssist<CubeInstance>(getStore(), ResourceStore.CUBE_RESOURCE_ROOT, CubeInstance.class, @@ -222,7 +222,7 @@ public class CubeManager implements IRealizationProvider { public CubeInstance createCube(String cubeName, String projectName, CubeDesc desc, String owner) throws IOException { try (AutoLock lock = cubeMapLock.lockForWrite()) { - logger.info("Creating cube '" + projectName + "-->" + cubeName + "' from desc '" + desc.getName() + "'"); + logger.info("Creating cube '{}-->{}' from desc '{}'", projectName, cubeName, desc.getName()); // save cube resource CubeInstance cube = CubeInstance.create(cubeName, desc); @@ -238,7 +238,7 @@ public class CubeManager implements IRealizationProvider { public CubeInstance createCube(CubeInstance cube, String projectName, String owner) throws IOException { try (AutoLock lock = cubeMapLock.lockForWrite()) { - logger.info("Creating cube '" + projectName + "-->" + cube.getName() + "' from instance object. '"); + logger.info("Creating cube '{}-->{}' from instance object. '", projectName, cube.getName()); // save cube resource cube.setOwner(owner); @@ -313,7 +313,7 @@ public class CubeManager implements IRealizationProvider { throw new IllegalStateException(); CubeInstance cube = update.getCubeInstance(); - logger.info("Updating cube instance '" + cube.getName() + "'"); + logger.info("Updating cube instance '{}'", cube.getName()); Segments<CubeSegment> newSegs = (Segments) cube.getSegments().clone(); @@ -327,7 +327,7 @@ public class CubeManager implements IRealizationProvider { CubeSegment currentSeg = iterator.next(); for (CubeSegment toRemoveSeg : update.getToRemoveSegs()) { if (currentSeg.getUuid().equals(toRemoveSeg.getUuid())) { - logger.info("Remove segment " + currentSeg.toString()); + logger.info("Remove segment {}", currentSeg.toString()); toRemoveResources.add(currentSeg.getStatisticsResourcePath()); iterator.remove(); break; @@ -380,7 +380,7 @@ public class CubeManager implements IRealizationProvider { try { cube = crud.save(cube); } catch (WriteConflictException ise) { - logger.warn("Write conflict to update cube " + cube.getName() + " at try " + retry + ", will retry..."); + logger.warn("Write conflict to update cube {} at try {}, will retry...", cube.getName(), retry); if (retry >= 7) { logger.error("Retried 7 times till got error, abandoning...", ise); throw ise; @@ -396,7 +396,7 @@ public class CubeManager implements IRealizationProvider { try { getStore().deleteResource(resource); } catch (IOException ioe) { - logger.error("Failed to delete resource " + toRemoveResources.toString()); + logger.error("Failed to delete resource {}", toRemoveResources.toString()); } } } @@ -438,7 +438,7 @@ public class CubeManager implements IRealizationProvider { public CubeInstance dropCube(String cubeName, boolean deleteDesc) throws IOException { try (AutoLock lock = cubeMapLock.lockForWrite()) { - logger.info("Dropping cube '" + cubeName + "'"); + logger.info("Dropping cube '{}'", cubeName); // load projects before remove cube from project // delete cube instance and cube desc @@ -872,7 +872,7 @@ public class CubeManager implements IRealizationProvider { "For cube " + cubeCopy + ", segment " + newSegCopy + " missing LastBuildJobID"); if (isReady(newSegCopy) == true) { - logger.warn("For cube " + cubeCopy + ", segment " + newSegCopy + " state should be NEW but is READY"); + logger.warn("For cube {}, segment {} state should be NEW but is READY", cubeCopy, newSegCopy); } List<CubeSegment> tobe = cubeCopy.calculateToBeSegments(newSegCopy); @@ -889,8 +889,7 @@ public class CubeManager implements IRealizationProvider { toRemoveSegs.add(segment); } - logger.info("Promoting cube " + cubeCopy + ", new segment " + newSegCopy + ", to remove segments " - + toRemoveSegs); + logger.info("Promoting cube {}, new segment {}, to remove segments {}", cubeCopy, newSegCopy, toRemoveSegs); CubeUpdate update = new CubeUpdate(cubeCopy); update.setToRemoveSegs(toRemoveSegs.toArray(new CubeSegment[toRemoveSegs.size()])) @@ -941,8 +940,8 @@ public class CubeManager implements IRealizationProvider { seg.setStatus(SegmentStatusEnum.READY); } - logger.info("Promoting cube " + cubeCopy + ", new segments " + Arrays.toString(optSegCopy) - + ", to remove segments " + originalSegments); + logger.info("Promoting cube {}, new segments {}, to remove segments {}", + cubeCopy, Arrays.toString(optSegCopy), originalSegments); CubeUpdate update = new CubeUpdate(cubeCopy); update.setToRemoveSegs(originalSegments) // @@ -975,7 +974,7 @@ public class CubeManager implements IRealizationProvider { DataModelDesc modelDesc = cube.getModel(); Preconditions.checkNotNull(cube); final List<CubeSegment> segments = cube.getSegments(); - logger.info("totally " + segments.size() + " cubeSegments"); + logger.info("totally {} cubeSegments", segments.size()); if (segments.size() == 0) { return holes; } diff --git a/source-hive/src/main/java/org/apache/kylin/source/jdbc/SqlUtil.java b/source-hive/src/main/java/org/apache/kylin/source/jdbc/SqlUtil.java index f86fd7e..b9cb391 100644 --- a/source-hive/src/main/java/org/apache/kylin/source/jdbc/SqlUtil.java +++ b/source-hive/src/main/java/org/apache/kylin/source/jdbc/SqlUtil.java @@ -33,6 +33,10 @@ import org.slf4j.LoggerFactory; public class SqlUtil { private static final Logger logger = LoggerFactory.getLogger(SqlUtil.class); + private SqlUtil() { + throw new IllegalStateException("Class CheckUtil is an utility class !"); + } + public static void closeResources(Connection con, Statement statement) { try { if (statement != null && !statement.isClosed()) { diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java index a7d2cb6..b746501 100644 --- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java +++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/util/ZookeeperDistributedLock.java @@ -140,22 +140,22 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock { public boolean lock(String lockPath) { lockPath = norm(lockPath); - logger.debug(client + " trying to lock " + lockPath); + logger.debug("{} trying to lock {}", client, lockPath); try { curator.create().creatingParentsIfNeeded().withMode(CreateMode.EPHEMERAL).forPath(lockPath, clientBytes); } catch (KeeperException.NodeExistsException ex) { - logger.debug(client + " see " + lockPath + " is already locked"); + logger.debug("{} see {} is already locked", client, lockPath); } catch (Exception ex) { throw new RuntimeException("Error while " + client + " trying to lock " + lockPath, ex); } String lockOwner = peekLock(lockPath); if (client.equals(lockOwner)) { - logger.info(client + " acquired lock at " + lockPath); + logger.info("{} acquired lock at {}", client, lockPath); return true; } else { - logger.debug(client + " failed to acquire lock at " + lockPath + ", which is held by " + lockOwner); + logger.debug("{} failed to acquire lock at {}, which is held by {}", client, lockPath, lockOwner); return false; } } @@ -170,7 +170,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock { if (timeout <= 0) timeout = Long.MAX_VALUE; - logger.debug(client + " will wait for lock path " + lockPath); + logger.debug("{} will wait for lock path {}", client, lockPath); long waitStart = System.currentTimeMillis(); Random random = new Random(); long sleep = 10 * 1000; // 10 seconds @@ -183,7 +183,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock { } if (lock(lockPath)) { - logger.debug(client + " waited " + (System.currentTimeMillis() - waitStart) + " ms for lock path " + lockPath); + logger.debug("{} waited {} ms for lock path {}", client, (System.currentTimeMillis() - waitStart), lockPath); return true; } } @@ -220,7 +220,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock { public void unlock(String lockPath) { lockPath = norm(lockPath); - logger.debug(client + " trying to unlock " + lockPath); + logger.debug("{} trying to unlock {}", client, lockPath); String owner = peekLock(lockPath); if (owner == null) @@ -231,7 +231,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock { try { curator.delete().guaranteed().deletingChildrenIfNeeded().forPath(lockPath); - logger.info(client + " released lock at " + lockPath); + logger.info("{} released lock at {}", client, lockPath); } catch (Exception ex) { throw new RuntimeException("Error while " + client + " trying to unlock " + lockPath, ex); @@ -245,7 +245,7 @@ public class ZookeeperDistributedLock implements DistributedLock, JobLock { try { curator.delete().guaranteed().deletingChildrenIfNeeded().forPath(lockPathRoot); - logger.info(client + " purged all locks under " + lockPathRoot); + logger.info("{} purged all locks under {}", client, lockPathRoot); } catch (Exception ex) { throw new RuntimeException("Error while " + client + " trying to purge " + lockPathRoot, ex);