You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kylin.apache.org by GitBox <gi...@apache.org> on 2018/09/27 14:45:04 UTC
[GitHub] shaofengshi closed pull request #263: KYLIN-3597 Fix some issues
reported by SonarCloud
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:
users@infra.apache.org
With regards,
Apache Git Services