You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/11/27 08:08:50 UTC

[GitHub] [iotdb] SzyWilliam opened a new pull request, #8202: Ratis disk usage control

SzyWilliam opened a new pull request, #8202:
URL: https://github.com/apache/iotdb/pull/8202

   Control total size of Raft Log


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


[GitHub] [iotdb] qiaojialin merged pull request #8202: [To rel/1.0] Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
qiaojialin merged PR #8202:
URL: https://github.com/apache/iotdb/pull/8202


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


[GitHub] [iotdb] SzyWilliam commented on pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#issuecomment-1328245094

   @OneSizeFitsQuorum Thanks for reviewing on this! I'll add a TODO for long term testing.


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


[GitHub] [iotdb] SzyWilliam commented on pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#issuecomment-1330423479

   The write throughput is not much affected. Using a 5GB strict restriction, write throughput is about 65% of IoTConsensus. (431w / 690w)


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


[GitHub] [iotdb] OneSizeFitsQuorum commented on a diff in pull request #8202: [To rel/1.0] Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#discussion_r1034672247


##########
node-commons/src/assembly/resources/conf/iotdb-common.properties:
##########
@@ -952,6 +952,11 @@
 # schema_region_ratis_preserve_logs_num_when_purge=1000
 # data_region_ratis_preserve_logs_num_when_purge=1000
 
+# Raft Log disk size control
+# config_node_ratis_log_max_size = 2147483648

Review Comment:
   Please make them consistent in different places.



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


[GitHub] [iotdb] OneSizeFitsQuorum commented on a diff in pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#discussion_r1032914611


##########
thrift-confignode/src/main/thrift/confignode.thrift:
##########
@@ -89,6 +89,9 @@ struct TRatisConfig {
 
   25: required i64 firstElectionTimeoutMin
   26: required i64 firstElectionTimeoutMax
+
+  27: required i64 schemaRatisLogMax
+  28: required i64 dataRatisLogMax

Review Comment:
   dataRegionRatisLogMax



##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -1035,6 +1035,9 @@ public class IoTDBConfig {
   private long ratisFirstElectionTimeoutMinMs = 50L;
   private long ratisFirstElectionTimeoutMaxMs = 150L;
 
+  private long dataRatisLogMaxMB = 20480;
+  private long schemaRatisLogMaxMB = 20480;

Review Comment:
   2 * 1024



##########
thrift-confignode/src/main/thrift/confignode.thrift:
##########
@@ -89,6 +89,9 @@ struct TRatisConfig {
 
   25: required i64 firstElectionTimeoutMin
   26: required i64 firstElectionTimeoutMax
+
+  27: required i64 schemaRatisLogMax

Review Comment:
   schemaRegionRatisLogMax



##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -1035,6 +1035,9 @@ public class IoTDBConfig {
   private long ratisFirstElectionTimeoutMinMs = 50L;
   private long ratisFirstElectionTimeoutMaxMs = 150L;
 
+  private long dataRatisLogMaxMB = 20480;

Review Comment:
   20 * 1024



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


[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #8202: [To rel/1.0] Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#discussion_r1034678809


##########
node-commons/src/assembly/resources/conf/iotdb-common.properties:
##########
@@ -952,6 +952,11 @@
 # schema_region_ratis_preserve_logs_num_when_purge=1000
 # data_region_ratis_preserve_logs_num_when_purge=1000
 
+# Raft Log disk size control
+# config_node_ratis_log_max_size = 2147483648

Review Comment:
   Done. Thanks for catching!



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


[GitHub] [iotdb] SzyWilliam commented on pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#issuecomment-1330420952

   When Disk control is set to 5GB, we can clearly see that the Consensus Dir usage reaches 5GB and then decreases. During the whole benchmark, the consensus dir disk usage is limited under 5GB.
   <img width="775" alt="image" src="https://user-images.githubusercontent.com/48054931/204506506-511e9d83-8678-4fc0-b874-95e686efa046.png">
   


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


[GitHub] [iotdb] OneSizeFitsQuorum commented on a diff in pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#discussion_r1032886138


##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeDescriptor.java:
##########
@@ -691,6 +691,30 @@ private void loadRatisConsensusConfig(Properties properties) {
                     "ratis_first_election_timeout_max_ms",
                     String.valueOf(conf.getRatisFirstElectionTimeoutMaxMs()))
                 .trim()));
+
+    conf.setConfigNodeRatisLogMaxMB(
+        Long.parseLong(
+            properties
+                .getProperty(
+                    "config_node_ratis_log_max_size_MB",

Review Comment:
   MB -> mb?



##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConfig.java:
##########
@@ -247,6 +247,10 @@ public class ConfigNodeConfig {
   private long ratisFirstElectionTimeoutMinMs = 50;
   private long ratisFirstElectionTimeoutMaxMs = 150;
 
+  private long configNodeRatisLogMaxMB = 20480;
+  private long schemaRegionRatisLogMaxMB = 20480;
+  private long dataRegionRatisLogMaxMB = 20480;

Review Comment:
   maybe we need different default size for different region



##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConfig.java:
##########
@@ -247,6 +247,10 @@ public class ConfigNodeConfig {
   private long ratisFirstElectionTimeoutMinMs = 50;
   private long ratisFirstElectionTimeoutMaxMs = 150;
 
+  private long configNodeRatisLogMaxMB = 20480;

Review Comment:
   20480->20 * 1024?



##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeDescriptor.java:
##########
@@ -691,6 +691,30 @@ private void loadRatisConsensusConfig(Properties properties) {
                     "ratis_first_election_timeout_max_ms",
                     String.valueOf(conf.getRatisFirstElectionTimeoutMaxMs()))
                 .trim()));
+
+    conf.setConfigNodeRatisLogMaxMB(
+        Long.parseLong(
+            properties
+                .getProperty(
+                    "config_node_ratis_log_max_size_MB",
+                    String.valueOf(conf.getConfigNodeRatisLogMaxMB()))
+                .trim()));
+
+    conf.setSchemaRegionRatisLogMaxMB(
+        Long.parseLong(
+            properties
+                .getProperty(
+                    "schema_region_ratis_log_max_size_MB",

Review Comment:
   .



##########
node-commons/src/assembly/resources/conf/iotdb-common.properties:
##########
@@ -931,6 +931,11 @@
 # schema_region_ratis_preserve_logs_num_when_purge=1000
 # data_region_ratis_preserve_logs_num_when_purge=1000
 
+# Raft Log disk size control
+# config_node_ratis_log_max_size_MB = 20480

Review Comment:
   1. MB -> mb
   2. maybe different size



##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/ConsensusManager.java:
##########
@@ -164,6 +164,8 @@ private void setConsensusLayer(ConfigNodeRegionStateMachine stateMachine) throws
                                           CONF.getConfigNodeRatisInitialSleepTimeMs())
                                       .setClientRetryMaxSleepTimeMs(
                                           CONF.getConfigNodeRatisMaxSleepTimeMs())
+                                      .setTriggerSnapshotFileSize(
+                                          CONF.getSchemaRegionRatisLogMaxMB() * 1024 * 1024)

Review Comment:
   configRegion ?



##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeDescriptor.java:
##########
@@ -691,6 +691,30 @@ private void loadRatisConsensusConfig(Properties properties) {
                     "ratis_first_election_timeout_max_ms",
                     String.valueOf(conf.getRatisFirstElectionTimeoutMaxMs()))
                 .trim()));
+
+    conf.setConfigNodeRatisLogMaxMB(
+        Long.parseLong(
+            properties
+                .getProperty(
+                    "config_node_ratis_log_max_size_MB",
+                    String.valueOf(conf.getConfigNodeRatisLogMaxMB()))
+                .trim()));
+
+    conf.setSchemaRegionRatisLogMaxMB(
+        Long.parseLong(
+            properties
+                .getProperty(
+                    "schema_region_ratis_log_max_size_MB",
+                    String.valueOf(conf.getSchemaRegionRatisLogMaxMB()))
+                .trim()));
+
+    conf.setDataRegionRatisLogMaxMB(
+        Long.parseLong(
+            properties
+                .getProperty(
+                    "data_region_ratis_log_max_size_MB",

Review Comment:
   .



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


[GitHub] [iotdb] qiaojialin commented on a diff in pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
qiaojialin commented on code in PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#discussion_r1032939113


##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -672,6 +680,42 @@ public ConsensusGenericResponse triggerSnapshot(ConsensusGroupId groupId) {
     return ConsensusGenericResponse.newBuilder().setSuccess(reply.isSuccess()).build();
   }
 
+  private void triggerSnapshotByCustomize() {
+
+    Iterable<RaftGroupId> groupIds = server.getGroupIds();
+
+    for (RaftGroupId raftGroupId : groupIds) {
+      File currentDir = null;
+
+      try {
+        currentDir =
+            server.getDivision(raftGroupId).getRaftStorage().getStorageDir().getCurrentDir();
+      } catch (IOException e) {
+        logger.warn("Get division failed: ", e);
+      }
+
+      final long currentDirLength =
+          org.apache.iotdb.consensus.common.Utils.getTotalFolderSize(currentDir);

Review Comment:
   If you recalculate in each call, This may time consuming



##########
thrift-confignode/src/main/thrift/confignode.thrift:
##########
@@ -89,6 +89,9 @@ struct TRatisConfig {
 
   25: required i64 firstElectionTimeoutMin
   26: required i64 firstElectionTimeoutMax
+
+  27: required i64 schemaRegionRatisLogMax
+  28: required i64 dataRegionRatisLogMax

Review Comment:
   optional



##########
consensus/src/main/java/org/apache/iotdb/consensus/common/Utils.java:
##########
@@ -74,4 +74,10 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc)
     }
     return allFiles;
   }
+
+  public static long getTotalFolderSize(File rootDir) {

Review Comment:
   refer to WAL, they calculate this in an incremented manner



##########
node-commons/src/assembly/resources/conf/iotdb-common.properties:
##########
@@ -931,6 +931,11 @@
 # schema_region_ratis_preserve_logs_num_when_purge=1000
 # data_region_ratis_preserve_logs_num_when_purge=1000
 
+# Raft Log disk size control
+# config_node_ratis_log_max_size_mb = 2048
+# schema_region_ratis_log_max_size_mb = 2048
+# data_region_ratis_log_max_size_mb = 20480

Review Comment:
   in byte



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


[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#discussion_r1032948076


##########
thrift-confignode/src/main/thrift/confignode.thrift:
##########
@@ -89,6 +89,9 @@ struct TRatisConfig {
 
   25: required i64 firstElectionTimeoutMin
   26: required i64 firstElectionTimeoutMax
+
+  27: required i64 schemaRegionRatisLogMax
+  28: required i64 dataRegionRatisLogMax

Review Comment:
   In current code path, these two options always will be passed to datanode. So I set it to required.



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


[GitHub] [iotdb] qiaojialin commented on pull request #8202: Ratis disk usage control

Posted by GitBox <gi...@apache.org>.
qiaojialin commented on PR #8202:
URL: https://github.com/apache/iotdb/pull/8202#issuecomment-1328274310

   need test


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