[GitHub] [iotdb] JackieTien97 commented on a diff in pull request #7450: [IOTDB-4541] Improve: cache latest snapshot info for ratis leader

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

JackieTien97 commented on code in PR #7450:
URL: https://github.com/apache/iotdb/pull/7450#discussion_r981051416


##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/SnapshotStorage.java:
##########
@@ -135,6 +137,28 @@ public SnapshotInfo getLatestSnapshot() {
         fileInfos, snapshotTermIndex.getTerm(), snapshotTermIndex.getIndex());
   }
 
+  /*
+  Snapshot cache will be updated upon:
+  1. initialize, when RaftServer first starts
+  2. reinitialize, when RaftServer resumes from pause. Leader may install a new snapshot during pause.
+  3. takeSnapshot, when RaftServer takes a new snapshot
+  4. loadSnapshot, when RaftServer is asked to load a snapshot

Review Comment:
   update the java doc, only need to update cache at the last step of `takeSnapshot` and first step of `loadSnapshot`. `loadSnapshot` will be called by `initialize` and `reinitialize`.



##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/SnapshotStorage.java:
##########
@@ -135,6 +137,28 @@ public SnapshotInfo getLatestSnapshot() {
         fileInfos, snapshotTermIndex.getTerm(), snapshotTermIndex.getIndex());
   }
 
+  /*
+  Snapshot cache will be updated upon:
+  1. initialize, when RaftServer first starts
+  2. reinitialize, when RaftServer resumes from pause. Leader may install a new snapshot during pause.
+  3. takeSnapshot, when RaftServer takes a new snapshot
+  4. loadSnapshot, when RaftServer is asked to load a snapshot
+   */
+  void updateSnapshotCache() {
+    snapshotCacheGuard.writeLock().lock();
+    currentSnapshot = findLatestSnapshot();
+    snapshotCacheGuard.writeLock().unlock();
+  }
+
+  @Override
+  public SnapshotInfo getLatestSnapshot() {
+    SnapshotInfo latestSnapshot;
+    snapshotCacheGuard.readLock().lock();
+    latestSnapshot = currentSnapshot;
+    snapshotCacheGuard.readLock().unlock();

Review Comment:
   same as above.



##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/SnapshotStorage.java:
##########
@@ -135,6 +137,28 @@ public SnapshotInfo getLatestSnapshot() {
         fileInfos, snapshotTermIndex.getTerm(), snapshotTermIndex.getIndex());
   }
 
+  /*
+  Snapshot cache will be updated upon:
+  1. initialize, when RaftServer first starts
+  2. reinitialize, when RaftServer resumes from pause. Leader may install a new snapshot during pause.
+  3. takeSnapshot, when RaftServer takes a new snapshot
+  4. loadSnapshot, when RaftServer is asked to load a snapshot
+   */
+  void updateSnapshotCache() {
+    snapshotCacheGuard.writeLock().lock();
+    currentSnapshot = findLatestSnapshot();
+    snapshotCacheGuard.writeLock().unlock();

Review Comment:
   ```suggestion
       try {
         currentSnapshot = findLatestSnapshot();
       } finally {
         snapshotCacheGuard.writeLock().unlock();
       }
   ```



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