You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/11/04 05:16:58 UTC

[GitHub] [ratis] szetszwo commented on a diff in pull request #774: RATIS-1734. Allow StateMachine to decide snapshot install path

szetszwo commented on code in PR #774:
URL: https://github.com/apache/ratis/pull/774#discussion_r1013586720


##########
ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java:
##########
@@ -50,11 +50,11 @@ public class FileChunkReader implements Closeable {
    * @param directory the directory where the file is stored.
    * @throws IOException if it failed to open the file.
    */
-  public FileChunkReader(FileInfo info, RaftStorageDirectory directory) throws IOException {
+  public FileChunkReader(FileInfo info, File directory) throws IOException {
     this.info = info;
     this.relativePath = Optional.of(info.getPath())
         .filter(Path::isAbsolute)
-        .map(p -> directory.getRoot().toPath().relativize(p))
+        .map(p -> directory.toPath().relativize(p))

Review Comment:
   `directory` becomes the snapshot dir but not the root dir.  Then, this becomes an incompatible change since  the new client/server may not able to talk to the old server/client.
   
   We have to use root dir although this is a good change.



##########
ratis-server-api/src/main/java/org/apache/ratis/statemachine/StateMachine.java:
##########
@@ -262,6 +267,27 @@ default CompletableFuture<TermIndex> notifyInstallSnapshotFromLeader(
     }
   }
 
+  /**
+   * An optional API for statemachine snapshot storage and install
+   * The method in this interface will be invoked when leader sends a snapshot to a follower
+   */
+  interface StorageApi {
+    StorageApi DEFAULT = null;
+
+    /**
+     * The directory of statemachine snapshot storage. It should be inside root directory.
+     */
+    File getSnapshotDir();
+
+    /**
+     * A temp directory
+     */
+    File getTmpDir();

Review Comment:
   Let's move these two methods to `StateMachineStorage`.



-- 
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: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org