Extend region migration speed limit to transfers and deletion#18118
Open
CRZbulabula wants to merge 4 commits into
Open
Extend region migration speed limit to transfers and deletion#18118CRZbulabula wants to merge 4 commits into
CRZbulabula wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #18118 +/- ##
============================================
+ Coverage 41.84% 41.90% +0.06%
Complexity 318 318
============================================
Files 5296 5302 +6
Lines 373593 374331 +738
Branches 48234 48319 +85
============================================
+ Hits 156313 156865 +552
- Misses 217280 217466 +186 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
CRZbulabula
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR broadens the existing region_migration_speed_limit_bytes_per_second throttling so it applies not only to region-migration transfer traffic (across IoTConsensus/IoTConsensusV2/Ratis paths), but also to region-related file deletions during cleanup, using a shared singleton rate limiter in node-commons.
Changes:
- Introduces a shared
RegionMigrationRateLimiterand wires it into IoTConsensus snapshot transfer, IoTConsensusV2 transfer requests/file-piece transfer, and Ratis install-snapshot chunk sending. - Adds rate-limited deletion helpers in
FileUtilsand updates DataRegion/SchemaRegion/WAL/consensus peer cleanup paths to use them. - Updates the config template description and adds unit tests for rate-limited deletion and transfer-size calculations.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/FileUtilsTest.java | Adds unit tests validating the new rate-limited deletion helpers accumulate expected byte/file counts. |
| iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/RegionMigrationRateLimiter.java | Adds the shared singleton rate limiter implementation backing both transfer and deletion throttling. |
| iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/FileUtils.java | Adds rate-limited deletion entry points and threads a LongConsumer through recursive deletion. |
| iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template | Updates the config description to reflect throttling applies to transfers and deletions. |
| iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/IoTConsensusV2RateLimiterTest.java | Adds unit tests for IoTConsensusV2 transfer body/batch size calculation. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java | Uses rate-limited deletion for region snapshot directory cleanup. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/allocation/FirstCreateStrategy.java | Uses rate-limited deletion when removing WAL node directories. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/allocation/ElasticStrategy.java | Uses rate-limited deletion when removing WAL node directories. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/DataRegion.java | Uses rate-limited deletion for region system/data/object folder cleanup. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/SchemaRegionUtils.java | Adds overload to accept a delete-rate-limiter callback and accounts file sizes before deletion. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/impl/SchemaRegionPBTreeImpl.java | Wires schema region folder deletion through the shared limiter. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/impl/SchemaRegionMemoryImpl.java | Wires schema region folder deletion through the shared limiter. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/IoTConsensusV2SyncSink.java | Rate-limits IoTConsensusV2 batch/transfer requests and file-piece reads before sending. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/IoTConsensusV2RateLimiter.java | Adds helper to compute transfer payload sizes and acquire shared limiter accordingly. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/handler/IoTConsensusV2TsFileInsertionEventHandler.java | Rate-limits TsFile piece transfer by acquired bytes before issuing async transfer. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/handler/IoTConsensusV2TabletInsertionEventHandler.java | Rate-limits tablet insertion transfer requests before sending. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/handler/IoTConsensusV2TabletBatchEventHandler.java | Rate-limits batch transfer requests before sending. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/handler/IoTConsensusV2DeleteEventHandler.java | Rate-limits delete-event transfer requests before sending. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java | Initializes the shared RegionMigrationRateLimiter from region_migration_speed_limit_bytes_per_second. |
| iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/ratis/RateLimitedGrpcLogAppenderTest.java | Adds unit tests for Ratis snapshot chunk size calculation and RPC type resolution. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/simple/SimpleConsensus.java | Uses rate-limited deletion for local peer directory cleanup. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java | Forces Ratis to use the custom rate-limited gRPC RPC type. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RateLimitedGrpcRpcType.java | Introduces a custom Ratis RpcType that returns a rate-limited gRPC factory. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RateLimitedGrpcLogAppender.java | Hooks limiter acquisition into install-snapshot chunk iteration. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RateLimitedGrpcFactory.java | Provides a gRPC factory that installs the rate-limited log appender. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/pipe/IoTConsensusV2.java | Uses rate-limited deletion for IoTConsensusV2 local peer directory cleanup. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/iot/snapshot/IoTConsensusRateLimiter.java | Switches IoTConsensus snapshot throttling to the shared RegionMigrationRateLimiter. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensus.java | Uses rate-limited deletion for IoTConsensus local peer directory cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+190
to
194
| 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)) { |
Contributor
Author
There was a problem hiding this comment.
Fixed in e84fe31. deleteDirectoryAndEmptyParent(...) now treats parentFolder == null as no parent to clean up, and I added testDeleteDirectoryAndEmptyParentWithRateLimiterAndNoParent to cover the relative-path/no-parent case.
jt2594838
reviewed
Jul 8, 2026
Comment on lines
+128
to
139
| 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()); | ||
| } |
Contributor
There was a problem hiding this comment.
Deleting a file only involves some file system metadata modification and inode reclaim, which will not introduce such io.
The rate-limiting step may slow down the migration significantly.
Contributor
Author
There was a problem hiding this comment.
Agreed, using the real file size for delete throttling is too conservative because removal is mostly metadata work. Fixed in e84fe31 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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
region_migration_speed_limit_bytes_per_secondto throttle region migration transfer across consensus protocols that have real transfer paths.Protocol coverage
Tests
mvn spotless:apply -pl iotdb-core/node-commons,iotdb-core/consensus,iotdb-core/datanodemvn test -pl iotdb-core/node-commons -am -Dtest=FileUtilsTest -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=falsemvn test -pl iotdb-core/consensus -am -Dtest=RateLimitedGrpcLogAppenderTest -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=falsemvn test -pl iotdb-core/datanode -am -Dtest=IoTConsensusV2RateLimiterTest -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=falsemvn compile -pl iotdb-core/consensus,iotdb-core/datanode -am -DskipTestsmvn test-compile -DskipTests -P with-zh-locale -pl iotdb-core/consensus,iotdb-core/datanode -amNote: full reactor
mvn test-compile -DskipTestsreaches the distribution module and fails to resolve existing 2.0.11-SNAPSHOT zip artifacts for iotdb-server, iotdb-cli, and iotdb-confignode. The changed modules compile successfully.