This is an automated email from the ASF dual-hosted git repository. tanxinyu pushed a commit to branch consensus_module_refactor in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit 9a7c1baec65fdb49791bad78bfd494fa3a08bc07 Author: BUAAserein <[email protected]> AuthorDate: Thu Aug 17 17:40:06 2023 +0800 refactor addRemotePeer --- .../manager/consensus/ConsensusManager.java | 19 ++++++++----------- .../consensus/simple/SimpleConsensusTest.java | 22 ++++++++++++++-------- .../iotdb/db/service/RegionMigrateService.java | 20 ++++++++------------ 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/consensus/ConsensusManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/consensus/ConsensusManager.java index 62345c5a136..b8869263d83 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/consensus/ConsensusManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/consensus/ConsensusManager.java @@ -269,17 +269,14 @@ public class ConsensusManager { * @throws AddPeerException When addPeer doesn't success */ public void addConfigNodePeer(TConfigNodeLocation configNodeLocation) throws AddPeerException { - boolean result = - consensusImpl - .addRemotePeer( - DEFAULT_CONSENSUS_GROUP_ID, - new Peer( - DEFAULT_CONSENSUS_GROUP_ID, - configNodeLocation.getConfigNodeId(), - configNodeLocation.getConsensusEndPoint())) - .isSuccess(); - - if (!result) { + try { + consensusImpl.addRemotePeer( + DEFAULT_CONSENSUS_GROUP_ID, + new Peer( + DEFAULT_CONSENSUS_GROUP_ID, + configNodeLocation.getConfigNodeId(), + configNodeLocation.getConsensusEndPoint())); + } catch (ConsensusException e) { throw new AddPeerException(configNodeLocation); } } diff --git a/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java b/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java index 48f0d240ddb..57dee99eae7 100644 --- a/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java +++ b/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java @@ -226,18 +226,24 @@ public class SimpleConsensusTest { @Test public void addPeer() { - ConsensusGenericResponse response = - consensusImpl.addRemotePeer( - dataRegionId, new Peer(dataRegionId, 1, new TEndPoint("0.0.0.0", 6667))); - assertFalse(response.isSuccess()); + try { + consensusImpl.addRemotePeer( + dataRegionId, new Peer(dataRegionId, 1, new TEndPoint("0.0.0.0", 6667))); + assert false; + } catch (ConsensusException e) { + assert true; + } } @Test public void removePeer() { - ConsensusGenericResponse response = - consensusImpl.removeRemotePeer( - dataRegionId, new Peer(dataRegionId, 1, new TEndPoint("0.0.0.0", 6667))); - assertFalse(response.isSuccess()); + try { + consensusImpl.removeRemotePeer( + dataRegionId, new Peer(dataRegionId, 1, new TEndPoint("0.0.0.0", 6667))); + assert false; + } catch (ConsensusException e) { + assert true; + } } @Test diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/RegionMigrateService.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/RegionMigrateService.java index 6a8f7958cae..f4360a8cd02 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/RegionMigrateService.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/RegionMigrateService.java @@ -224,9 +224,10 @@ public class RegionMigrateService implements IService { if (!addPeerSucceed) { Thread.sleep(SLEEP_MILLIS); } - resp = - addRegionPeer( - regionId, new Peer(regionId, destDataNode.getDataNodeId(), destEndpoint)); + addRegionPeer(regionId, new Peer(regionId, destDataNode.getDataNodeId(), destEndpoint)); + if (addPeerSucceed) { + break; + } } catch (Throwable e) { addPeerSucceed = false; taskLogger.error( @@ -237,12 +238,9 @@ public class RegionMigrateService implements IService { i, e); } - if (addPeerSucceed && resp != null && resp.isSuccess()) { - break; - } } - if (!addPeerSucceed || resp == null || !resp.isSuccess()) { + if (!addPeerSucceed) { String errorMsg = String.format( "%s, AddPeer for region error after max retry times, peerId: %s, regionId: %s, resp: %s", @@ -263,14 +261,12 @@ public class RegionMigrateService implements IService { return status; } - private ConsensusGenericResponse addRegionPeer(ConsensusGroupId regionId, Peer newPeer) { - ConsensusGenericResponse resp; + private void addRegionPeer(ConsensusGroupId regionId, Peer newPeer) throws ConsensusException { if (regionId instanceof DataRegionId) { - resp = DataRegionConsensusImpl.getInstance().addRemotePeer(regionId, newPeer); + DataRegionConsensusImpl.getInstance().addRemotePeer(regionId, newPeer); } else { - resp = SchemaRegionConsensusImpl.getInstance().addRemotePeer(regionId, newPeer); + SchemaRegionConsensusImpl.getInstance().addRemotePeer(regionId, newPeer); } - return resp; } }
