[ https://issues.apache.org/jira/browse/KYLIN-3597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16630540#comment-16630540 ]
ASF GitHub Bot commented on KYLIN-3597: --------------------------------------- shaofengshi closed pull request #263: KYLIN-3597 Fix some issues reported by SonarCloud URL: https://github.com/apache/kylin/pull/263 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/util/CheckUtil.java b/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java index 0f75ff2474..fced09d711 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 @@ 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 static int randomAvailablePort(int minPort, int maxPort) { * @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 0dc825f165..1cacd0dc9b 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 @@ static CubeManager newInstance(KylinConfig config) throws IOException { 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 CubeInstance getCubeByUuid(String uuid) { 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 CubeInstance createCube(String cubeName, String projectName, CubeDesc des 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 @@ private CubeInstance updateCubeWithRetry(CubeUpdate update, int retry) throws IO 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 @@ private CubeInstance updateCubeWithRetry(CubeUpdate update, int retry) throws IO 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 @@ private CubeInstance updateCubeWithRetry(CubeUpdate update, int retry) throws IO 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 @@ private CubeInstance updateCubeWithRetry(CubeUpdate update, int retry) throws IO 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 void removeCubeLocal(String cubeName) { 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 void promoteNewlyBuiltSegments(CubeInstance cube, CubeSegment newSegCopy) "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 void promoteNewlyBuiltSegments(CubeInstance cube, CubeSegment newSegCopy) 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 void promoteCheckpointOptimizeSegments(CubeInstance cube, Map<Long, Long> 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 @@ private void validateNewSegments(CubeInstance cube, CubeSegment newSegments) { 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 f86fd7e71d..b9cb3918fe 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 @@ 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 a7d2cb61d6..b746501539 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 String getClient() { 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 boolean lock(String lockPath, long timeout) { 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 boolean lock(String lockPath, long timeout) { } 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 boolean isLockedByMe(String lockPath) { 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 void unlock(String lockPath) { 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 void purgeLocks(String lockPathRoot) { 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); ---------------------------------------------------------------- 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 > -- This message was sent by Atlassian JIRA (v7.6.3#76005)