You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/11/11 14:33:21 UTC
[GitHub] [ratis] SzyWilliam opened a new pull request, #780: RATIS-1734. Separate state machine snapshot install path
SzyWilliam opened a new pull request, #780:
URL: https://github.com/apache/ratis/pull/780
The same changes as proposed in https://github.com/apache/ratis/pull/774. This PR is for master branch.
Jira issue see https://issues.apache.org/jira/browse/RATIS-1734
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #780: RATIS-1734. Separate state machine snapshot install path
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #780:
URL: https://github.com/apache/ratis/pull/780#discussion_r1020631136
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java:
##########
@@ -116,7 +116,14 @@ class ServerState {
this.raftStorage = MemoizedCheckedSupplier.valueOf(
() -> StorageImplUtils.initRaftStorage(storageDirName, option, prop));
- this.snapshotManager = StorageImplUtils.newSnapshotManager(id);
+ this.snapshotManager = StorageImplUtils.newSnapshotManager(id, () -> {
+ try {
+ return raftStorage.get().getStorageDir();
+ } catch (IOException e) {
+ throw new IllegalStateException("RaftStorageDirectory is not properly initialized " + storageDirName, e);
+ }
+ },
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #780: RATIS-1734. Separate state machine snapshot install path
Posted by GitBox <gi...@apache.org>.
szetszwo merged PR #780:
URL: https://github.com/apache/ratis/pull/780
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #780: RATIS-1734. Separate state machine snapshot install path
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #780:
URL: https://github.com/apache/ratis/pull/780#discussion_r1020631180
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -55,10 +58,22 @@ public class SnapshotManager {
private static final String TMP = ".tmp";
private final RaftPeerId selfId;
+
+ private final Supplier<File> snapshotDir;
+ private final Supplier<File> tmp;
+ private final Function<FileChunkProto, String> getRelativePath;
private final Supplier<MessageDigest> digester = JavaUtils.memoize(MD5Hash::getDigester);
- SnapshotManager(RaftPeerId selfId) {
+ SnapshotManager(RaftPeerId selfId, Supplier<RaftStorageDirectory> dir, StateMachineStorage smStorage) {
this.selfId = selfId;
+ this.snapshotDir = MemoizedSupplier.valueOf(
+ () -> Optional.ofNullable(smStorage.getSnapshotDir()).orElseGet(() -> dir.get().getStateMachineDir()));
+ this.tmp = MemoizedSupplier.valueOf(
+ () -> Optional.ofNullable(smStorage.getTmpDir()).orElseGet(() -> dir.get().getTmpDir()));
+
+ this.getRelativePath =
+ c -> dir.get().getStateMachineDir().toPath()
+ .relativize(new File(dir.get().getRoot(), c.getFilename()).toPath()).toString();
Review Comment:
Got it. 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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #780: RATIS-1734. Separate state machine snapshot install path
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #780:
URL: https://github.com/apache/ratis/pull/780#discussion_r1020457404
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -55,10 +58,22 @@ public class SnapshotManager {
private static final String TMP = ".tmp";
private final RaftPeerId selfId;
+
+ private final Supplier<File> snapshotDir;
+ private final Supplier<File> tmp;
+ private final Function<FileChunkProto, String> getRelativePath;
private final Supplier<MessageDigest> digester = JavaUtils.memoize(MD5Hash::getDigester);
- SnapshotManager(RaftPeerId selfId) {
+ SnapshotManager(RaftPeerId selfId, Supplier<RaftStorageDirectory> dir, StateMachineStorage smStorage) {
this.selfId = selfId;
+ this.snapshotDir = MemoizedSupplier.valueOf(
+ () -> Optional.ofNullable(smStorage.getSnapshotDir()).orElseGet(() -> dir.get().getStateMachineDir()));
+ this.tmp = MemoizedSupplier.valueOf(
+ () -> Optional.ofNullable(smStorage.getTmpDir()).orElseGet(() -> dir.get().getTmpDir()));
+
+ this.getRelativePath =
+ c -> dir.get().getStateMachineDir().toPath()
+ .relativize(new File(dir.get().getRoot(), c.getFilename()).toPath()).toString();
Review Comment:
Use a MemoizedSupplier so that the sm dir path don't have to be computed in every call:
```java
final Supplier<Path> smDir = MemoizedSupplier.valueOf(() -> dir.get().getStateMachineDir().toPath());
this.getRelativePath = c -> smDir.get().relativize(
new File(dir.get().getRoot(), c.getFilename()).toPath()).toString();
```
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java:
##########
@@ -116,7 +116,14 @@ class ServerState {
this.raftStorage = MemoizedCheckedSupplier.valueOf(
() -> StorageImplUtils.initRaftStorage(storageDirName, option, prop));
- this.snapshotManager = StorageImplUtils.newSnapshotManager(id);
+ this.snapshotManager = StorageImplUtils.newSnapshotManager(id, () -> {
+ try {
+ return raftStorage.get().getStorageDir();
+ } catch (IOException e) {
+ throw new IllegalStateException("RaftStorageDirectory is not properly initialized " + storageDirName, e);
+ }
+ },
Review Comment:
Use `() -> getStorage().getStorageDir()`.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org