Skip to content

Region migration related work#12293

Merged
OneSizeFitsQuorum merged 45 commits intoapache:masterfrom
liyuheng55555:Working/region-migration-0401
Apr 12, 2024
Merged

Region migration related work#12293
OneSizeFitsQuorum merged 45 commits intoapache:masterfrom
liyuheng55555:Working/region-migration-0401

Conversation

@liyuheng55555
Copy link
Collaborator

No description provided.

Copy link
Collaborator Author

@liyuheng55555 liyuheng55555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL~ @BUAAserein

Comment on lines +420 to +435
@Override
public List<ConsensusGroupId> getAllConsensusGroupIdsFromDisk() {
List<ConsensusGroupId> consensusGroupIds = new ArrayList<>();
try (DirectoryStream<Path> stream = Files.newDirectoryStream(storageDir.toPath())) {
for (Path path : stream) {
String[] items = path.getFileName().toString().split("_");
ConsensusGroupId consensusGroupId =
ConsensusGroupId.Factory.create(Integer.parseInt(items[0]), Integer.parseInt(items[1]));
consensusGroupIds.add(consensusGroupId);
}
} catch (IOException e) {
logger.error("Failed to get all consensus group ids from disk", e);
}
return consensusGroupIds;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BUAAserein Maybe write an UT for this function ? Test the cooperation between getAllConsensusGroupIdsFromDisk and IoTConsensus.buildPeerDir .

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is the same as IoTConsensus, so may no need to add UT

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is the same as IoTConsensus, so may no need to add UT

@BUAAserein I still believe it is necessary, as UT are an effective tools to ensure the logic consistency between two functions. This can be exemplified by the numerous serialize-deserialize UTs in IoTDB, such as NodeInfoTest.testSnapshot.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we add parsingAndConstructIDTest

@liyuheng55555 liyuheng55555 force-pushed the Working/region-migration-0401 branch from 60aaa61 to bf31c3b Compare April 7, 2024 07:45
@liyuheng55555 liyuheng55555 force-pushed the Working/region-migration-0401 branch from 9511e26 to 5e99732 Compare April 9, 2024 06:20
Copy link
Collaborator Author

@liyuheng55555 liyuheng55555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL~ @BUAAserein

+ region.getDatabaseName()
+ "-"
+ region.getDataRegionId();
String snapshotDir = "null";
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BUAAserein seems not necessary to set to "null"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default is ""? or just use null

+ region.getDatabaseName()
+ "-"
+ region.getDataRegionId();
String snapshotDir = "null";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default is ""? or just use null

*
* @return consensusGroupId list
*/
List<ConsensusGroupId> getAllConsensusGroupIdsFromDisk();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getAllConsensusGroupIdsWithoutStarting()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

filePath.getFileName().toString().endsWith(CONFIGURATION_TMP_FILE_NAME))
.collect(Collectors.toList());
for (Path filePath : paths) {
String formalPath =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if formalPath exsists, should delete first

}
});
}
serializeConfigurationAndFsyncToDisk();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add rename

liyuheng55555 and others added 6 commits April 12, 2024 11:50
Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
Copy link
Contributor

@OneSizeFitsQuorum OneSizeFitsQuorum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@OneSizeFitsQuorum OneSizeFitsQuorum merged commit f52d752 into apache:master Apr 12, 2024
@OneSizeFitsQuorum OneSizeFitsQuorum deleted the Working/region-migration-0401 branch April 12, 2024 11:09
liyuheng55555 added a commit to liyuheng55555/iotdb that referenced this pull request Apr 15, 2024
Co-authored-by: BUAAserein <18376359@buaa.edu.cn>
HTHou pushed a commit that referenced this pull request Jun 26, 2024
SzyWilliam pushed a commit to SzyWilliam/iotdb that referenced this pull request Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants