[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #316: WIP:HDDS-2680. Fix updating lastAppliedIndex in OzoneManagerStateMachine.

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

bharatviswa504 commented on a change in pull request #316: WIP:HDDS-2680. Fix updating lastAppliedIndex in OzoneManagerStateMachine.
URL: https://github.com/apache/hadoop-ozone/pull/316#discussion_r354620359
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java
 ##########
 @@ -349,22 +357,51 @@ private Message runCommand(OMRequest request, long trxLogIndex) {
   /**
    * Update lastAppliedIndex term and it's corresponding term in the
    * stateMachine.
+   * @param flushedEpochs
+   */
+  public void updateLastAppliedIndex(List<Long> flushedEpochs) {
+    Preconditions.checkArgument(flushedEpochs.size() > 0);
+    updateLastAppliedIndex(flushedEpochs.get(flushedEpochs.size() -1),
+        -1L, flushedEpochs, true);
+  }
+
+  /**
+   * Update State machine lastAppliedTermIndex.
    * @param lastFlushedIndex
+   * @param currentTerm
+   * @param flushedEpochs - list of ratis transactions flushed to DB. If it
+   * is just one index and term, this can be set to null.
+   * @param checkMap - if true check applyTransactionMap, ratisTransaction
+   * Map and update lastAppliedTermIndex accordingly, else check
+   * lastAppliedTermIndex and update it.
    */
-  public synchronized void updateLastAppliedIndex(long lastFlushedIndex) {
-    Long appliedTerm = null;
-    long appliedIndex = -1;
-    for(long i = getLastAppliedTermIndex().getIndex() + 1;
-        i <= lastFlushedIndex; i++) {
-      final Long removed = applyTransactionMap.remove(i);
-      if (removed == null) {
-        break;
+  private synchronized void updateLastAppliedIndex(long lastFlushedIndex,
+      long currentTerm, List<Long> flushedEpochs, boolean checkMap) {
+    if (checkMap) {
+      Long appliedTerm = null;
+      long appliedIndex = -1;
+      for (long i = getLastAppliedTermIndex().getIndex() + 1; ; i++) {
+        if (flushedEpochs.contains(i)) {
+          appliedIndex = i;
+          final Long removed = applyTransactionMap.remove(i);
+          appliedTerm = removed;
+        } else if (ratisTransactionMap.containsKey(i)) {
+          final Long removed = ratisTransactionMap.remove(i);
+          appliedTerm = removed;
+          appliedIndex = i;
+        } else {
+          break;
+        }
 
 Review comment:
   I think with multiple executors this can certainly possible. Thanks for catching it. Fixed to handle this issue in latest commits.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org