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:07:46 UTC
[iotdb] branch master updated: Fix some iotdb-confignode code smells (#8647)
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 358be12ca7 Fix some iotdb-confignode code smells (#8647)
358be12ca7 is described below
commit 358be12ca728bf6ff704a000e8d8f8d36f95b4bf
Author: BUAAserein <65...@users.noreply.github.com>
AuthorDate: Mon Jan 2 12:07:40 2023 +0800
Fix some iotdb-confignode code smells (#8647)
---
.../handlers/rpc/AbstractAsyncRPCHandler.java | 2 +-
.../statemachine/ConfigNodeRegionStateMachine.java | 20 +++++++-------
.../confignode/exception/ConfigNodeException.java | 2 +-
.../confignode/manager/ClusterSchemaManager.java | 31 ++++++++++------------
.../confignode/manager/cq/CQScheduleTask.java | 26 +++++++++---------
.../region/CopySetRegionGroupAllocator.java | 4 +--
.../manager/node/heartbeat/BaseNodeCache.java | 2 +-
.../iotdb/confignode/persistence/cq/CQInfo.java | 6 ++---
8 files changed, 45 insertions(+), 48 deletions(-)
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/client/async/handlers/rpc/AbstractAsyncRPCHandler.java b/confignode/src/main/java/org/apache/iotdb/confignode/client/async/handlers/rpc/AbstractAsyncRPCHandler.java
index 7fe5d780f9..12624c9acb 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/client/async/handlers/rpc/AbstractAsyncRPCHandler.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/client/async/handlers/rpc/AbstractAsyncRPCHandler.java
@@ -60,7 +60,7 @@ public abstract class AbstractAsyncRPCHandler<T> implements AsyncMethodCallback<
protected final String formattedTargetLocation;
- public AbstractAsyncRPCHandler(
+ protected AbstractAsyncRPCHandler(
DataNodeRequestType requestType,
int requestId,
TDataNodeLocation targetDataNode,
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/consensus/statemachine/ConfigNodeRegionStateMachine.java b/confignode/src/main/java/org/apache/iotdb/confignode/consensus/statemachine/ConfigNodeRegionStateMachine.java
index dc3ac09144..74c5ed5784 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/consensus/statemachine/ConfigNodeRegionStateMachine.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/consensus/statemachine/ConfigNodeRegionStateMachine.java
@@ -68,11 +68,11 @@ public class ConfigNodeRegionStateMachine
private int startIndex;
private int endIndex;
- private static final String currentFileDir =
+ private static final String CURRENT_FILE_DIR =
CONF.getConsensusDir() + File.separator + "simple" + File.separator + "current";
- private static final String progressFilePath =
- currentFileDir + File.separator + "log_inprogress_";
- private static final String filePath = currentFileDir + File.separator + "log_";
+ private static final String PROGRESS_FILE_PATH =
+ CURRENT_FILE_DIR + File.separator + "log_inprogress_";
+ private static final String FILE_PATH = CURRENT_FILE_DIR + File.separator + "log_";
private static final long LOG_FILE_MAX_SIZE =
CONF.getConfigNodeSimpleConsensusLogSegmentSizeMax();
private final TEndPoint currentNodeTEndPoint;
@@ -264,7 +264,7 @@ public class ConfigNodeRegionStateMachine
if (logFile.length() > LOG_FILE_MAX_SIZE) {
try {
logWriter.force();
- File completedFilePath = new File(filePath + startIndex + "_" + endIndex);
+ File completedFilePath = new File(FILE_PATH + startIndex + "_" + endIndex);
Files.move(logFile.toPath(), completedFilePath.toPath(), StandardCopyOption.ATOMIC_MOVE);
} catch (IOException e) {
LOGGER.error("Can't force logWriter for ConfigNode SimpleConsensus mode", e);
@@ -298,7 +298,7 @@ public class ConfigNodeRegionStateMachine
buffer.position(buffer.limit());
logWriter.write(buffer);
endIndex = endIndex + 1;
- File tmpLogFile = new File(progressFilePath + endIndex);
+ File tmpLogFile = new File(PROGRESS_FILE_PATH + endIndex);
Files.move(logFile.toPath(), tmpLogFile.toPath(), StandardCopyOption.ATOMIC_MOVE);
logFile = tmpLogFile;
logWriter = new LogWriter(logFile, false);
@@ -309,9 +309,9 @@ public class ConfigNodeRegionStateMachine
}
private void initStandAloneConfigNode() {
- File dir = new File(currentFileDir);
+ File dir = new File(CURRENT_FILE_DIR);
dir.mkdirs();
- String[] list = new File(currentFileDir).list();
+ String[] list = new File(CURRENT_FILE_DIR).list();
if (list != null && list.length != 0) {
for (String logFileName : list) {
int tmp = Integer.parseInt(logFileName.substring(logFileName.lastIndexOf("_") + 1));
@@ -323,7 +323,7 @@ public class ConfigNodeRegionStateMachine
}
}
File logFile =
- SystemFileFactory.INSTANCE.getFile(currentFileDir + File.separator + logFileName);
+ SystemFileFactory.INSTANCE.getFile(CURRENT_FILE_DIR + File.separator + logFileName);
SingleFileLogReader logReader;
try {
logReader = new SingleFileLogReader(logFile);
@@ -354,7 +354,7 @@ public class ConfigNodeRegionStateMachine
}
private void createLogFile(int endIndex) {
- logFile = SystemFileFactory.INSTANCE.getFile(progressFilePath + endIndex);
+ logFile = SystemFileFactory.INSTANCE.getFile(PROGRESS_FILE_PATH + endIndex);
try {
logFile.createNewFile();
logWriter = new LogWriter(logFile, false);
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/exception/ConfigNodeException.java b/confignode/src/main/java/org/apache/iotdb/confignode/exception/ConfigNodeException.java
index 0c3ea33ddb..60b26d59e3 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/exception/ConfigNodeException.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/exception/ConfigNodeException.java
@@ -20,7 +20,7 @@ package org.apache.iotdb.confignode.exception;
public abstract class ConfigNodeException extends Exception {
- public ConfigNodeException(String message) {
+ protected ConfigNodeException(String message) {
super(message);
}
}
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/ClusterSchemaManager.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/ClusterSchemaManager.java
index 48fc65ae91..70341b82c2 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/ClusterSchemaManager.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/ClusterSchemaManager.java
@@ -366,9 +366,8 @@ public class ClusterSchemaManager {
// (createdStorageGroupNum * schemaReplicationFactor)
SCHEMA_REGION_PER_DATA_NODE
* dataNodeNum
- / (double)
- (storageGroupNum
- * storageGroupSchema.getSchemaReplicationFactor())),
+ / storageGroupNum
+ * storageGroupSchema.getSchemaReplicationFactor()),
allocatedSchemaRegionGroupCount));
LOGGER.info(
"[AdjustRegionGroupNum] The maximum number of SchemaRegionGroups for Database: {} is adjusted to: {}",
@@ -397,9 +396,8 @@ public class ClusterSchemaManager {
// (createdStorageGroupNum * dataReplicationFactor)
DATA_REGION_PER_PROCESSOR
* totalCpuCoreNum
- / (double)
- (storageGroupNum
- * storageGroupSchema.getDataReplicationFactor())),
+ / storageGroupNum
+ * storageGroupSchema.getDataReplicationFactor()),
allocatedDataRegionGroupCount));
LOGGER.info(
"[AdjustRegionGroupNum] The maximum number of DataRegionGroups for Database: {} is adjusted to: {}",
@@ -501,12 +499,11 @@ public class ClusterSchemaManager {
(TemplateInfoResp) getConsensusManager().read(getAllSchemaTemplatePlan).getDataset();
TGetAllTemplatesResp resp = new TGetAllTemplatesResp();
resp.setStatus(templateResp.getStatus());
- if (resp.getStatus().getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- if (templateResp.getTemplateList() != null) {
- List<ByteBuffer> list = new ArrayList<>();
- templateResp.getTemplateList().forEach(template -> list.add(template.serialize()));
- resp.setTemplateList(list);
- }
+ if (resp.getStatus().getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()
+ && templateResp.getTemplateList() != null) {
+ List<ByteBuffer> list = new ArrayList<>();
+ templateResp.getTemplateList().forEach(template -> list.add(template.serialize()));
+ resp.setTemplateList(list);
}
return resp;
}
@@ -517,11 +514,11 @@ public class ClusterSchemaManager {
TemplateInfoResp templateResp =
(TemplateInfoResp) getConsensusManager().read(getSchemaTemplatePlan).getDataset();
TGetTemplateResp resp = new TGetTemplateResp();
- if (templateResp.getStatus().getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- if (templateResp.getTemplateList() != null && !templateResp.getTemplateList().isEmpty()) {
- ByteBuffer byteBuffer = templateResp.getTemplateList().get(0).serialize();
- resp.setTemplate(byteBuffer);
- }
+ if (templateResp.getStatus().getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()
+ && templateResp.getTemplateList() != null
+ && !templateResp.getTemplateList().isEmpty()) {
+ ByteBuffer byteBuffer = templateResp.getTemplateList().get(0).serialize();
+ resp.setTemplate(byteBuffer);
}
resp.setStatus(templateResp.getStatus());
return resp;
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/cq/CQScheduleTask.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/cq/CQScheduleTask.java
index f60bc09080..b1f94f21a1 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/cq/CQScheduleTask.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/cq/CQScheduleTask.java
@@ -43,7 +43,7 @@ public class CQScheduleTask implements Runnable {
private static final Logger LOGGER = LoggerFactory.getLogger(CQScheduleTask.class);
- private static final long DEFAULT_RETRY_WAIT_TIME_IN_MS = 20 * 1_000;
+ private static final long DEFAULT_RETRY_WAIT_TIME_IN_MS = 20L * 1_000;
private final String cqId;
private final long everyInterval;
@@ -190,18 +190,6 @@ public class CQScheduleTask implements Runnable {
return configManager.getConsensusManager().isLeader() && !executor.isShutdown();
}
- private void updateExecutionTime() {
- if (timeoutPolicy == TimeoutPolicy.BLOCKED) {
- executionTime = executionTime + everyInterval;
- } else if (timeoutPolicy == TimeoutPolicy.DISCARD) {
- long now = System.currentTimeMillis();
- executionTime =
- executionTime + ((now - executionTime - 1) / everyInterval + 1) * everyInterval;
- } else {
- throw new IllegalArgumentException("Unknown TimeoutPolicy: " + timeoutPolicy);
- }
- }
-
private void submitSelf(long delay, TimeUnit unit) {
executor.schedule(this, delay, unit);
}
@@ -216,6 +204,18 @@ public class CQScheduleTask implements Runnable {
this.endTime = endTime;
}
+ private void updateExecutionTime() {
+ if (timeoutPolicy == TimeoutPolicy.BLOCKED) {
+ executionTime = executionTime + everyInterval;
+ } else if (timeoutPolicy == TimeoutPolicy.DISCARD) {
+ long now = System.currentTimeMillis();
+ executionTime =
+ executionTime + ((now - executionTime - 1) / everyInterval + 1) * everyInterval;
+ } else {
+ throw new IllegalArgumentException("Unknown TimeoutPolicy: " + timeoutPolicy);
+ }
+ }
+
@Override
public void onComplete(TSStatus response) {
if (response.code == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/region/CopySetRegionGroupAllocator.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/region/CopySetRegionGroupAllocator.java
index 13c21fcaf2..c801e2b353 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/region/CopySetRegionGroupAllocator.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/balancer/region/CopySetRegionGroupAllocator.java
@@ -38,7 +38,7 @@ import java.util.Set;
*/
public class CopySetRegionGroupAllocator implements IRegionGroupAllocator {
- private static final int maximumRandomNum = 10;
+ private static final int MAXIMUM_RANDOM_NUM = 10;
private int maxId = 0;
private int intersectionSize = 0;
@@ -62,7 +62,7 @@ public class CopySetRegionGroupAllocator implements IRegionGroupAllocator {
boolean accepted = false;
while (true) {
- for (int retry = 0; retry < maximumRandomNum; retry++) {
+ for (int retry = 0; retry < MAXIMUM_RANDOM_NUM; retry++) {
result = genWeightedRandomRegion(replicationFactor);
if (intersectionCheck(allocatedRegionGroups, result)) {
accepted = true;
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/BaseNodeCache.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/BaseNodeCache.java
index ca185f7039..24f9a9edb5 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/BaseNodeCache.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/node/heartbeat/BaseNodeCache.java
@@ -55,7 +55,7 @@ public abstract class BaseNodeCache {
synchronized (slidingWindow) {
// Only sequential heartbeats are accepted.
// And un-sequential heartbeats will be discarded.
- if (slidingWindow.size() == 0
+ if (slidingWindow.isEmpty()
|| slidingWindow.getLast().getSendTimestamp() < newHeartbeatSample.getSendTimestamp()) {
slidingWindow.add(newHeartbeatSample);
}
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/cq/CQInfo.java b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/cq/CQInfo.java
index 23f7d3eb2b..3c296765d1 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/persistence/cq/CQInfo.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/persistence/cq/CQInfo.java
@@ -57,7 +57,7 @@ public class CQInfo implements SnapshotProcessor {
private static final Logger LOGGER = LoggerFactory.getLogger(CQInfo.class);
- private static final String snapshotFileName = "cq_info.snapshot";
+ private static final String SNAPSHOT_FILENAME = "cq_info.snapshot";
private final Map<String, CQEntry> cqMap;
@@ -210,7 +210,7 @@ public class CQInfo 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 of CQInfo, because snapshot file [{}] is already exist.",
@@ -245,7 +245,7 @@ public class CQInfo 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 of CQInfo, snapshot file [{}] does not exist.",