szetszwo commented on code in PR #892:
URL: https://github.com/apache/ratis/pull/892#discussion_r1253170257
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java:
##########
@@ -130,13 +131,14 @@ RaftStorageImpl run() throws IOException {
}
}
+ @SuppressWarnings("java:S1181")
Review Comment:
Could you add a comment at the end for this and the other SuppressWarnings?
```java
@SuppressWarnings("java:S1181") // catch Throwable
```
```java
@SuppressWarnings("java:S2095") // return Closable
```
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java:
##########
@@ -112,8 +113,8 @@ private StorageState analyzeAndRecoverStorage(boolean
toLock) throws IOException
if (!f.exists()) {
throw new FileNotFoundException("Metadata file " + f + " does not
exists.");
}
- metaFile = new RaftStorageMetadataFileImpl(f);
- final RaftStorageMetadata metadata = metaFile.getMetadata();
+ metaFile.set(new RaftStorageMetadataFileImpl(f));
+ final RaftStorageMetadata metadata = metaFile.get().getMetadata();
Review Comment:
Similarly, this becomes a single llne
```java
final RaftStorageMetadata metadata = metaFile.set(f).getMetadata();
```
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java:
##########
@@ -39,7 +40,7 @@ public class RaftStorageImpl implements RaftStorage {
private final StartupOption startupOption;
private final CorruptionPolicy logCorruptionPolicy;
private volatile StorageState state = StorageState.UNINITIALIZED;
- private volatile RaftStorageMetadataFileImpl metaFile;
+ private final AtomicReference<RaftStorageMetadataFileImpl> metaFile = new
AtomicReference<>();
Review Comment:
Let's create a class:
```java
static class MetaFile {
private final AtomicReference<RaftStorageMetadataFileImpl> ref = new
AtomicReference<>();
RaftStorageMetadataFile get() {
return ref.get();
}
RaftStorageMetadataFile set(File file) {
final RaftStorageMetadataFileImpl impl = new
RaftStorageMetadataFileImpl(file);
ref.set(impl);
return impl;
}
}
```
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java:
##########
@@ -91,9 +92,9 @@ public CorruptionPolicy getLogCorruptionPolicy() {
private void format() throws IOException {
storageDir.clearDirectory();
- metaFile = new RaftStorageMetadataFileImpl(storageDir.getMetaFile());
- metaFile.persist(RaftStorageMetadata.getDefault());
- LOG.info("Storage directory " + storageDir.getRoot() + " has been
successfully formatted.");
+ metaFile.set(new RaftStorageMetadataFileImpl(storageDir.getMetaFile()));
+ metaFile.get().persist(RaftStorageMetadata.getDefault());
Review Comment:
After created a class, this becomes a single llne
```java
metaFile.set(storageDir.getMetaFile()).persist(RaftStorageMetadata.getDefault());
```
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java:
##########
@@ -150,7 +152,7 @@ private RaftStorageImpl format() throws IOException {
}
throw new IOException("Failed to FORMAT a new storage dir for " +
storageDirName + " from " + dirsInConf);
}
-
Review Comment:
Please keep this empty line.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
