Misc minor fixes in Helix tool, ReplicationEngine and Composite Manager #1157
Conversation
jsjtzyy
commented
Apr 16, 2019
jsjtzyy
commented
- Add null check in getDatacenterName() in CompositeClusterManager.
- Make instance config change ignore count more accurate.
- Ensure Helix bootstrap/upgrade tool can exit in the end.
- Break the loop if remoteReplicaInfo is found in getRemoteReplicaInfo().
1. Add null check in getDatacenterName() in CompositeClusterManager. 2. Make instance config change ignore count more accurate. 3. Ensure Helix bootstrap/upgrade tool can exit in the end. 4. Break the loop if remoteReplicaInfo is found in getRemoteReplicaInfo().
|
./gradlew build && ./gradlew test succeeded. |
Codecov Report
@@ Coverage Diff @@
## master #1157 +/- ##
=========================================
Coverage ? 69.58%
Complexity ? 5240
=========================================
Files ? 414
Lines ? 32311
Branches ? 4116
=========================================
Hits ? 22483
Misses ? 8698
Partials ? 1130
Continue to review full report at Codecov.
|
| @@ -1840,6 +1840,7 @@ void addMessage(PartitionId id, MessageInfo messageInfo, ByteBuffer buffer) { | |||
| new RemoteReplicaInfo(peerReplicaId, replicaId, store, new MockFindToken(0, 0), Long.MAX_VALUE, | |||
| SystemTime.getInstance(), new Port(peerReplicaId.getDataNodeId().getPort(), PortType.PLAINTEXT)); | |||
| remoteReplicaInfos.add(remoteReplicaInfo); | |||
| break; | |||
There was a problem hiding this comment.
It's possible to have two replicas(belong to same partition) on same host. We can't assume there is only one, right?
There was a problem hiding this comment.
I made the change based on the assumption that no two replicas belonging to same partition would reside on same node. Since this is only a test and we may allow two replicas on single node in the future, so I decide to remove the break here.
