Fix region migration retries and peer recreation#17512
Open
Pengzna wants to merge 1 commit intoapache:masterfrom
Open
Fix region migration retries and peer recreation#17512Pengzna wants to merge 1 commit intoapache:masterfrom
Pengzna wants to merge 1 commit intoapache:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses stability issues in consensus/region-migration workflows, focusing on bounding retry behavior, preventing duplicate peer lists during migration, and fixing a writer-allocation race in the IoTConsensusV2 pipe receiver.
Changes:
- Bound
DELETE_OLD_REGION_PEERRPC retries and avoid adding duplicate destination peers during IoT/IoTV2 region migration. - Make
IoTConsensus#createLocalPeertolerate an already-existing consensus directory after restart recovery (and validate it’s a directory). - Improve IoTConsensusV2 receiver tsfile-writer pooling by reserving writers more deterministically and waking waiters when writers are returned.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/receiver/protocol/iotconsensusv2/IoTConsensusV2Receiver.java | Adjust tsfile-writer borrowing logic and signal waiters on writer release to mitigate writer-pool races. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensus.java | Allow createLocalPeer to proceed when the consensus dir already exists; validate path type. |
| iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java | Avoid duplicate peers in create-peer requests; bound DELETE_OLD_REGION_PEER RPC retries per attempt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1017
to
+1031
| // We should synchronously find the idle writer to avoid concurrency issues. | ||
| lock.lock(); | ||
| try { | ||
| // We need to check tsFileWriter.isPresent() here. Since there may be both retry-sent | ||
| // tsfile | ||
| // events and real-time-sent tsfile events, causing the receiver's tsFileWriter load to | ||
| // exceed IOTDB_CONFIG.getIoTConsensusV2PipelineSize(). | ||
| while (!tsFileWriter.isPresent()) { | ||
| while (true) { | ||
| tsFileWriter = | ||
| iotConsensusV2TsFileWriterPool.stream() | ||
| .filter( | ||
| item -> | ||
| item.isUsed() | ||
| && Objects.equals( | ||
| commitId, item.getCommitIdOfCorrespondingHolderEvent())) | ||
| .findFirst(); | ||
| if (tsFileWriter.isPresent()) { | ||
| break; | ||
| } |
Comment on lines
+1339
to
+1345
| tsFileWriter.returnSelf(consensusPipeName); | ||
| iotConsensusV2TsFileWriterPool.lock.lock(); | ||
| try { | ||
| iotConsensusV2TsFileWriterPool.condition.signalAll(); | ||
| } finally { | ||
| iotConsensusV2TsFileWriterPool.lock.unlock(); | ||
| } |
1748fa3 to
ee1d27e
Compare
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
master#17495Verification
mvn -pl iotdb-core/confignode,iotdb-core/consensus -am -DskipTests -Drat.skip=true -ntp compile