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/02 04:09:59 UTC
[iotdb] branch master updated: [IOTDB-5284] Fix some iotdb-confignode code smells (#8656)
This is an automated email from the ASF dual-hosted git repository.
tanxinyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git
The following commit(s) were added to refs/heads/master by this push:
new 8eb7e24eb2 [IOTDB-5284] Fix some iotdb-confignode code smells (#8656)
8eb7e24eb2 is described below
commit 8eb7e24eb2c94af20c40d2a79e114f367437f0da
Author: BUAAserein <65...@users.noreply.github.com>
AuthorDate: Mon Jan 2 12:09:54 2023 +0800
[IOTDB-5284] Fix some iotdb-confignode code smells (#8656)
* fix some code smells
* fix some code smells
---
.../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 | 6 ++---
.../persistence/schema/TemplateTable.java | 6 ++---
.../confignode/procedure/InternalProcedure.java | 2 +-
.../procedure/env/DataNodeRemoveHandler.java | 2 +-
.../impl/schema/DeleteStorageGroupProcedure.java | 5 ++++-
11 files changed, 43 insertions(+), 38 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 98494e494e..3022dfac26 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,7 +95,7 @@ public class ClusterSchemaInfo implements SnapshotProcessor {
private final ReentrantReadWriteLock storageGroupReadWriteLock;
private final ConfigMTree mTree;
- private final String snapshotFileName = "cluster_schema.bin";
+ private final String SNAPSHOT_FILENAME = "cluster_schema.bin";
private final TemplateTable templateTable;
@@ -471,7 +471,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.",
@@ -511,7 +511,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