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/05/05 10:55:44 UTC

[GitHub] [iotdb] cigarl opened a new pull request, #5807: [IOTDB-3097] PartitionInfo snapshot interface

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

   implements interface of PartitionInfo snapshot


-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap
+      serializeRegionMap(byteBuffer);
+      // third, put schemaPartition
+      schemaPartition.serialize(byteBuffer);
+      // then, put dataPartition
+      dataPartition.serialize(byteBuffer);
+      // write to file
+      try (FileOutputStream fileOutputStream = new FileOutputStream(tmpFile);
+          FileChannel fileChannel = fileOutputStream.getChannel()) {
+        fileChannel.write(byteBuffer);
+      }
+      // rename file
+      return tmpFile.renameTo(snapshotFile);
+    } finally {
+      unlockAllRead();
+      byteBuffer.clear();
+      // with or without success, delete temporary files anyway
+      tmpFile.delete();
+    }
+  }
+
+  public void loadSnapshot(File snapshotFile) throws TException, IOException {
+
+    // no operations are processed at this time
+    lockAllWrite();
+
+    ByteBuffer buffer = ByteBuffer.allocate(bufferSize);
+    try (FileInputStream fileInputStream = new FileInputStream(snapshotFile);
+        FileChannel fileChannel = fileInputStream.getChannel()) {
+      // get buffer from fileChannel
+      fileChannel.read(buffer);
+      // before restoring a snapshot, clear all old data
+      clear();
+      // start to restore
+      nextRegionGroupId.set(buffer.getInt());
+      deserializeRegionMap(buffer);
+      schemaPartition.deserialize(buffer);
+      dataPartition.deserialize(buffer);
+    } finally {
+      unlockAllWrite();
+      buffer.clear();
+    }

Review Comment:
   same as above



##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/DataPartition.java:
##########
@@ -238,11 +248,73 @@ public void createDataPartition(
         .put(timePartitionSlot, Collections.singletonList(regionReplicaSet));
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize DataPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {

Review Comment:
   fixed.



-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/DataPartition.java:
##########
@@ -238,11 +248,73 @@ public void createDataPartition(
         .put(timePartitionSlot, Collections.singletonList(regionReplicaSet));
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize DataPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {
+    try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
+      for (Entry<
+              String, Map<TSeriesPartitionSlot, Map<TTimePartitionSlot, List<TRegionReplicaSet>>>>
+          entry : dataPartitionMap.entrySet()) {
+        ReadWriteIOUtils.write(entry.getKey(), byteArrayOutputStream);
+        try (TIOStreamTransport tioStreamTransport =
+            new TIOStreamTransport(byteArrayOutputStream)) {
+          TProtocol protocol = new TBinaryProtocol(tioStreamTransport);
+          for (Entry<TSeriesPartitionSlot, Map<TTimePartitionSlot, List<TRegionReplicaSet>>>
+              entry1 : entry.getValue().entrySet()) {
+            entry1.getKey().write(protocol);
+            for (Entry<TTimePartitionSlot, List<TRegionReplicaSet>> entry2 :
+                entry1.getValue().entrySet()) {
+              entry2.getKey().write(protocol);
+              ReadWriteIOUtils.write(entry2.getValue().size(), byteArrayOutputStream);
+              entry2
+                  .getValue()
+                  .forEach(
+                      x -> {
+                        try {
+                          x.write(protocol);
+                        } catch (TException e) {
+                          throw new RuntimeException(e);
+                        }
+                      });
+            }
+          }
+        }
+      }
+      byte[] toArray = byteArrayOutputStream.toByteArray();
+      buffer.putInt(toArray.length);
+      buffer.put(toArray);
+    }
   }
 
-  public void deserialize(ByteBuffer buffer) {
-    // TODO: Deserialize DataPartition
+  public void deserialize(ByteBuffer buffer) throws TException, IOException {

Review Comment:
   fixed



##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/SchemaPartition.java:
##########
@@ -163,11 +173,61 @@ public void createSchemaPartition(
         .put(seriesPartitionSlot, regionReplicaSet);
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize SchemaPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {

Review Comment:
   fixed



##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/SchemaPartition.java:
##########
@@ -163,11 +173,61 @@ public void createSchemaPartition(
         .put(seriesPartitionSlot, regionReplicaSet);
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize SchemaPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {
+    try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
+      for (Entry<String, Map<TSeriesPartitionSlot, TRegionReplicaSet>> entry :
+          schemaPartitionMap.entrySet()) {
+        ReadWriteIOUtils.write(entry.getKey(), byteArrayOutputStream);
+        writeMap(entry.getValue(), byteArrayOutputStream);
+      }
+      byte[] toArray = byteArrayOutputStream.toByteArray();
+      buffer.putInt(toArray.length);
+      buffer.put(toArray);
+    }
   }
 
-  public void deserialize(ByteBuffer buffer) {
-    // TODO: Deserialize DataPartition
+  public void deserialize(ByteBuffer buffer) throws TException, IOException {

Review Comment:
   fixed



-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +321,142 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotDir) throws TException, IOException {
+
+    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    if (snapshotFile.exists() && snapshotFile.isFile()) {
+      LOGGER.error(
+          "Failed to take snapshot, because snapshot file [{}] is already exist.",
+          snapshotFile.getAbsolutePath());
+      return false;
+    }
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId

Review Comment:
   fixed



-- 
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 #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());

Review Comment:
   FileName is controlled by executor,see [#5816](https://github.com/apache/iotdb/pull/5816)



-- 
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 #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +321,142 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotDir) throws TException, IOException {
+
+    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    if (snapshotFile.exists() && snapshotFile.isFile()) {
+      LOGGER.error(
+          "Failed to take snapshot, because snapshot file [{}] is already exist.",
+          snapshotFile.getAbsolutePath());
+      return false;
+    }
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId

Review Comment:
   ```suggestion
         // put nextRegionGroupId
   ```



##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +321,142 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotDir) throws TException, IOException {
+
+    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    if (snapshotFile.exists() && snapshotFile.isFile()) {
+      LOGGER.error(
+          "Failed to take snapshot, because snapshot file [{}] is already exist.",
+          snapshotFile.getAbsolutePath());
+      return false;
+    }
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap

Review Comment:
   ```suggestion
         // put regionMap
   ```



-- 
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] CRZbulabula commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap
+      serializeRegionMap(byteBuffer);
+      // third, put schemaPartition
+      schemaPartition.serialize(byteBuffer);
+      // then, put dataPartition
+      dataPartition.serialize(byteBuffer);
+      // write to file
+      try (FileOutputStream fileOutputStream = new FileOutputStream(tmpFile);
+          FileChannel fileChannel = fileOutputStream.getChannel()) {
+        fileChannel.write(byteBuffer);
+      }
+      // rename file
+      return tmpFile.renameTo(snapshotFile);
+    } finally {
+      unlockAllRead();
+      byteBuffer.clear();
+      // with or without success, delete temporary files anyway
+      tmpFile.delete();

Review Comment:
   Got it!



-- 
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] CRZbulabula commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/DataPartition.java:
##########
@@ -238,11 +248,73 @@ public void createDataPartition(
         .put(timePartitionSlot, Collections.singletonList(regionReplicaSet));
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize DataPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {

Review Comment:
   Don't forget to add tests for serialize~



##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap
+      serializeRegionMap(byteBuffer);
+      // third, put schemaPartition
+      schemaPartition.serialize(byteBuffer);
+      // then, put dataPartition
+      dataPartition.serialize(byteBuffer);
+      // write to file
+      try (FileOutputStream fileOutputStream = new FileOutputStream(tmpFile);
+          FileChannel fileChannel = fileOutputStream.getChannel()) {
+        fileChannel.write(byteBuffer);
+      }
+      // rename file
+      return tmpFile.renameTo(snapshotFile);
+    } finally {
+      unlockAllRead();
+      byteBuffer.clear();
+      // with or without success, delete temporary files anyway
+      tmpFile.delete();

Review Comment:
   Don't forget to close FileChannel in finally block~



##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap
+      serializeRegionMap(byteBuffer);
+      // third, put schemaPartition
+      schemaPartition.serialize(byteBuffer);
+      // then, put dataPartition
+      dataPartition.serialize(byteBuffer);
+      // write to file
+      try (FileOutputStream fileOutputStream = new FileOutputStream(tmpFile);
+          FileChannel fileChannel = fileOutputStream.getChannel()) {
+        fileChannel.write(byteBuffer);
+      }
+      // rename file
+      return tmpFile.renameTo(snapshotFile);
+    } finally {
+      unlockAllRead();
+      byteBuffer.clear();
+      // with or without success, delete temporary files anyway
+      tmpFile.delete();
+    }
+  }
+
+  public void loadSnapshot(File snapshotFile) throws TException, IOException {

Review Comment:
   Don't forget to add test for loadSnapshot~



##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());

Review Comment:
   You may need to modify these two lines of code to
   
   ``` Java
   public boolean takeSnapshot(File snapshotDir) {
   
     File tmpFile = new File(snapshotFile.getAbsolutePath() + File.separator + UUID.randomUUID());
   }
   ```
   
   Because the takeSnapshot interface of the ConsensusLayer requires that snapshot files be stored in the **snapshotDir** directory each time.
   
   Perhaps it would be better for the takeSnapshot interface to just pass in **snapshotDir** but not **snapshotFile**. Because PartitionRegionStateMachine only responsible for informing takeSnapshot instruction, the snapshot file name should be managed by PartitionInfo.



##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {

Review Comment:
   Don't forget to add test for takeSnapshot~



##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/SchemaPartition.java:
##########
@@ -163,11 +173,61 @@ public void createSchemaPartition(
         .put(seriesPartitionSlot, regionReplicaSet);
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize SchemaPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {

Review Comment:
   Don't forget to add tests for serialize~



##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap
+      serializeRegionMap(byteBuffer);
+      // third, put schemaPartition
+      schemaPartition.serialize(byteBuffer);
+      // then, put dataPartition
+      dataPartition.serialize(byteBuffer);
+      // write to file
+      try (FileOutputStream fileOutputStream = new FileOutputStream(tmpFile);
+          FileChannel fileChannel = fileOutputStream.getChannel()) {
+        fileChannel.write(byteBuffer);
+      }
+      // rename file
+      return tmpFile.renameTo(snapshotFile);
+    } finally {
+      unlockAllRead();
+      byteBuffer.clear();
+      // with or without success, delete temporary files anyway
+      tmpFile.delete();
+    }
+  }
+
+  public void loadSnapshot(File snapshotFile) throws TException, IOException {
+
+    // no operations are processed at this time
+    lockAllWrite();
+
+    ByteBuffer buffer = ByteBuffer.allocate(bufferSize);
+    try (FileInputStream fileInputStream = new FileInputStream(snapshotFile);
+        FileChannel fileChannel = fileInputStream.getChannel()) {
+      // get buffer from fileChannel
+      fileChannel.read(buffer);
+      // before restoring a snapshot, clear all old data
+      clear();
+      // start to restore
+      nextRegionGroupId.set(buffer.getInt());
+      deserializeRegionMap(buffer);
+      schemaPartition.deserialize(buffer);
+      dataPartition.deserialize(buffer);
+    } finally {
+      unlockAllWrite();
+      buffer.clear();
+    }

Review Comment:
   Don't forget to close FileChannel in finally block~



##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/SchemaPartition.java:
##########
@@ -163,11 +173,61 @@ public void createSchemaPartition(
         .put(seriesPartitionSlot, regionReplicaSet);
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize SchemaPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {
+    try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
+      for (Entry<String, Map<TSeriesPartitionSlot, TRegionReplicaSet>> entry :
+          schemaPartitionMap.entrySet()) {
+        ReadWriteIOUtils.write(entry.getKey(), byteArrayOutputStream);
+        writeMap(entry.getValue(), byteArrayOutputStream);
+      }
+      byte[] toArray = byteArrayOutputStream.toByteArray();
+      buffer.putInt(toArray.length);
+      buffer.put(toArray);
+    }
   }
 
-  public void deserialize(ByteBuffer buffer) {
-    // TODO: Deserialize DataPartition
+  public void deserialize(ByteBuffer buffer) throws TException, IOException {

Review Comment:
   Don't forget to add tests for deserialize~



##########
node-commons/src/main/java/org/apache/iotdb/commons/partition/DataPartition.java:
##########
@@ -238,11 +248,73 @@ public void createDataPartition(
         .put(timePartitionSlot, Collections.singletonList(regionReplicaSet));
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize DataPartition
+  public void serialize(ByteBuffer buffer) throws IOException, TException {
+    try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
+      for (Entry<
+              String, Map<TSeriesPartitionSlot, Map<TTimePartitionSlot, List<TRegionReplicaSet>>>>
+          entry : dataPartitionMap.entrySet()) {
+        ReadWriteIOUtils.write(entry.getKey(), byteArrayOutputStream);
+        try (TIOStreamTransport tioStreamTransport =
+            new TIOStreamTransport(byteArrayOutputStream)) {
+          TProtocol protocol = new TBinaryProtocol(tioStreamTransport);
+          for (Entry<TSeriesPartitionSlot, Map<TTimePartitionSlot, List<TRegionReplicaSet>>>
+              entry1 : entry.getValue().entrySet()) {
+            entry1.getKey().write(protocol);
+            for (Entry<TTimePartitionSlot, List<TRegionReplicaSet>> entry2 :
+                entry1.getValue().entrySet()) {
+              entry2.getKey().write(protocol);
+              ReadWriteIOUtils.write(entry2.getValue().size(), byteArrayOutputStream);
+              entry2
+                  .getValue()
+                  .forEach(
+                      x -> {
+                        try {
+                          x.write(protocol);
+                        } catch (TException e) {
+                          throw new RuntimeException(e);
+                        }
+                      });
+            }
+          }
+        }
+      }
+      byte[] toArray = byteArrayOutputStream.toByteArray();
+      buffer.putInt(toArray.length);
+      buffer.put(toArray);
+    }
   }
 
-  public void deserialize(ByteBuffer buffer) {
-    // TODO: Deserialize DataPartition
+  public void deserialize(ByteBuffer buffer) throws TException, IOException {

Review Comment:
   Don't forget to add tests for deserialize~



-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap
+      serializeRegionMap(byteBuffer);
+      // third, put schemaPartition
+      schemaPartition.serialize(byteBuffer);
+      // then, put dataPartition
+      dataPartition.serialize(byteBuffer);
+      // write to file
+      try (FileOutputStream fileOutputStream = new FileOutputStream(tmpFile);
+          FileChannel fileChannel = fileOutputStream.getChannel()) {
+        fileChannel.write(byteBuffer);
+      }
+      // rename file
+      return tmpFile.renameTo(snapshotFile);
+    } finally {
+      unlockAllRead();
+      byteBuffer.clear();
+      // with or without success, delete temporary files anyway
+      tmpFile.delete();

Review Comment:
   `FileChannel` and `FileOutputStream` are wrapped in try clause, so the resources are managed by trys and do not need to be released manually  



-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {

Review Comment:
   fixed



##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap
+      serializeRegionMap(byteBuffer);
+      // third, put schemaPartition
+      schemaPartition.serialize(byteBuffer);
+      // then, put dataPartition
+      dataPartition.serialize(byteBuffer);
+      // write to file
+      try (FileOutputStream fileOutputStream = new FileOutputStream(tmpFile);
+          FileChannel fileChannel = fileOutputStream.getChannel()) {
+        fileChannel.write(byteBuffer);
+      }
+      // rename file
+      return tmpFile.renameTo(snapshotFile);
+    } finally {
+      unlockAllRead();
+      byteBuffer.clear();
+      // with or without success, delete temporary files anyway
+      tmpFile.delete();
+    }
+  }
+
+  public void loadSnapshot(File snapshotFile) throws TException, IOException {

Review Comment:
   fixed



-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());

Review Comment:
   fixed.



-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +310,114 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotFile) throws TException, IOException {
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());

Review Comment:
   FileName is controlled by executor,see [#5816](https://github.com/apache/iotdb/pull/5816)



-- 
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] cigarl commented on a diff in pull request #5807: [IOTDB-3097] PartitionInfo snapshot interface

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


##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/PartitionInfo.java:
##########
@@ -298,15 +321,142 @@ public List<TRegionReplicaSet> getRegionReplicaSets(List<TConsensusGroupId> grou
     return result;
   }
 
-  public void serialize(ByteBuffer buffer) {
-    // TODO: Serialize PartitionInfo
+  public boolean takeSnapshot(File snapshotDir) throws TException, IOException {
+
+    File snapshotFile = new File(snapshotDir, snapshotFileName);
+    if (snapshotFile.exists() && snapshotFile.isFile()) {
+      LOGGER.error(
+          "Failed to take snapshot, because snapshot file [{}] is already exist.",
+          snapshotFile.getAbsolutePath());
+      return false;
+    }
+
+    File tmpFile = new File(snapshotFile.getAbsolutePath() + "-" + UUID.randomUUID());
+
+    lockAllRead();
+    ByteBuffer byteBuffer = ByteBuffer.allocate(bufferSize);
+    try {
+      // first, put nextRegionGroupId
+      byteBuffer.putInt(nextRegionGroupId.get());
+      // second, put regionMap

Review Comment:
   fixed



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