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


##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -83,9 +83,10 @@ public void installSnapshot(StateMachine stateMachine,
       }
 
       String fileName = chunk.getFilename(); // this is relative to the root 
dir
-      // TODO: assumes flat layout inside SM dir
-      File tmpSnapshotFile = new File(tmpDir,
-          new File(dir.getRoot(), fileName).getName());
+      String fileNameToStateMachineDir = fileName.substring(
+              (dir.STATE_MACHINE_DIR_NAME.length()));

Review Comment:
   We need handle the path separator.  How about using Path to relativize it?
   
   ```
   @@ -74,6 +74,7 @@ public class SnapshotManager {
        // TODO: Make sure that subsequent requests for the same 
installSnapshot are coming in order,
        // and are not lost when whole request cycle is done. Check requestId 
and requestIndex here
    
   +    final Path stateMachineDir = dir.getStateMachineDir().toPath();
        for (FileChunkProto chunk : snapshotChunkRequest.getFileChunksList()) {
          SnapshotInfo pi = stateMachine.getLatestSnapshot();
          if (pi != null && pi.getTermIndex().getIndex() >= lastIncludedIndex) {
   @@ -83,9 +84,9 @@ public class SnapshotManager {
          }
    
          String fileName = chunk.getFilename(); // this is relative to the 
root dir
   -      // TODO: assumes flat layout inside SM dir
   -      File tmpSnapshotFile = new File(tmpDir,
   -          new File(dir.getRoot(), fileName).getName());
   +      final Path relative = stateMachineDir.relativize(new 
File(dir.getRoot(), fileName).toPath());
   +      final File tmpSnapshotFile = new File(tmpDir, relative.toString());
   +      FileUtils.createDirectories(tmpSnapshotFile);
   ```



##########
ratis-proto/src/main/proto/Grpc.proto:
##########
@@ -44,7 +44,7 @@ service RaftServerProtocolService {
       returns(stream ratis.common.AppendEntriesReplyProto) {}
 
   rpc installSnapshot(stream ratis.common.InstallSnapshotRequestProto)
-      returns(ratis.common.InstallSnapshotReplyProto) {}
+      returns(stream ratis.common.InstallSnapshotReplyProto) {}

Review Comment:
   Is this an incompatible change?  If yes, we should document 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to