You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/06/09 10:40:16 UTC
[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #6206: [IOTDB-3435] Interface of migrate region for internal service
CRZbulabula commented on code in PR #6206:
URL: https://github.com/apache/iotdb/pull/6206#discussion_r893347474
##########
server/src/main/java/org/apache/iotdb/db/service/thrift/impl/InternalServiceImpl.java:
##########
@@ -382,6 +372,55 @@ public TSStatus deleteRegion(TConsensusGroupId tconsensusGroupId) throws TExcept
}
}
+ @Override
+ public TMigrateRegionResp migrateRegion(TMigrateRegionReq req) throws TException {
+ TRegionReplicaSet regionReplicaSet = req.migrateRegion;
+ TSStatus tsStatus;
+ ConsensusGenericResponse consensusGenericResponse;
+ switch (regionReplicaSet.regionId.type) {
+ case DataRegion:
+ DataRegionId dataRegionId = new DataRegionId(regionReplicaSet.getRegionId().getId());
+ List<Peer> newPeers = new ArrayList<>();
+ for (TDataNodeLocation dataNodeLocation : regionReplicaSet.getDataNodeLocations()) {
+ TEndPoint endpoint =
+ new TEndPoint(
+ dataNodeLocation.getDataRegionConsensusEndPoint().getIp(),
+ dataNodeLocation.getDataRegionConsensusEndPoint().getPort());
+ newPeers.add(new Peer(dataRegionId, endpoint));
+ }
+ consensusGenericResponse =
+ DataRegionConsensusImpl.getInstance().changePeer(dataRegionId, newPeers);
+ break;
+ case SchemaRegion:
+ SchemaRegionId schemaRegionId = new SchemaRegionId(regionReplicaSet.getRegionId().getId());
+ newPeers = new ArrayList<>();
+ for (TDataNodeLocation dataNodeLocation : regionReplicaSet.getDataNodeLocations()) {
+ TEndPoint endpoint =
+ new TEndPoint(
+ dataNodeLocation.getSchemaRegionConsensusEndPoint().getIp(),
+ dataNodeLocation.getSchemaRegionConsensusEndPoint().getPort());
+ newPeers.add(new Peer(schemaRegionId, endpoint));
+ }
+ consensusGenericResponse =
+ SchemaRegionConsensusImpl.getInstance().changePeer(schemaRegionId, newPeers);
+ break;
+ default:
+ // unsupported region type
+ tsStatus = new TSStatus(TSStatusCode.INTERNAL_SERVER_ERROR.getStatusCode());
Review Comment:
We should avoid using the INTERNAL_SERVER_ERROR. Otherwise, the interface user would never know what's wrong with the internal server.
##########
server/src/main/java/org/apache/iotdb/db/service/thrift/impl/InternalServiceImpl.java:
##########
@@ -382,6 +372,55 @@ public TSStatus deleteRegion(TConsensusGroupId tconsensusGroupId) throws TExcept
}
}
+ @Override
+ public TMigrateRegionResp migrateRegion(TMigrateRegionReq req) throws TException {
+ TRegionReplicaSet regionReplicaSet = req.migrateRegion;
+ TSStatus tsStatus;
+ ConsensusGenericResponse consensusGenericResponse;
+ switch (regionReplicaSet.regionId.type) {
+ case DataRegion:
+ DataRegionId dataRegionId = new DataRegionId(regionReplicaSet.getRegionId().getId());
+ List<Peer> newPeers = new ArrayList<>();
+ for (TDataNodeLocation dataNodeLocation : regionReplicaSet.getDataNodeLocations()) {
+ TEndPoint endpoint =
+ new TEndPoint(
+ dataNodeLocation.getDataRegionConsensusEndPoint().getIp(),
+ dataNodeLocation.getDataRegionConsensusEndPoint().getPort());
+ newPeers.add(new Peer(dataRegionId, endpoint));
+ }
+ consensusGenericResponse =
+ DataRegionConsensusImpl.getInstance().changePeer(dataRegionId, newPeers);
+ break;
+ case SchemaRegion:
+ SchemaRegionId schemaRegionId = new SchemaRegionId(regionReplicaSet.getRegionId().getId());
+ newPeers = new ArrayList<>();
+ for (TDataNodeLocation dataNodeLocation : regionReplicaSet.getDataNodeLocations()) {
+ TEndPoint endpoint =
+ new TEndPoint(
+ dataNodeLocation.getSchemaRegionConsensusEndPoint().getIp(),
+ dataNodeLocation.getSchemaRegionConsensusEndPoint().getPort());
+ newPeers.add(new Peer(schemaRegionId, endpoint));
+ }
+ consensusGenericResponse =
+ SchemaRegionConsensusImpl.getInstance().changePeer(schemaRegionId, newPeers);
+ break;
+ default:
+ // unsupported region type
+ tsStatus = new TSStatus(TSStatusCode.INTERNAL_SERVER_ERROR.getStatusCode());
+ tsStatus.setMessage("Region type is invalid");
+ return new TMigrateRegionResp(tsStatus);
+ }
+
+ if (consensusGenericResponse.isSuccess()) {
+ tsStatus = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
+ } else {
+ tsStatus = new TSStatus(TSStatusCode.INTERNAL_SERVER_ERROR.getStatusCode());
Review Comment:
We should avoid using the INTERNAL_SERVER_ERROR. Otherwise, the interface user would never know what's wrong with the internal server.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org