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/27 08:28:43 UTC
[GitHub] [iotdb] SzyWilliam opened a new pull request, #7450: [IOTDB-4541] Improve: cache latest snapshot info for ratis leader
SzyWilliam opened a new pull request, #7450:
URL: https://github.com/apache/iotdb/pull/7450
Ratis Leader will query latest snapshot info every time it sends a AppendEntries. We should cache the latest snapshot and return directly, instead of query statemachine and scanning disk.
--
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 #7450: [IOTDB-4541] Improve: cache latest snapshot info for ratis leader
Posted by GitBox <gi...@apache.org>.
JackieTien97 merged PR #7450:
URL: https://github.com/apache/iotdb/pull/7450
--
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 #7450: [IOTDB-4541] Improve: cache latest snapshot info for ratis leader
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #7450:
URL: https://github.com/apache/iotdb/pull/7450#discussion_r981134063
##########
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:
done
##########
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:
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 #7450: [IOTDB-4541] Improve: cache latest snapshot info for ratis leader
Posted by 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
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #7450: [IOTDB-4541] Improve: cache latest snapshot info for ratis leader
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #7450:
URL: https://github.com/apache/iotdb/pull/7450#discussion_r981134227
##########
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:
updated
--
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