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


##########
ratis-client/src/main/java/org/apache/ratis/client/api/SnapshotManagementApi.java:
##########
@@ -28,5 +28,15 @@
 public interface SnapshotManagementApi {
 
   /** trigger create snapshot file. */
-  RaftClientReply create(long timeoutMs) throws IOException;
+  default RaftClientReply create(long timeoutMs) throws IOException {
+    return create(0, timeoutMs);
+  }
+
+  /** trigger create snapshot file. If forced, ignore the creation gap. */
+  default RaftClientReply create(boolean force, long timeoutMs) throws 
IOException {
+    return create(1, timeoutMs);
+  }
+
+  /** trigger create snapshot file if the number of newly applied logs since 
last snapshot exceeds creationGap. */

Review Comment:
   Let's add javadoc for `creationGap` and `@return`:
   ```java
     /**
      * Trigger to create a snapshot.
      *
      * @param creationGap When (creationGap > 0) and (astAppliedIndex - 
lastSnapshotIndex < creationGap),
      *                    return lastSnapshotIndex; otherwise, take a new 
snapshot and then return its index.
      *                    When creationGap == 0, use the server configured 
value as the creationGap.
      * @return a reply.  When {@link RaftClientReply#isSuccess()} is true,
      *         {@link RaftClientReply#getLogIndex()} is the snapshot index 
fulfilling the operation.
      */
   ```



##########
ratis-client/src/main/java/org/apache/ratis/client/api/SnapshotManagementApi.java:
##########
@@ -28,5 +28,15 @@
 public interface SnapshotManagementApi {
 
   /** trigger create snapshot file. */
-  RaftClientReply create(long timeoutMs) throws IOException;
+  default RaftClientReply create(long timeoutMs) throws IOException {
+    return create(0, timeoutMs);
+  }
+
+  /** trigger create snapshot file. If forced, ignore the creation gap. */
+  default RaftClientReply create(boolean force, long timeoutMs) throws 
IOException {
+    return create(1, timeoutMs);
+  }

Review Comment:
   It should check the `force` parameter:
   ```java
     /** The same as create(force? 1 : 0, timeoutMs). */
     default RaftClientReply create(boolean force, long timeoutMs) throws 
IOException {
       return create(force? 1 : 0, timeoutMs);
     }
   ```



##########
ratis-client/src/main/java/org/apache/ratis/client/api/SnapshotManagementApi.java:
##########
@@ -28,5 +28,15 @@
 public interface SnapshotManagementApi {
 
   /** trigger create snapshot file. */

Review Comment:
   Let's update the javadoc:
   ```java
   /** The same as create(0, timeoutMs). */
   ```



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