You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by ta...@apache.org on 2023/01/03 15:48:46 UTC

[iotdb] branch rel/1.0 updated: [To rel/1.0][IOTDB-5284] Fix some code smells (#8711)

This is an automated email from the ASF dual-hosted git repository.

tanxinyu pushed a commit to branch rel/1.0
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/rel/1.0 by this push:
     new 2880c61053 [To rel/1.0][IOTDB-5284] Fix some code smells (#8711)
2880c61053 is described below

commit 2880c61053c512b5bed4c2ad18ef76dbcbf3efb2
Author: BUAAserein <65...@users.noreply.github.com>
AuthorDate: Tue Jan 3 23:48:39 2023 +0800

    [To rel/1.0][IOTDB-5284] Fix some code smells (#8711)
    
    * fix some code smells
    
    * fix some code smells
    
    Co-authored-by: Potato <ta...@apache.org>
---
 .../consensus/response/DataNodeToStatusResp.java   |  4 +++-
 .../router/leader/MinCostFlowLeaderBalancer.java   | 26 +++++++++++-----------
 .../node/heartbeat/DataNodeHeartbeatCache.java     |  2 +-
 .../iotdb/confignode/persistence/TriggerInfo.java  |  6 ++---
 .../confignode/persistence/node/NodeInfo.java      | 16 ++++++-------
 .../persistence/partition/PartitionInfo.java       |  6 ++---
 .../persistence/schema/ClusterSchemaInfo.java      |  9 ++++----
 .../persistence/schema/TemplateTable.java          |  6 ++---
 .../confignode/procedure/InternalProcedure.java    |  2 +-
 .../procedure/env/DataNodeRemoveHandler.java       |  2 +-
 .../impl/schema/DeleteStorageGroupProcedure.java   |  5 ++++-
 11 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/consensus/response/DataNodeToStatusResp.java b/confignode/src/main/java/org/apache/iotdb/confignode/consensus/response/DataNodeToStatusResp.java
index c16b03417f..17e265b376 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/consensus/response/DataNodeToStatusResp.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/consensus/response/DataNodeToStatusResp.java
@@ -32,7 +32,9 @@ public class DataNodeToStatusResp implements DataSet {
 
   private Map<TDataNodeLocation, TSStatus> dataNodeLocationToStatus;
 
-  public DataNodeToStatusResp() {}
+  public DataNodeToStatusResp() {
+    // empty constructor
+  }
 
   public TDataNodeRemoveResp convertToRpCDataNodeRemoveResp() {
     TDataNodeRemoveResp resp = new TDataNodeRemoveResp();
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/router/leader/MinCostFlowLeaderBalancer.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/router/leader/MinCostFlowLeaderBalancer.java
index 8e7fabe9b5..9b775d71cd 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/router/leader/MinCostFlowLeaderBalancer.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/router/leader/MinCostFlowLeaderBalancer.java
@@ -48,11 +48,11 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
 
   /** Graph nodes */
   // Super source node
-  private static final int sNode = 0;
+  private static final int S_NODE = 0;
   // Super terminal node
-  private static final int tNode = 1;
+  private static final int T_NODE = 1;
   // Maximum index of graph nodes
-  private int maxNode = tNode + 1;
+  private int maxNode = T_NODE + 1;
   // Map<RegionGroupId, rNode>
   private final Map<TConsensusGroupId, Integer> rNodeMap;
   // Map<DataNodeId, dNode>
@@ -124,7 +124,7 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
     this.isNodeVisited = null;
     this.nodeMinimumCost = null;
 
-    this.maxNode = tNode + 1;
+    this.maxNode = T_NODE + 1;
     this.maxEdge = 0;
   }
 
@@ -154,7 +154,7 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
     /* Construct edges: sNode -> rNodes */
     for (int rNode : rNodeMap.values()) {
       // Cost: 0
-      addAdjacentEdges(sNode, rNode, 1, 0);
+      addAdjacentEdges(S_NODE, rNode, 1, 0);
     }
 
     /* Construct edges: rNodes -> dNodes */
@@ -203,7 +203,7 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
       for (int extraEdge = 1; extraEdge <= maxLeaderCount; extraEdge++) {
         // Cost: x^2 for the x-th edge at the current dNode.
         // Thus, the leader distribution will be as balance as possible.
-        addAdjacentEdges(dNode, tNode, 1, extraEdge * extraEdge);
+        addAdjacentEdges(dNode, T_NODE, 1, extraEdge * extraEdge);
       }
     }
   }
@@ -232,9 +232,9 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
     Arrays.fill(nodeMinimumCost, INFINITY);
 
     Queue<Integer> queue = new LinkedList<>();
-    nodeMinimumCost[sNode] = 0;
-    isNodeVisited[sNode] = true;
-    queue.offer(sNode);
+    nodeMinimumCost[S_NODE] = 0;
+    isNodeVisited[S_NODE] = true;
+    queue.offer(S_NODE);
     while (!queue.isEmpty()) {
       int currentNode = queue.poll();
       isNodeVisited[currentNode] = false;
@@ -253,12 +253,12 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
       }
     }
 
-    return nodeMinimumCost[tNode] < INFINITY;
+    return nodeMinimumCost[T_NODE] < INFINITY;
   }
 
   /** Do augmentation by dfs algorithm */
   private int dfsAugmentation(int currentNode, int inputFlow) {
-    if (currentNode == tNode || inputFlow == 0) {
+    if (currentNode == T_NODE || inputFlow == 0) {
       return inputFlow;
     }
 
@@ -300,7 +300,7 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
     while (bellmanFordCheck()) {
       int currentFlow;
       System.arraycopy(nodeHeadEdge, 0, nodeCurrentEdge, 0, maxNode);
-      while ((currentFlow = dfsAugmentation(sNode, INFINITY)) > 0) {
+      while ((currentFlow = dfsAugmentation(S_NODE, INFINITY)) > 0) {
         maximumFlow += currentFlow;
       }
     }
@@ -317,7 +317,7 @@ public class MinCostFlowLeaderBalancer implements ILeaderBalancer {
               currentEdge >= 0;
               currentEdge = minCostFlowEdges.get(currentEdge).nextEdge) {
             MinCostFlowEdge edge = minCostFlowEdges.get(currentEdge);
-            if (edge.destNode != sNode && edge.capacity == 0) {
+            if (edge.destNode != S_NODE && edge.capacity == 0) {
               matchLeader = true;
               result.put(regionGroupId, dNodeReflect.get(edge.destNode));
             }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/DataNodeHeartbeatCache.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/DataNodeHeartbeatCache.java
index d35dce9d4e..3965fc6566 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/DataNodeHeartbeatCache.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/DataNodeHeartbeatCache.java
@@ -36,7 +36,7 @@ public class DataNodeHeartbeatCache extends BaseNodeCache {
   protected void updateCurrentStatistics() {
     NodeHeartbeatSample lastSample = null;
     synchronized (slidingWindow) {
-      if (slidingWindow.size() > 0) {
+      if (!slidingWindow.isEmpty()) {
         lastSample = slidingWindow.getLast();
       }
     }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/TriggerInfo.java b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/TriggerInfo.java
index ef6353c5e5..54fa77e2da 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/TriggerInfo.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/TriggerInfo.java
@@ -80,7 +80,7 @@ public class TriggerInfo implements SnapshotProcessor {
 
   private final ReentrantLock triggerTableLock = new ReentrantLock();
 
-  private static final String snapshotFileName = "trigger_info.bin";
+  private static final String SNAPSHOT_FILENAME = "trigger_info.bin";
 
   public TriggerInfo() throws IOException {
     triggerTable = new TriggerTable();
@@ -241,7 +241,7 @@ public class TriggerInfo implements SnapshotProcessor {
 
   @Override
   public boolean processTakeSnapshot(File snapshotDir) throws TException, IOException {
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (snapshotFile.exists() && snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to take snapshot, because snapshot file [{}] is already exist.",
@@ -276,7 +276,7 @@ public class TriggerInfo implements SnapshotProcessor {
 
   @Override
   public void processLoadSnapshot(File snapshotDir) throws TException, IOException {
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (!snapshotFile.exists() || !snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to load snapshot,snapshot file [{}] is not exist.",
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/node/NodeInfo.java b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/node/NodeInfo.java
index bcfe13d61a..67790476ff 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/node/NodeInfo.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/node/NodeInfo.java
@@ -69,7 +69,7 @@ public class NodeInfo implements SnapshotProcessor {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(NodeInfo.class);
 
-  private static final int minimumDataNode =
+  private static final int MINIMUM_DATANODE =
       Math.max(
           ConfigNodeDescriptor.getInstance().getConf().getSchemaReplicationFactor(),
           ConfigNodeDescriptor.getInstance().getConf().getDataReplicationFactor());
@@ -83,7 +83,7 @@ public class NodeInfo implements SnapshotProcessor {
   private final AtomicInteger nextNodeId = new AtomicInteger(-1);
   private final Map<Integer, TDataNodeConfiguration> registeredDataNodes;
 
-  private final String snapshotFileName = "node_info.bin";
+  private static final String SNAPSHOT_FILENAME = "node_info.bin";
 
   public NodeInfo() {
     this.configNodeInfoReadWriteLock = new ReentrantReadWriteLock();
@@ -116,12 +116,12 @@ public class NodeInfo implements SnapshotProcessor {
       registeredDataNodes.put(info.getLocation().getDataNodeId(), info);
 
       result = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
-      if (nextNodeId.get() < minimumDataNode) {
+      if (nextNodeId.get() < MINIMUM_DATANODE) {
         result.setMessage(
             String.format(
                 "To enable IoTDB-Cluster's data service, please register %d more IoTDB-DataNode",
-                minimumDataNode - nextNodeId.get()));
-      } else if (nextNodeId.get() == minimumDataNode) {
+                MINIMUM_DATANODE - nextNodeId.get()));
+      } else if (nextNodeId.get() == MINIMUM_DATANODE) {
         result.setMessage("IoTDB-Cluster could provide data service, now enjoy yourself!");
       }
     } finally {
@@ -353,7 +353,7 @@ public class NodeInfo implements SnapshotProcessor {
 
   @Override
   public boolean processTakeSnapshot(File snapshotDir) throws IOException, TException {
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (snapshotFile.exists() && snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to take snapshot, because snapshot file [{}] is already exist.",
@@ -416,7 +416,7 @@ public class NodeInfo implements SnapshotProcessor {
   @Override
   public void processLoadSnapshot(File snapshotDir) throws IOException, TException {
 
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (!snapshotFile.exists() || !snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to load snapshot,snapshot file [{}] is not exist.",
@@ -470,7 +470,7 @@ public class NodeInfo implements SnapshotProcessor {
   }
 
   public static int getMinimumDataNode() {
-    return minimumDataNode;
+    return MINIMUM_DATANODE;
   }
 
   public void clear() {
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java
index ffedfa87ad..b8015707cb 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java
@@ -111,7 +111,7 @@ public class PartitionInfo implements SnapshotProcessor {
   // For RegionReplicas' asynchronous management
   private final List<RegionMaintainTask> regionMaintainTaskList;
 
-  private static final String snapshotFileName = "partition_info.bin";
+  private static final String SNAPSHOT_FILENAME = "partition_info.bin";
 
   public PartitionInfo() {
     this.nextRegionGroupId = new AtomicInteger(-1);
@@ -704,7 +704,7 @@ public class PartitionInfo implements SnapshotProcessor {
   @Override
   public boolean processTakeSnapshot(File snapshotDir) throws TException, IOException {
 
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (snapshotFile.exists() && snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to take snapshot, because snapshot file [{}] is already exist.",
@@ -757,7 +757,7 @@ public class PartitionInfo implements SnapshotProcessor {
 
   public void processLoadSnapshot(File snapshotDir) throws TException, IOException {
 
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (!snapshotFile.exists() || !snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to load snapshot,snapshot file [{}] is not exist.",
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java
index 3f70f71bc2..42a11a6cd3 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java
@@ -95,9 +95,8 @@ public class ClusterSchemaInfo implements SnapshotProcessor {
   private final ReentrantReadWriteLock storageGroupReadWriteLock;
   private final ConfigMTree mTree;
 
-  private static final String snapshotFileName = "cluster_schema.bin";
-
-  private final String ERROR_NAME = "Error StorageGroup name";
+  private static final String SNAPSHOT_FILENAME = "cluster_schema.bin";
+  private static final String ERROR_NAME = "Error StorageGroup name";
 
   private final TemplateTable templateTable;
 
@@ -465,7 +464,7 @@ public class ClusterSchemaInfo implements SnapshotProcessor {
   }
 
   public boolean processMtreeTakeSnapshot(File snapshotDir) throws IOException {
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (snapshotFile.exists() && snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to take snapshot, because snapshot file [{}] is already exist.",
@@ -505,7 +504,7 @@ public class ClusterSchemaInfo implements SnapshotProcessor {
   }
 
   public void processMtreeLoadSnapshot(File snapshotDir) throws IOException {
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (!snapshotFile.exists() || !snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to load snapshot,snapshot file [{}] is not exist.",
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/TemplateTable.java b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/TemplateTable.java
index 8dca4ab431..9c30a19165 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/TemplateTable.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/TemplateTable.java
@@ -57,7 +57,7 @@ public class TemplateTable {
   private final Map<String, Template> templateMap = new ConcurrentHashMap<>();
   private final Map<Integer, Template> templateIdMap = new ConcurrentHashMap<>();
 
-  private static final String snapshotFileName = "template_info.bin";
+  private static final String SNAPSHOT_FILENAME = "template_info.bin";
 
   public TemplateTable() {
     templateReadWriteLock = new ReentrantReadWriteLock();
@@ -166,7 +166,7 @@ public class TemplateTable {
   }
 
   public boolean processTakeSnapshot(File snapshotDir) throws IOException {
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (snapshotFile.exists() && snapshotFile.isFile()) {
       LOGGER.error(
           "template failed to take snapshot, because snapshot file [{}] is already exist.",
@@ -197,7 +197,7 @@ public class TemplateTable {
   }
 
   public void processLoadSnapshot(File snapshotDir) throws IOException {
-    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    File snapshotFile = new File(snapshotDir, SNAPSHOT_FILENAME);
     if (!snapshotFile.exists() || !snapshotFile.isFile()) {
       LOGGER.error(
           "Failed to load snapshot,snapshot file [{}] is not exist.",
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/InternalProcedure.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/InternalProcedure.java
index a3f9a110b6..45908b3014 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/InternalProcedure.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/InternalProcedure.java
@@ -31,7 +31,7 @@ import java.nio.ByteBuffer;
  * @param <Env>
  */
 public abstract class InternalProcedure<Env> extends Procedure<Env> {
-  public InternalProcedure(long toMillis) {
+  protected InternalProcedure(long toMillis) {
     setTimeout(toMillis);
   }
 
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java
index bcc487858c..29cbe04500 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java
@@ -514,7 +514,7 @@ public class DataNodeRemoveHandler {
               "Failed to remove data node {} because it is not in running and the configuration of cluster is one replication",
               dataNodeLocation);
         }
-        if (removedDataNodes.size() == 0) {
+        if (removedDataNodes.isEmpty()) {
           status.setCode(TSStatusCode.NO_ENOUGH_DATANODE.getStatusCode());
           status.setMessage("Failed to remove all requested data nodes");
           return status;
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteStorageGroupProcedure.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteStorageGroupProcedure.java
index fddf085734..759843be8a 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteStorageGroupProcedure.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteStorageGroupProcedure.java
@@ -158,6 +158,8 @@ public class DeleteStorageGroupProcedure
         LOG.info("Rollback preDeleted:{}", deleteSgSchema.getName());
         env.preDelete(PreDeleteStorageGroupPlan.PreDeleteType.ROLLBACK, deleteSgSchema.getName());
         break;
+      default:
+        break;
     }
   }
 
@@ -167,8 +169,9 @@ public class DeleteStorageGroupProcedure
       case DELETE_PRE:
       case INVALIDATE_CACHE:
         return true;
+      default:
+        return false;
     }
-    return false;
   }
 
   @Override