[GitHub] [iotdb] Beyyes commented on a diff in pull request #7361: [IOTDB-4420] Monitor DataNode's remain disk space through cluster heartbeat

[GitBox <gi...@apache.org>]

Beyyes commented on code in PR #7361:
URL: https://github.com/apache/iotdb/pull/7361#discussion_r975236740


##########
node-commons/src/main/java/org/apache/iotdb/commons/cluster/NodeStatus.java:
##########
@@ -29,6 +29,9 @@ public enum NodeStatus {
   /** Node is in removing */
   Removing("Removing"),
 
+  /** Node disk is running out */
+  Full("Full"),

Review Comment:
   How about renaming `DiskFull`?



##########
thrift/src/main/thrift/datanode.thrift:
##########
@@ -199,7 +199,8 @@ struct THeartbeatResp {
   2: required string status
   3: optional map<common.TConsensusGroupId, bool> judgedLeaders
   4: optional i16 cpu
-  5: optional i16 memory
+  5: optional double memory
+  6: optional double disk

Review Comment:
   add comments and example?
   disk are the real disk usage?



##########
confignode/src/assembly/resources/conf/iotdb-confignode.properties:
##########
@@ -342,4 +342,20 @@ target_config_nodes=127.0.0.1:22277
 ####################
 
 # max payload size for a single log-sync-RPC from leader to follower
-# ratis_log_appender_buffer_size_max = 4194304
\ No newline at end of file
+# ratis_log_appender_buffer_size_max = 4194304
+
+
+####################
+### Disk Monitor
+####################
+
+
+# Threshold at which DataNode is set to Full status
+# i.e. DataNode will no longer be allocated more partitions when its status is set to full
+# Datatype: double(percentage)
+# full_threshold=5.0
+
+# Threshold at which DataNode is set to ReadOnly status

Review Comment:
   Disk remaining threshold?



##########
confignode/src/assembly/resources/conf/iotdb-confignode.properties:
##########
@@ -342,4 +342,20 @@ target_config_nodes=127.0.0.1:22277
 ####################
 
 # max payload size for a single log-sync-RPC from leader to follower
-# ratis_log_appender_buffer_size_max = 4194304
\ No newline at end of file
+# ratis_log_appender_buffer_size_max = 4194304
+
+
+####################
+### Disk Monitor
+####################
+
+
+# Threshold at which DataNode is set to Full status

Review Comment:
   ```suggestion
   # Disk remaining threshold at which DataNode is set to Full status
   ```
   is better?



##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/NodeManager.java:
##########
@@ -670,17 +678,23 @@ public boolean isNodeRemoving(int dataNodeId) {
     DataNodeHeartbeatCache cache =
         (DataNodeHeartbeatCache) configManager.getNodeManager().getNodeCacheMap().get(dataNodeId);
     if (cache != null) {
-      return cache.isRemoving();
+      return NodeStatus.Removing.equals(cache.getNodeStatus());
     }
     return false;
   }
 
-  public void setNodeRemovingStatus(int dataNodeId, boolean isRemoving) {
+  public void setNodeRemovingStatus(TDataNodeLocation dataNodeLocation) {
     DataNodeHeartbeatCache cache =
-        (DataNodeHeartbeatCache) configManager.getNodeManager().getNodeCacheMap().get(dataNodeId);
+        (DataNodeHeartbeatCache)
+            configManager.getNodeManager().getNodeCacheMap().get(dataNodeLocation.getDataNodeId());
     if (cache != null) {
-      cache.setRemoving(isRemoving);

Review Comment:
   Will heartbeat change the status of `Remvoing`?



##########
confignode/src/assembly/resources/conf/iotdb-confignode.properties:
##########
@@ -342,4 +342,20 @@ target_config_nodes=127.0.0.1:22277
 ####################
 
 # max payload size for a single log-sync-RPC from leader to follower
-# ratis_log_appender_buffer_size_max = 4194304
\ No newline at end of file
+# ratis_log_appender_buffer_size_max = 4194304
+
+
+####################
+### Disk Monitor
+####################
+
+
+# Threshold at which DataNode is set to Full status
+# i.e. DataNode will no longer be allocated more partitions when its status is set to full
+# Datatype: double(percentage)
+# full_threshold=5.0
+
+# Threshold at which DataNode is set to ReadOnly status
+# i.e. DataNode will no longer be able to perform write requests
+# Datatype: double(percentage)
+# read_only_threshold=1.0

Review Comment:
   disk_remaining_to_set_read_only_threshold?
   
   hard to naming.  I think just `read_only_threshold` is ambiguous



##########
confignode/src/assembly/resources/conf/iotdb-confignode.properties:
##########
@@ -342,4 +342,20 @@ target_config_nodes=127.0.0.1:22277
 ####################
 
 # max payload size for a single log-sync-RPC from leader to follower
-# ratis_log_appender_buffer_size_max = 4194304
\ No newline at end of file
+# ratis_log_appender_buffer_size_max = 4194304
+
+
+####################
+### Disk Monitor
+####################
+
+
+# Threshold at which DataNode is set to Full status
+# i.e. DataNode will no longer be allocated more partitions when its status is set to full
+# Datatype: double(percentage)
+# full_threshold=5.0

Review Comment:
   maybe `disk_full_threshold` is better?



-- 
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