You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "liyuheng55555 (via GitHub)" <gi...@apache.org> on 2024/03/25 07:07:22 UTC
[PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
liyuheng55555 opened a new pull request, #12232:
URL: https://github.com/apache/iotdb/pull/12232
Over the past period, some initialization work has been brought forward to the DataNode startup phase, which has slightly slowed down the startup speed of the DataNode, and in turn, increased the time between DataNode registration and the start of the RPC service.
Now, if the ConfigNode and DataNode are started simultaneously, a fault may occur where the DataNode cannot obtain a complete list of ConfigNodes.
The occurrence of this fault requires a specific sequence of events:
- When the DataNode registration returns, because the ConfigNode has not completed registration, the DataNode does not get a complete list of ConfigNodes.
- After the ConfigNode registration is complete, it fails to send the complete list to the DataNode via RPC because the DataNode RPC service has not yet started.
To solve this problem, we will use heartbeat to transit ConfigNodes' list. The heartbeat will always carry ConfigNodes' list, until the DataNode confirms this list.
--
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
Re: [PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
Posted by "CRZbulabula (via GitHub)" <gi...@apache.org>.
CRZbulabula commented on code in PR #12232:
URL: https://github.com/apache/iotdb/pull/12232#discussion_r1537534426
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/client/DataNodeRequestType.java:
##########
@@ -32,7 +32,6 @@ public enum DataNodeRequestType {
STOP_REPAIR_DATA,
LOAD_CONFIGURATION,
SET_SYSTEM_STATUS,
- BROADCAST_LATEST_CONFIG_NODE_GROUP,
Review Comment:
We could keep the BROADCAST_LATEST_CONFIG_NODE_GROUP interface, since it's never too much to broadcast immediately when the cluster ConfigNode group has been updated. Therefore the services that rely on ConfigNodeList will save the 1s heartbeat interval time. The heartbeat function should be took as a security method to ensure the DataNodes can update their ConfigNode list when BROADCAST_LATEST_CONFIG_NODE_GROUP failed.
--
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
Re: [PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
Posted by "liyuheng55555 (via GitHub)" <gi...@apache.org>.
liyuheng55555 commented on code in PR #12232:
URL: https://github.com/apache/iotdb/pull/12232#discussion_r1538560452
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/client/DataNodeRequestType.java:
##########
@@ -32,7 +32,6 @@ public enum DataNodeRequestType {
STOP_REPAIR_DATA,
LOAD_CONFIGURATION,
SET_SYSTEM_STATUS,
- BROADCAST_LATEST_CONFIG_NODE_GROUP,
Review Comment:
This is indeed worth considering, and we can have a discussion.
I think there is no need to retain the original RPC notification mechanism here, as the ConfigNode list is not time-sensitive. After a ConfigNode is added, there doesn't seem to be anything that requires an immediate notification to the DataNode.
--
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
Re: [PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
Posted by "liyuheng55555 (via GitHub)" <gi...@apache.org>.
liyuheng55555 commented on code in PR #12232:
URL: https://github.com/apache/iotdb/pull/12232#discussion_r1538560452
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/client/DataNodeRequestType.java:
##########
@@ -32,7 +32,6 @@ public enum DataNodeRequestType {
STOP_REPAIR_DATA,
LOAD_CONFIGURATION,
SET_SYSTEM_STATUS,
- BROADCAST_LATEST_CONFIG_NODE_GROUP,
Review Comment:
This is indeed worth considering, and we can have a discussion.
I think there is no need to retain the original RPC notification mechanism here, as the ConfigNode list is not time-sensitive. After a ConfigNode is added, there doesn't seem to be anything that requires an immediate response from the DataNode.
--
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
Re: [PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #12232:
URL: https://github.com/apache/iotdb/pull/12232#discussion_r1537353159
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/service/HeartbeatService.java:
##########
@@ -150,6 +153,17 @@ private TDataNodeHeartbeatReq genHeartbeatReq() {
return heartbeatReq;
}
+ private void addConfigNodeLocationsToReq(int dataNodeId, TDataNodeHeartbeatReq req) {
+ Set<TConfigNodeLocation> confirmedConfigNodes =
+ loadCache.getConfirmedConfigNodeLocations(dataNodeId);
+ Set<TConfigNodeLocation> actualConfigNodes =
+ new HashSet<>(getNodeManager().getRegisteredConfigNodes());
+ if (!actualConfigNodes.equals(confirmedConfigNodes)
Review Comment:
always carry this list?
##########
iotdb-protocol/thrift-datanode/src/main/thrift/datanode.thrift:
##########
@@ -252,6 +252,7 @@ struct TDataNodeHeartbeatReq {
8: optional bool needPipeMetaList
9: optional i64 deviceQuotaRemain
10: optional TDataNodeActivation activation
+ 11: optional set<common.TConfigNodeLocation> configNodeLocations
Review Comment:
use id is enough?
--
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
Re: [PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
Posted by "liyuheng55555 (via GitHub)" <gi...@apache.org>.
liyuheng55555 commented on code in PR #12232:
URL: https://github.com/apache/iotdb/pull/12232#discussion_r1538560452
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/client/DataNodeRequestType.java:
##########
@@ -32,7 +32,6 @@ public enum DataNodeRequestType {
STOP_REPAIR_DATA,
LOAD_CONFIGURATION,
SET_SYSTEM_STATUS,
- BROADCAST_LATEST_CONFIG_NODE_GROUP,
Review Comment:
This is indeed worth considering, and we can have a discussion. I believe there is no need to retain the original RPC notification mechanism here, as the ConfigNode list is not time-sensitive. After a ConfigNode is added, there doesn't seem to be anything that requires an immediate response from the DataNode.
--
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
Re: [PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum merged PR #12232:
URL: https://github.com/apache/iotdb/pull/12232
--
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
Re: [PR] Use heartbeat to update DataNode's ConfigNodeList [iotdb]
Posted by "liyuheng55555 (via GitHub)" <gi...@apache.org>.
liyuheng55555 commented on code in PR #12232:
URL: https://github.com/apache/iotdb/pull/12232#discussion_r1537483007
##########
iotdb-protocol/thrift-datanode/src/main/thrift/datanode.thrift:
##########
@@ -252,6 +252,7 @@ struct TDataNodeHeartbeatReq {
8: optional bool needPipeMetaList
9: optional i64 deviceQuotaRemain
10: optional TDataNodeActivation activation
+ 11: optional set<common.TConfigNodeLocation> configNodeLocations
Review Comment:
use TEndPoint
--
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