You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by xi...@apache.org on 2023/06/30 07:01:29 UTC
[iotdb] branch master updated: Fix some code smells and bugs in tools.settle module (#10396)
This is an automated email from the ASF dual-hosted git repository.
xingtanzjr 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 da0aea2793c Fix some code smells and bugs in tools.settle module (#10396)
da0aea2793c is described below
commit da0aea2793c1d046a075082c1b1c80d4c5bbd4d8
Author: shuwenwei <55...@users.noreply.github.com>
AuthorDate: Fri Jun 30 15:01:23 2023 +0800
Fix some code smells and bugs in tools.settle module (#10396)
---
.../db/tools/settle/TsFileAndModSettleTool.java | 78 +++++++++++++---------
.../tools/settle/TsFileSettleByCompactionTool.java | 35 ++++++----
2 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileAndModSettleTool.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileAndModSettleTool.java
index 1dfca1317c3..c6dc533b337 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileAndModSettleTool.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileAndModSettleTool.java
@@ -36,6 +36,7 @@ import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
+import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -49,10 +50,13 @@ import static org.apache.iotdb.tsfile.common.constant.TsFileConstant.TSFILE_SUFF
* Offline Settle tool, which is used to settle TsFile and its corresponding mods file to a new
* TsFile.
*/
+@SuppressWarnings("squid:S6548")
public class TsFileAndModSettleTool {
private static final Logger logger = LoggerFactory.getLogger(TsFileAndModSettleTool.class);
// TsFilePath -> SettleCheckStatus
+ @SuppressWarnings("squid:S1104")
public Map<String, Integer> recoverSettleFileMap = new HashMap<>();
+
private static final TsFileAndModSettleTool tsFileAndModSettleTool = new TsFileAndModSettleTool();
private TsFileAndModSettleTool() {}
@@ -72,28 +76,26 @@ public class TsFileAndModSettleTool {
}
List<File> tsFiles = checkArgs(args);
for (File file : tsFiles) {
- if (!oldTsFileResources.containsKey(file.getName())) {
- if (new File(file + TsFileResource.RESOURCE_SUFFIX).exists()) {
- TsFileResource resource = new TsFileResource(file);
- resource.setStatus(TsFileResourceStatus.NORMAL);
- oldTsFileResources.put(file.getName(), resource);
- }
+ if (!oldTsFileResources.containsKey(file.getName())
+ && (new File(file + TsFileResource.RESOURCE_SUFFIX).exists())) {
+ TsFileResource resource = new TsFileResource(file);
+ resource.setStatus(TsFileResourceStatus.NORMAL);
+ oldTsFileResources.put(file.getName(), resource);
}
}
- System.out.println(
- "Totally find "
- + oldTsFileResources.size()
- + " tsFiles to be settled, including "
- + getInstance().recoverSettleFileMap.size()
- + " tsFiles to be recovered.");
+ logger.info(
+ "Totally find {} tsFiles to be settled, including {} tsFiles to be recovered.",
+ oldTsFileResources.size(),
+ getInstance().recoverSettleFileMap.size());
settleTsFilesAndMods(oldTsFileResources);
}
+ @SuppressWarnings({"squid:S1854", "squid:S1481"})
public static List<File> checkArgs(String[] args) {
String filePath = "test.tsfile";
List<File> files = new ArrayList<>();
if (args.length == 0) {
- return null;
+ return Collections.emptyList();
} else {
for (String arg : args) {
if (arg.endsWith(TSFILE_SUFFIX)) { // it's a file
@@ -150,7 +152,7 @@ public class TsFileAndModSettleTool {
List<TsFileResource> settledTsFileResources = new ArrayList<>();
try {
TsFileAndModSettleTool tsFileAndModSettleTool = TsFileAndModSettleTool.getInstance();
- System.out.println("Start settling for tsFile : " + resourceToBeSettled.getTsFilePath());
+ logger.info("Start settling for tsFile : {}", resourceToBeSettled.getTsFilePath());
if (tsFileAndModSettleTool.isSettledFileGenerated(resourceToBeSettled)) {
settledTsFileResources = tsFileAndModSettleTool.findSettledFile(resourceToBeSettled);
newTsFileResources.put(resourceToBeSettled.getTsFile().getName(), settledTsFileResources);
@@ -175,23 +177,21 @@ public class TsFileAndModSettleTool {
resourceToBeSettled.getTsFilePath()
+ SettleLog.COMMA_SEPERATOR
+ SettleCheckStatus.SETTLE_SUCCESS);
- System.out.println(
- "Finish settling successfully for tsFile : " + resourceToBeSettled.getTsFilePath());
+ logger.info(
+ "Finish settling successfully for tsFile : {}", resourceToBeSettled.getTsFilePath());
successCount++;
} catch (Exception e) {
- System.out.println(
- "Meet error while settling the tsFile : " + resourceToBeSettled.getTsFilePath());
+ logger.info(
+ "Meet error while settling the tsFile : {}", resourceToBeSettled.getTsFilePath());
e.printStackTrace();
}
}
if (resourcesToBeSettled.size() == successCount) {
SettleLog.closeLogWriter();
- System.out.println("Finish settling all tsfiles Successfully!");
+ logger.info("Finish settling all tsfiles Successfully!");
} else {
- System.out.println(
- "Finish Settling, "
- + (resourcesToBeSettled.size() - successCount)
- + " tsfiles meet errors.");
+ logger.info(
+ "Finish Settling, {} tsfiles meet errors.", (resourcesToBeSettled.size() - successCount));
}
}
@@ -238,7 +238,14 @@ public class TsFileAndModSettleTool {
logger.error(
"meet error when reading settle log, log path:{}", SettleLog.getSettleLogPath(), e);
} finally {
- FSFactoryProducer.getFSFactory().getFile(SettleLog.getSettleLogPath()).delete();
+ File f = FSFactoryProducer.getFSFactory().getFile(SettleLog.getSettleLogPath());
+ try {
+ if (f.exists()) {
+ Files.delete(f.toPath());
+ }
+ } catch (IOException e) {
+ logger.error("failed to delete settle log, log path:{}", SettleLog.getSettleLogPath());
+ }
}
}
}
@@ -251,7 +258,11 @@ public class TsFileAndModSettleTool {
== SettleCheckStatus.AFTER_SETTLE_FILE.getCheckStatus();
}
- /** when the new file is settled , we need to find and deserialize it. */
+ /**
+ * when the new file is settled , we need to find and deserialize it.
+ *
+ * @throws IOException if io errors occurred
+ */
public List<TsFileResource> findSettledFile(TsFileResource resourceToBeSettled)
throws IOException {
List<TsFileResource> settledTsFileResources = new ArrayList<>();
@@ -292,8 +303,9 @@ public class TsFileAndModSettleTool {
* @param oldTsFileResource
* @param newTsFileResources if the old TsFile has not any deletions or all the data in which has
* been deleted or its modFile does not exist, then this size will be 0.
- * @throws IOException
+ * @throws IOException if io errors occurred
*/
+ @SuppressWarnings("squid:S3776")
public static void moveNewTsFile(
TsFileResource oldTsFileResource, List<TsFileResource> newTsFileResources)
throws IOException {
@@ -305,19 +317,19 @@ public class TsFileAndModSettleTool {
oldTsFileResource.getTsFile().getParent()
+ File.separator
+ oldTsFileResource.getTimePartition());
- if (newTsFileResources.size() == 0) { // if the oldTsFile has no mods, it should not be deleted.
+ if (newTsFileResources.isEmpty()) { // if the oldTsFile has no mods, it should not be deleted.
if (oldTsFileResource.isDeleted()) {
oldTsFileResource.remove();
}
- if (newPartitionDir.exists()) {
- newPartitionDir.delete();
- }
+ Files.delete(newPartitionDir.toPath());
return;
}
FSFactory fsFactory = FSFactoryProducer.getFSFactory();
File oldTsFile = oldTsFileResource.getTsFile();
boolean isOldFileExisted = oldTsFile.exists();
- oldTsFile.delete();
+ if (isOldFileExisted) {
+ Files.delete(oldTsFile.toPath());
+ }
for (TsFileResource newTsFileResource : newTsFileResources) {
newPartitionDir =
new File(
@@ -345,12 +357,12 @@ public class TsFileAndModSettleTool {
fsFactory.getFile(
newPartitionDir, newTsFile.getName() + TsFileResource.RESOURCE_SUFFIX);
if (tmpResourceFile.exists()) {
- tmpResourceFile.delete();
+ Files.delete(tmpResourceFile.toPath());
}
}
// if the newPartition folder is empty, then it will be deleted
if (newPartitionDir.exists()) {
- newPartitionDir.delete();
+ Files.delete(newPartitionDir.toPath());
}
}
}
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileSettleByCompactionTool.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileSettleByCompactionTool.java
index 7cc41020870..07d5604e5a0 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileSettleByCompactionTool.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/tools/settle/TsFileSettleByCompactionTool.java
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+
package org.apache.iotdb.db.tools.settle;
import org.apache.iotdb.common.rpc.thrift.TSStatus;
@@ -35,11 +36,15 @@ import org.apache.thrift.protocol.TProtocol;
import org.apache.thrift.transport.TSocket;
import org.apache.thrift.transport.TTransport;
import org.apache.thrift.transport.layered.TFramedTransport;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.Arrays;
public class TsFileSettleByCompactionTool {
+ private static final Logger logger = LoggerFactory.getLogger(TsFileSettleByCompactionTool.class);
+
private static final String HOST_ARGS = "h";
private static final String HOST_NAME = "host";
@@ -61,7 +66,7 @@ public class TsFileSettleByCompactionTool {
try {
commandLine = parser.parse(commandLineOptions, args);
} catch (ParseException e) {
- System.out.println("Parse command line args failed: " + e.getMessage());
+ logger.info("Parse command line args failed: {}", e.getMessage());
return;
}
@@ -70,19 +75,21 @@ public class TsFileSettleByCompactionTool {
int port = Integer.parseInt(portValue);
filePaths = commandLine.getOptionValues(FILE_PATH_ARGS);
- TTransport transport = new TFramedTransport(new TSocket(hostValue, port));
- transport.open();
- TProtocol protocol = new TBinaryProtocol(transport);
- IDataNodeRPCService.Client.Factory clientFactory = new IDataNodeRPCService.Client.Factory();
- IDataNodeRPCService.Client client = clientFactory.getClient(protocol);
-
- TSettleReq tSettleReq = new TSettleReq();
- tSettleReq.setPaths(Arrays.asList(filePaths));
- TSStatus result = client.settle(tSettleReq);
- if (result.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- System.out.println("Add Settle Compaction Task Successfully");
- } else {
- System.out.println("Add settle compaction task failed with status code: " + result);
+ TProtocol protocol;
+ try (TTransport transport = new TFramedTransport(new TSocket(hostValue, port))) {
+ transport.open();
+ protocol = new TBinaryProtocol(transport);
+ IDataNodeRPCService.Client.Factory clientFactory = new IDataNodeRPCService.Client.Factory();
+ IDataNodeRPCService.Client client = clientFactory.getClient(protocol);
+
+ TSettleReq settleReq = new TSettleReq();
+ settleReq.setPaths(Arrays.asList(filePaths));
+ TSStatus result = client.settle(settleReq);
+ if (result.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ logger.info("Add Settle Compaction Task Successfully");
+ } else {
+ logger.info("Add settle compaction task failed with status code: {}", result);
+ }
}
}