CRZbulabula commented on code in PR #18118:
URL: https://github.com/apache/iotdb/pull/18118#discussion_r3544568444
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/FileUtils.java:
##########
@@ -161,7 +179,16 @@ public static void deleteFileOrDirectoryWithRetry(File
file) {
}
public static void deleteDirectoryAndEmptyParent(File folder) {
- deleteFileOrDirectory(folder);
+ deleteDirectoryAndEmptyParent(folder, null);
+ }
+
+ public static void deleteDirectoryAndEmptyParentWithRateLimiter(
+ File folder, LongConsumer deleteRateLimiter) {
+ deleteDirectoryAndEmptyParent(folder, deleteRateLimiter);
+ }
+
+ private static void deleteDirectoryAndEmptyParent(File folder, LongConsumer
deleteRateLimiter) {
+ deleteFileOrDirectory(folder, false, deleteRateLimiter);
final File parentFolder = folder.getParentFile();
File[] files = parentFolder.listFiles();
if (parentFolder.isDirectory() && (files == null || files.length == 0)) {
Review Comment:
Fixed in e84fe31364. `deleteDirectoryAndEmptyParent(...)` now treats
`parentFolder == null` as no parent to clean up, and I added
`testDeleteDirectoryAndEmptyParentWithRateLimiterAndNoParent` to cover the
relative-path/no-parent case.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/FileUtils.java:
##########
@@ -111,13 +112,30 @@ public static void deleteFileOrDirectory(File file) {
}
public static void deleteFileOrDirectory(File file, boolean
quietForNoSuchFile) {
+ deleteFileOrDirectory(file, quietForNoSuchFile, null);
+ }
+
+ public static void deleteFileOrDirectoryWithRateLimiter(
+ File file, LongConsumer deleteRateLimiter) {
+ deleteFileOrDirectory(file, false, deleteRateLimiter);
+ }
+
+ public static void deleteFileOrDirectoryWithRateLimiter(
+ File file, boolean quietForNoSuchFile, LongConsumer deleteRateLimiter) {
+ deleteFileOrDirectory(file, quietForNoSuchFile, deleteRateLimiter);
+ }
+
+ private static void deleteFileOrDirectory(
+ File file, boolean quietForNoSuchFile, LongConsumer deleteRateLimiter) {
if (file.isDirectory()) {
File[] files = file.listFiles();
if (files != null) {
for (File subfile : files) {
- deleteFileOrDirectory(subfile, quietForNoSuchFile);
+ deleteFileOrDirectory(subfile, quietForNoSuchFile,
deleteRateLimiter);
}
}
+ } else if (deleteRateLimiter != null && file.isFile()) {
+ deleteRateLimiter.accept(file.length());
}
Review Comment:
Agreed, using the real file size for delete throttling is too conservative
because removal is mostly metadata work. Fixed in e84fe31364 by moving cleanup
to an independent `region_migration_file_remove_speed_limit_bytes_per_second`
limiter and charging fixed estimated remove costs instead: 64 KiB per regular
file and 4 KiB per directory. I also removed the IoTConsensusV2 transfer
limiter so regular consensus pipe replication is not throttled by the migration
transfer limit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
