[GitHub] [ratis] szetszwo commented on a diff in pull request #780: RATIS-1734. Separate state machine snapshot install path

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

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


##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -55,10 +58,22 @@ public class SnapshotManager {
   private static final String TMP = ".tmp";
 
   private final RaftPeerId selfId;
+
+  private final Supplier<File> snapshotDir;
+  private final Supplier<File> tmp;
+  private final Function<FileChunkProto, String> getRelativePath;
   private final Supplier<MessageDigest> digester = JavaUtils.memoize(MD5Hash::getDigester);
 
-  SnapshotManager(RaftPeerId selfId) {
+  SnapshotManager(RaftPeerId selfId, Supplier<RaftStorageDirectory> dir, StateMachineStorage smStorage) {
     this.selfId = selfId;
+    this.snapshotDir = MemoizedSupplier.valueOf(
+        () -> Optional.ofNullable(smStorage.getSnapshotDir()).orElseGet(() -> dir.get().getStateMachineDir()));
+    this.tmp = MemoizedSupplier.valueOf(
+        () -> Optional.ofNullable(smStorage.getTmpDir()).orElseGet(() -> dir.get().getTmpDir()));
+
+    this.getRelativePath =
+        c -> dir.get().getStateMachineDir().toPath()
+            .relativize(new File(dir.get().getRoot(), c.getFilename()).toPath()).toString();

Review Comment:
   Use a MemoizedSupplier so that the sm dir path don't have to be computed in every call:
   ```java
       final Supplier<Path> smDir = MemoizedSupplier.valueOf(() -> dir.get().getStateMachineDir().toPath());
       this.getRelativePath = c -> smDir.get().relativize(
           new File(dir.get().getRoot(), c.getFilename()).toPath()).toString();
   ```



##########
ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java:
##########
@@ -116,7 +116,14 @@ class ServerState {
     this.raftStorage = MemoizedCheckedSupplier.valueOf(
         () -> StorageImplUtils.initRaftStorage(storageDirName, option, prop));
 
-    this.snapshotManager = StorageImplUtils.newSnapshotManager(id);
+    this.snapshotManager = StorageImplUtils.newSnapshotManager(id, () -> {
+          try {
+            return raftStorage.get().getStorageDir();
+          } catch (IOException e) {
+            throw new IllegalStateException("RaftStorageDirectory is not properly initialized " + storageDirName, e);
+          }
+        },

Review Comment:
   Use `() -> getStorage().getStorageDir()`.



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