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/09/14 05:21:10 UTC

[GitHub] [iotdb] SzyWilliam opened a new pull request, #7303: [IOTDB-4403] Add retry interface for IStateMachine

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

   To guarantee consistency in RatisConsensus, we should retry some failed apply write result.
   
   refer to design doc: https://apache-iotdb.feishu.cn/docx/doxcnj0pH6Jm9pFWEEexa4opdXb


-- 
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] JackieTien97 merged pull request #7303: [IOTDB-4403] Add retry interface for IStateMachine

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


-- 
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] JackieTien97 commented on a diff in pull request #7303: [IOTDB-4403] Add retry interface for IStateMachine

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


##########
consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java:
##########
@@ -92,6 +92,43 @@ default List<Path> getSnapshotFiles(File latestSnapshotRootDir) {
     return Utils.listAllRegularFilesRecursively(latestSnapshotRootDir);
   }
 
+  /**
+   * To guarantee the statemachine replication property, when {@link #write(IConsensusRequest)}
+   * failed in this statemachine, Upper consensus implementation like RatisConsensus may choose to
+   * retry the operation until it succeed.
+   */
+  interface RetryPolicy {
+
+    /** Given the last write result, should we retry? */
+    default boolean shouldRetry(TSStatus writeResult) {
+      return false;
+    }
+
+    /**
+     * Use the latest write result to update final write result
+     *
+     * @param previousResult previous write result
+     * @param retryResult latest write result
+     * @return the aggregated result upon current retry
+     */
+    default TSStatus updateResult(TSStatus previousResult, TSStatus retryResult) {
+      return retryResult;
+    }
+
+    /**
+     * sleep time before the next retry
+     *
+     * @return time in millis
+     */
+    default long getSleepTime() {
+      return 100;
+    };
+  }
+
+  default RetryPolicy retryPolicy() {
+    return (IStateMachine.RetryPolicy) this;
+  }

Review Comment:
   ```suggestion
   ```



-- 
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] SzyWilliam commented on a diff in pull request #7303: [IOTDB-4403] Add retry interface for IStateMachine

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


##########
consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java:
##########
@@ -92,6 +92,43 @@ default List<Path> getSnapshotFiles(File latestSnapshotRootDir) {
     return Utils.listAllRegularFilesRecursively(latestSnapshotRootDir);
   }
 
+  /**
+   * To guarantee the statemachine replication property, when {@link #write(IConsensusRequest)}
+   * failed in this statemachine, Upper consensus implementation like RatisConsensus may choose to
+   * retry the operation until it succeed.
+   */
+  interface RetryPolicy {
+
+    /** Given the last write result, should we retry? */
+    default boolean shouldRetry(TSStatus writeResult) {
+      return false;
+    }
+
+    /**
+     * Use the latest write result to update final write result
+     *
+     * @param previousResult previous write result
+     * @param retryResult latest write result
+     * @return the aggregated result upon current retry
+     */
+    default TSStatus updateResult(TSStatus previousResult, TSStatus retryResult) {
+      return retryResult;
+    }
+
+    /**
+     * sleep time before the next retry
+     *
+     * @return time in millis
+     */
+    default long getSleepTime() {
+      return 100;
+    };
+  }
+
+  default RetryPolicy retryPolicy() {
+    return (IStateMachine.RetryPolicy) this;
+  }

Review Comment:
   done



-- 
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] JackieTien97 commented on a diff in pull request #7303: [IOTDB-4403] Add retry interface for IStateMachine

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


##########
consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java:
##########
@@ -92,6 +92,38 @@ default List<Path> getSnapshotFiles(File latestSnapshotRootDir) {
     return Utils.listAllRegularFilesRecursively(latestSnapshotRootDir);
   }
 
+  /**
+   * To guarantee the statemachine replication property, when {@link #write(IConsensusRequest)}
+   * failed in this statemachine, Upper consensus implementation like RatisConsensus may choose to
+   * retry the operation until it exceeds.
+   */
+  interface RetryPolicy {
+
+    /** Given the last write result, should we retry? */
+    default boolean shouldRetry(TSStatus writeResult) {
+      return false;
+    }
+
+    /**
+     * Use the latest write result to update final write result
+     *
+     * @param retryResult the latest write result
+     * @return the aggregated result upon current retry
+     */
+    default TSStatus updateResult(TSStatus retryResult) {
+      return retryResult;
+    }
+
+    /**
+     * sleep time before the next retry
+     *
+     * @return time in millis
+     */
+    default long getSleepTime() {
+      return 0;

Review Comment:
   give it a default value intstead of 0



-- 
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] SzyWilliam commented on a diff in pull request #7303: [IOTDB-4403] Add retry interface for IStateMachine

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


##########
consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java:
##########
@@ -92,6 +92,38 @@ default List<Path> getSnapshotFiles(File latestSnapshotRootDir) {
     return Utils.listAllRegularFilesRecursively(latestSnapshotRootDir);
   }
 
+  /**
+   * To guarantee the statemachine replication property, when {@link #write(IConsensusRequest)}
+   * failed in this statemachine, Upper consensus implementation like RatisConsensus may choose to
+   * retry the operation until it exceeds.
+   */
+  interface RetryPolicy {
+
+    /** Given the last write result, should we retry? */
+    default boolean shouldRetry(TSStatus writeResult) {
+      return false;
+    }
+
+    /**
+     * Use the latest write result to update final write result
+     *
+     * @param retryResult the latest write result
+     * @return the aggregated result upon current retry
+     */
+    default TSStatus updateResult(TSStatus retryResult) {
+      return retryResult;
+    }
+
+    /**
+     * sleep time before the next retry
+     *
+     * @return time in millis
+     */
+    default long getSleepTime() {
+      return 0;

Review Comment:
   done



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