OneSizeFitsQuorum commented on code in PR #9155:
URL: https://github.com/apache/iotdb/pull/9155#discussion_r1119517611
##########
confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/datanode/UpdateDataNodePlan.java:
##########
@@ -30,42 +31,49 @@
public class UpdateDataNodePlan extends ConfigPhysicalPlan {
- private TDataNodeLocation dataNodeLocation;
+ private TDataNodeConfiguration dataNodeConfiguration;
public UpdateDataNodePlan() {
super(ConfigPhysicalPlanType.UpdateDataNodeConfiguration);
}
- public UpdateDataNodePlan(TDataNodeLocation datanodeLocation) {
+ public UpdateDataNodePlan(TDataNodeConfiguration dataNodeConfiguration) {
this();
- this.dataNodeLocation = datanodeLocation;
+ this.dataNodeConfiguration = dataNodeConfiguration;
}
- public TDataNodeLocation getDataNodeLocation() {
- return dataNodeLocation;
+ public TDataNodeConfiguration getDataNodeConfiguration() {
+ return dataNodeConfiguration;
}
@Override
protected void serializeImpl(DataOutputStream stream) throws IOException {
stream.writeShort(getType().getPlanType());
- ThriftCommonsSerDeUtils.serializeTDataNodeLocation(dataNodeLocation,
stream);
+
ThriftCommonsSerDeUtils.serializeTDataNodeConfiguration(dataNodeConfiguration,
stream);
}
@Override
protected void deserializeImpl(ByteBuffer buffer) {
- dataNodeLocation =
ThriftCommonsSerDeUtils.deserializeTDataNodeLocation(buffer);
+ dataNodeConfiguration =
ThriftCommonsSerDeUtils.deserializeTDataNodeConfiguration(buffer);
}
@Override
public boolean equals(Object o) {
- if (this == o) return true;
- if (o == null || getClass() != o.getClass()) return false;
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ if (!super.equals(o)) {
Review Comment:
What about remove this line and compare `type` directly...
##########
confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/datanode/UpdateDataNodePlan.java:
##########
@@ -30,42 +31,49 @@
public class UpdateDataNodePlan extends ConfigPhysicalPlan {
- private TDataNodeLocation dataNodeLocation;
+ private TDataNodeConfiguration dataNodeConfiguration;
public UpdateDataNodePlan() {
super(ConfigPhysicalPlanType.UpdateDataNodeConfiguration);
}
- public UpdateDataNodePlan(TDataNodeLocation datanodeLocation) {
+ public UpdateDataNodePlan(TDataNodeConfiguration dataNodeConfiguration) {
this();
- this.dataNodeLocation = datanodeLocation;
+ this.dataNodeConfiguration = dataNodeConfiguration;
}
- public TDataNodeLocation getDataNodeLocation() {
- return dataNodeLocation;
+ public TDataNodeConfiguration getDataNodeConfiguration() {
+ return dataNodeConfiguration;
}
@Override
protected void serializeImpl(DataOutputStream stream) throws IOException {
stream.writeShort(getType().getPlanType());
- ThriftCommonsSerDeUtils.serializeTDataNodeLocation(dataNodeLocation,
stream);
+
ThriftCommonsSerDeUtils.serializeTDataNodeConfiguration(dataNodeConfiguration,
stream);
}
@Override
protected void deserializeImpl(ByteBuffer buffer) {
- dataNodeLocation =
ThriftCommonsSerDeUtils.deserializeTDataNodeLocation(buffer);
+ dataNodeConfiguration =
ThriftCommonsSerDeUtils.deserializeTDataNodeConfiguration(buffer);
}
@Override
public boolean equals(Object o) {
- if (this == o) return true;
- if (o == null || getClass() != o.getClass()) return false;
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ if (!super.equals(o)) {
+ return false;
+ }
UpdateDataNodePlan that = (UpdateDataNodePlan) o;
- return dataNodeLocation.equals(that.dataNodeLocation);
+ return dataNodeConfiguration.equals(that.dataNodeConfiguration);
}
@Override
public int hashCode() {
- return Objects.hash(dataNodeLocation);
+ return Objects.hash(super.hashCode(), dataNodeConfiguration);
Review Comment:
use `type` directly
##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/node/ClusterNodeStartUtils.java:
##########
@@ -336,47 +345,54 @@ public static TDataNodeLocation matchRegisteredDataNode(
/**
* Check if some TEndPoints of the specified ConfigNode have updated.
*
- * @return True if some TEndPoints of the specified ConfigNode have updated,
false otherwise.
+ * @param restartLocation The location of restart ConfigNode
+ * @param recordLocation The record ConfigNode location
+ * @return The set of TEndPoints that have modified. 0: internalEndPoint, 1:
consensusEndPoint
Review Comment:
Remove these magic number?
##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/node/ClusterNodeStartUtils.java:
##########
@@ -195,30 +195,39 @@ public static TSStatus confirmNodeRestart(
return status;
}
- boolean isTEndPointUpdated;
+ boolean acceptRestart = true;
+ Set<Integer> updatedTEndPoints;
switch (nodeType) {
case ConfigNode:
- isTEndPointUpdated =
- isTEndPointsOfTConfigNodeLocationUpdated(
+ updatedTEndPoints =
+ checkUpdatedTEndPointOfConfigNode(
(TConfigNodeLocation) nodeLocation, (TConfigNodeLocation)
matchedNodeLocation);
+ if (!updatedTEndPoints.isEmpty()) {
+ // TODO: Accept internal TEndPoints
+ acceptRestart = false;
+ }
break;
case DataNode:
default:
- isTEndPointUpdated =
- isTEndPointsOfTDataNodeLocationUpdated(
+ updatedTEndPoints =
+ checkUpdatedTEndPointOfDataNode(
(TDataNodeLocation) nodeLocation, (TDataNodeLocation)
matchedNodeLocation);
+ if (!updatedTEndPoints.isEmpty()
Review Comment:
You can remove this line as
`updatedTEndPoints.stream().max(Integer::compare)` will `return a
Optional.empty()` when `updatedTEndPoints` is empty
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
