From e9a51de865b51def0caa72de9321603df97920a4 Mon Sep 17 00:00:00 2001 From: dragonyliu Date: Wed, 1 Jun 2022 21:10:30 +0800 Subject: [PATCH 1/7] Change the constructor of RaftStorageImpl for forward compatibility --- .../org/apache/ratis/server/storage/RaftStorageImpl.java | 5 +++++ .../apache/ratis/server/storage/RaftStorageTestUtils.java | 2 +- .../org/apache/ratis/server/storage/TestRaftStorage.java | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java index 0bbf902e27..d0500d6981 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java @@ -19,6 +19,7 @@ import org.apache.ratis.proto.RaftProtos.LogEntryProto; import org.apache.ratis.server.RaftConfiguration; +import org.apache.ratis.server.RaftServerConfigKeys; import org.apache.ratis.server.RaftServerConfigKeys.Log.CorruptionPolicy; import org.apache.ratis.server.raftlog.LogProtoUtils; import org.apache.ratis.server.storage.RaftStorageDirectoryImpl.StorageState; @@ -47,6 +48,10 @@ public enum StartupOption { private final CorruptionPolicy logCorruptionPolicy; private volatile RaftStorageMetadataFileImpl metaFile; + public RaftStorageImpl(File dir, CorruptionPolicy logCorruptionPolicy) throws IOException { + this(dir, logCorruptionPolicy, null, RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()); + } + public RaftStorageImpl(File dir, CorruptionPolicy logCorruptionPolicy, long storageFeeSpaceMin) throws IOException { this(dir, logCorruptionPolicy, null, storageFeeSpaceMin); diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java index d8c48ff672..9f192e4619 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java @@ -33,7 +33,7 @@ public interface RaftStorageTestUtils { static RaftStorage newRaftStorage(File dir) throws IOException { - return new RaftStorageImpl(dir, null, 0L); + return new RaftStorageImpl(dir, null); } static String getLogFlushTimeMetric(String memberId) { diff --git a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java index 89e2ccece8..69156b57e1 100644 --- a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java +++ b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java @@ -47,7 +47,7 @@ */ public class TestRaftStorage extends BaseTest { static RaftStorageImpl newRaftStorage(File dir) throws IOException { - return new RaftStorageImpl(dir, null, 0); + return new RaftStorageImpl(dir, null); } private File storageDir; From f2c2ddedb0487824cc627e300569d313ba9c2f01 Mon Sep 17 00:00:00 2001 From: dragonyliu Date: Tue, 7 Jun 2022 13:11:56 +0800 Subject: [PATCH 2/7] Add a builder to the RaftStorageImpl --- .../apache/ratis/server/impl/ServerState.java | 7 +++- .../ratis/server/storage/RaftStorageImpl.java | 40 ++++++++++++++++--- .../server/storage/RaftStorageTestUtils.java | 6 ++- .../ratis/server/storage/TestRaftStorage.java | 12 +++++- 4 files changed, 55 insertions(+), 10 deletions(-) diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java index b66ba7336a..69660627d0 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java @@ -112,8 +112,11 @@ class ServerState implements Closeable { // use full uuid string to create a subdirectory File dir = chooseStorageDir(directories, group.getGroupId().getUuid().toString()); try { - storage = new RaftStorageImpl(dir, RaftServerConfigKeys.Log.corruptionPolicy(prop), - RaftServerConfigKeys.storageFreeSpaceMin(prop).getSize()); + storage = RaftStorageImpl.newBuilder() + .setDir(dir) + .setLogCorruptionPolicy(RaftServerConfigKeys.Log.corruptionPolicy(prop)) + .setStorageFeeSpaceMin(RaftServerConfigKeys.storageFreeSpaceMin(prop).getSize()) + .build(); storageFound = true; break; } catch (IOException e) { diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java index d0500d6981..314b4bf786 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java @@ -19,7 +19,6 @@ import org.apache.ratis.proto.RaftProtos.LogEntryProto; import org.apache.ratis.server.RaftConfiguration; -import org.apache.ratis.server.RaftServerConfigKeys; import org.apache.ratis.server.RaftServerConfigKeys.Log.CorruptionPolicy; import org.apache.ratis.server.raftlog.LogProtoUtils; import org.apache.ratis.server.storage.RaftStorageDirectoryImpl.StorageState; @@ -42,16 +41,47 @@ public enum StartupOption { FORMAT } + public static Builder newBuilder() { + return new RaftStorageImpl.Builder(); + } + + public static class Builder { + private File dir; + private CorruptionPolicy logCorruptionPolicy; + private StartupOption option; + private long storageFeeSpaceMin; + + public Builder setDir(File dir) { + this.dir = dir; + return this; + } + + public Builder setLogCorruptionPolicy(CorruptionPolicy logCorruptionPolicy) { + this.logCorruptionPolicy = logCorruptionPolicy; + return this; + } + + public Builder setOption(StartupOption option) { + this.option = option; + return this; + } + + public Builder setStorageFeeSpaceMin(long storageFeeSpaceMin) { + this.storageFeeSpaceMin = storageFeeSpaceMin; + return this; + } + + public RaftStorageImpl build() throws IOException { + return new RaftStorageImpl(dir, logCorruptionPolicy, option, storageFeeSpaceMin); + } + } + // TODO support multiple storage directories private final RaftStorageDirectoryImpl storageDir; private final StorageState state; private final CorruptionPolicy logCorruptionPolicy; private volatile RaftStorageMetadataFileImpl metaFile; - public RaftStorageImpl(File dir, CorruptionPolicy logCorruptionPolicy) throws IOException { - this(dir, logCorruptionPolicy, null, RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()); - } - public RaftStorageImpl(File dir, CorruptionPolicy logCorruptionPolicy, long storageFeeSpaceMin) throws IOException { this(dir, logCorruptionPolicy, null, storageFeeSpaceMin); diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java index 9f192e4619..280a105e87 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java @@ -21,6 +21,7 @@ import static org.apache.ratis.server.metrics.SegmentedRaftLogMetrics.RATIS_LOG_WORKER_METRICS; import org.apache.ratis.metrics.RatisMetrics; +import org.apache.ratis.server.RaftServerConfigKeys; import org.apache.ratis.server.protocol.TermIndex; import org.apache.ratis.server.raftlog.LogProtoUtils; import org.apache.ratis.server.raftlog.RaftLogBase; @@ -33,7 +34,10 @@ public interface RaftStorageTestUtils { static RaftStorage newRaftStorage(File dir) throws IOException { - return new RaftStorageImpl(dir, null); + return RaftStorageImpl.newBuilder() + .setDir(dir) + .setStorageFeeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) + .build(); } static String getLogFlushTimeMetric(String memberId) { diff --git a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java index 69156b57e1..dbbd6df335 100644 --- a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java +++ b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java @@ -22,6 +22,7 @@ import org.apache.ratis.BaseTest; import org.apache.ratis.RaftTestUtil; import org.apache.ratis.protocol.RaftPeerId; +import org.apache.ratis.server.RaftServerConfigKeys; import org.apache.ratis.server.protocol.TermIndex; import org.apache.ratis.server.storage.RaftStorageDirectoryImpl.StorageState; import org.apache.ratis.statemachine.impl.SimpleStateMachineStorage; @@ -47,7 +48,10 @@ */ public class TestRaftStorage extends BaseTest { static RaftStorageImpl newRaftStorage(File dir) throws IOException { - return new RaftStorageImpl(dir, null); + return RaftStorageImpl.newBuilder() + .setDir(dir) + .setStorageFeeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) + .build(); } private File storageDir; @@ -65,7 +69,11 @@ public void tearDown() throws Exception { } static RaftStorageImpl formatRaftStorage(File dir) throws IOException { - return new RaftStorageImpl(dir, null, RaftStorageImpl.StartupOption.FORMAT, 0); + return RaftStorageImpl.newBuilder() + .setDir(dir) + .setOption(RaftStorageImpl.StartupOption.FORMAT) + .setStorageFeeSpaceMin(0) + .build(); } @Test From 6bf7e659d45156c435ea55980c4a94355d28422e Mon Sep 17 00:00:00 2001 From: dragonyliu Date: Mon, 13 Jun 2022 20:08:59 +0800 Subject: [PATCH 3/7] make code better --- .../ratis/server/storage/RaftStorage.java | 75 +++++++++++++++++++ .../apache/ratis/server/impl/ServerState.java | 6 +- .../ratis/server/storage/RaftStorageImpl.java | 40 ---------- .../server/storage/StorageImplUtils.java | 56 ++++++++++++++ .../server/storage/RaftStorageTestUtils.java | 7 +- .../ratis/server/storage/TestRaftStorage.java | 13 ++-- 6 files changed, 145 insertions(+), 52 deletions(-) create mode 100644 ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java diff --git a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java index d8f4fec398..5eb1186bb1 100644 --- a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java +++ b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java @@ -18,10 +18,16 @@ package org.apache.ratis.server.storage; import org.apache.ratis.server.RaftServerConfigKeys.Log.CorruptionPolicy; +import org.apache.ratis.util.IOUtils; +import org.apache.ratis.util.ReflectionUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.Closeable; +import java.io.File; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; /** The storage of a raft server. */ public interface RaftStorage extends Closeable { @@ -35,4 +41,73 @@ public interface RaftStorage extends Closeable { /** @return the corruption policy for raft log. */ CorruptionPolicy getLogCorruptionPolicy(); + + static Builder newBuilder() { + return new Builder(); + } + + enum StartupOption { + /** Format the storage. */ + FORMAT, + RECOVER + } + + class Builder { + + private static final Method NEW_RAFT_STORAGE_METHOD = initNewRaftStorageMethod(); + + private static Method initNewRaftStorageMethod() { + final String className = RaftStorage.class.getPackage().getName() + ".StorageImplUtils"; + //final String className = "org.apache.ratis.server.storage.RaftStorageImpl"; + final Class[] argClasses = { File.class, CorruptionPolicy.class, StartupOption.class, long.class }; + try { + final Class clazz = ReflectionUtils.getClassByName(className); + return clazz.getMethod("newRaftStorage", argClasses); + } catch (Exception e) { + throw new IllegalStateException("Failed to initNewRaftStorageMethod", e); + } + } + + private static RaftStorage newRaftStorage(File dir, CorruptionPolicy logCorruptionPolicy, + StartupOption option, long storageFreeSpaceMin) throws IOException { + try { + return (RaftStorage) NEW_RAFT_STORAGE_METHOD.invoke(null, + dir, logCorruptionPolicy, option, storageFreeSpaceMin); + } catch (IllegalAccessException e) { + throw new IllegalStateException("Failed to build " + dir, e); + } catch (InvocationTargetException e) { + throw IOUtils.asIOException(e.getCause()); + } + } + + + private File directory; + private CorruptionPolicy logCorruptionPolicy; + private StartupOption option; + private long storageFreeSpaceMin; + + public Builder setDirectory(File directory) { + this.directory = directory; + return this; + } + + public Builder setLogCorruptionPolicy(CorruptionPolicy logCorruptionPolicy) { + this.logCorruptionPolicy = logCorruptionPolicy; + return this; + } + + public Builder setOption(StartupOption option) { + this.option = option; + return this; + } + + public Builder setStorageFreeSpaceMin(long storageFreeSpaceMin) { + this.storageFreeSpaceMin = storageFreeSpaceMin; + return this; + } + + public RaftStorage build() throws IOException { + return newRaftStorage(directory, logCorruptionPolicy, option, storageFreeSpaceMin); + } + } } diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java index 69660627d0..8de0321f21 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java @@ -112,10 +112,10 @@ class ServerState implements Closeable { // use full uuid string to create a subdirectory File dir = chooseStorageDir(directories, group.getGroupId().getUuid().toString()); try { - storage = RaftStorageImpl.newBuilder() - .setDir(dir) + storage = (RaftStorageImpl) RaftStorage.newBuilder() + .setDirectory(dir) .setLogCorruptionPolicy(RaftServerConfigKeys.Log.corruptionPolicy(prop)) - .setStorageFeeSpaceMin(RaftServerConfigKeys.storageFreeSpaceMin(prop).getSize()) + .setStorageFreeSpaceMin(RaftServerConfigKeys.storageFreeSpaceMin(prop).getSize()) .build(); storageFound = true; break; diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java index 314b4bf786..c899cdc77b 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java @@ -36,46 +36,6 @@ /** The storage of a {@link org.apache.ratis.server.RaftServer}. */ public class RaftStorageImpl implements RaftStorage { - public enum StartupOption { - /** Format the storage. */ - FORMAT - } - - public static Builder newBuilder() { - return new RaftStorageImpl.Builder(); - } - - public static class Builder { - private File dir; - private CorruptionPolicy logCorruptionPolicy; - private StartupOption option; - private long storageFeeSpaceMin; - - public Builder setDir(File dir) { - this.dir = dir; - return this; - } - - public Builder setLogCorruptionPolicy(CorruptionPolicy logCorruptionPolicy) { - this.logCorruptionPolicy = logCorruptionPolicy; - return this; - } - - public Builder setOption(StartupOption option) { - this.option = option; - return this; - } - - public Builder setStorageFeeSpaceMin(long storageFeeSpaceMin) { - this.storageFeeSpaceMin = storageFeeSpaceMin; - return this; - } - - public RaftStorageImpl build() throws IOException { - return new RaftStorageImpl(dir, logCorruptionPolicy, option, storageFeeSpaceMin); - } - } - // TODO support multiple storage directories private final RaftStorageDirectoryImpl storageDir; private final StorageState state; diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java new file mode 100644 index 0000000000..cb9df1fdbc --- /dev/null +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ratis.server.storage; + +import org.apache.ratis.server.RaftServerConfigKeys; +import org.apache.ratis.util.IOUtils; +import org.apache.ratis.util.JavaUtils; +import org.apache.ratis.util.TimeDuration; + +import java.io.File; +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +public final class StorageImplUtils { + + private StorageImplUtils() { + //Never constructed + } + + /** Create a {@link RaftStorageImpl}. */ + public static RaftStorageImpl newRaftStorage(File dir, RaftServerConfigKeys.Log.CorruptionPolicy logCorruptionPolicy, + RaftStorage.StartupOption option, long storageFeeSpaceMin) throws IOException { + RaftStorage.LOG.debug("newRaftStorage: {}, {}, {}, {}",dir, logCorruptionPolicy, option, storageFeeSpaceMin); + + final TimeDuration sleepTime = TimeDuration.valueOf(500, TimeUnit.MILLISECONDS); + final RaftStorageImpl raftStorage; + try { + // attempt multiple times to avoid temporary bind exception + raftStorage = JavaUtils.attemptRepeatedly( + () -> new RaftStorageImpl(dir, logCorruptionPolicy, option, storageFeeSpaceMin), + 5, sleepTime, "new RaftServerProxy", RaftStorage.LOG); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw IOUtils.toInterruptedIOException( + "Interrupted when creating RaftStorage " + dir, e); + } catch (IOException e) { + throw new RuntimeException(e); + } + return raftStorage; + } +} diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java index 280a105e87..781e30741f 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java @@ -34,9 +34,10 @@ public interface RaftStorageTestUtils { static RaftStorage newRaftStorage(File dir) throws IOException { - return RaftStorageImpl.newBuilder() - .setDir(dir) - .setStorageFeeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) + return RaftStorage.newBuilder() + .setDirectory(dir) + .setOption(RaftStorage.StartupOption.RECOVER) + .setStorageFreeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) .build(); } diff --git a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java index dbbd6df335..77bf834d4f 100644 --- a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java +++ b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java @@ -48,9 +48,10 @@ */ public class TestRaftStorage extends BaseTest { static RaftStorageImpl newRaftStorage(File dir) throws IOException { - return RaftStorageImpl.newBuilder() - .setDir(dir) - .setStorageFeeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) + return (RaftStorageImpl) RaftStorage.newBuilder() + .setDirectory(dir) + .setOption(RaftStorage.StartupOption.RECOVER) + .setStorageFreeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) .build(); } @@ -69,10 +70,10 @@ public void tearDown() throws Exception { } static RaftStorageImpl formatRaftStorage(File dir) throws IOException { - return RaftStorageImpl.newBuilder() - .setDir(dir) + return (RaftStorageImpl) RaftStorage.newBuilder() + .setDirectory(dir) .setOption(RaftStorageImpl.StartupOption.FORMAT) - .setStorageFeeSpaceMin(0) + .setStorageFreeSpaceMin(0) .build(); } From 59c850131b4231e51f227ad966ead3cbb8613fc7 Mon Sep 17 00:00:00 2001 From: dragonyliu Date: Tue, 14 Jun 2022 19:41:24 +0800 Subject: [PATCH 4/7] make code better --- .../src/main/java/org/apache/ratis/server/impl/ServerState.java | 1 + .../java/org/apache/ratis/server/storage/StorageImplUtils.java | 2 +- .../java/org/apache/ratis/server/storage/TestRaftStorage.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java index 8de0321f21..7357f3dc14 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java @@ -114,6 +114,7 @@ class ServerState implements Closeable { try { storage = (RaftStorageImpl) RaftStorage.newBuilder() .setDirectory(dir) + .setOption(RaftStorage.StartupOption.RECOVER) .setLogCorruptionPolicy(RaftServerConfigKeys.Log.corruptionPolicy(prop)) .setStorageFreeSpaceMin(RaftServerConfigKeys.storageFreeSpaceMin(prop).getSize()) .build(); diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java index cb9df1fdbc..10296657b4 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java @@ -43,7 +43,7 @@ public static RaftStorageImpl newRaftStorage(File dir, RaftServerConfigKeys.Log. // attempt multiple times to avoid temporary bind exception raftStorage = JavaUtils.attemptRepeatedly( () -> new RaftStorageImpl(dir, logCorruptionPolicy, option, storageFeeSpaceMin), - 5, sleepTime, "new RaftServerProxy", RaftStorage.LOG); + 5, sleepTime, "new RaftStorageImpl", RaftStorage.LOG); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw IOUtils.toInterruptedIOException( diff --git a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java index 77bf834d4f..3701bf9ac0 100644 --- a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java +++ b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java @@ -72,7 +72,7 @@ public void tearDown() throws Exception { static RaftStorageImpl formatRaftStorage(File dir) throws IOException { return (RaftStorageImpl) RaftStorage.newBuilder() .setDirectory(dir) - .setOption(RaftStorageImpl.StartupOption.FORMAT) + .setOption(RaftStorage.StartupOption.FORMAT) .setStorageFreeSpaceMin(0) .build(); } From b84c2e31b6d9c8d8148069e31bf8d1ee0a9b64d1 Mon Sep 17 00:00:00 2001 From: dragonyliu Date: Thu, 16 Jun 2022 20:57:53 +0800 Subject: [PATCH 5/7] fix ut and remove extra code --- .../apache/ratis/server/storage/RaftStorage.java | 14 ++++++++++---- .../org/apache/ratis/server/impl/ServerState.java | 2 +- .../ratis/server/storage/RaftStorageImpl.java | 5 ----- .../ratis/server/storage/RaftStorageTestUtils.java | 2 +- .../ratis/server/storage/TestRaftStorage.java | 5 +++-- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java index 5eb1186bb1..c3eaef98d7 100644 --- a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java +++ b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java @@ -20,6 +20,7 @@ import org.apache.ratis.server.RaftServerConfigKeys.Log.CorruptionPolicy; import org.apache.ratis.util.IOUtils; import org.apache.ratis.util.ReflectionUtils; +import org.apache.ratis.util.SizeInBytes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -28,6 +29,7 @@ import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.nio.channels.OverlappingFileLockException; /** The storage of a raft server. */ public interface RaftStorage extends Closeable { @@ -69,13 +71,17 @@ private static Method initNewRaftStorageMethod() { } private static RaftStorage newRaftStorage(File dir, CorruptionPolicy logCorruptionPolicy, - StartupOption option, long storageFreeSpaceMin) throws IOException { + StartupOption option, SizeInBytes storageFreeSpaceMin) throws IOException { try { return (RaftStorage) NEW_RAFT_STORAGE_METHOD.invoke(null, - dir, logCorruptionPolicy, option, storageFreeSpaceMin); + dir, logCorruptionPolicy, option, storageFreeSpaceMin.getSize()); } catch (IllegalAccessException e) { throw new IllegalStateException("Failed to build " + dir, e); } catch (InvocationTargetException e) { + Throwable t = e.getTargetException(); + if (t.getCause() instanceof IOException) { + throw IOUtils.asIOException(t.getCause().getCause()); + } throw IOUtils.asIOException(e.getCause()); } } @@ -84,7 +90,7 @@ private static RaftStorage newRaftStorage(File dir, CorruptionPolicy logCorrupti private File directory; private CorruptionPolicy logCorruptionPolicy; private StartupOption option; - private long storageFreeSpaceMin; + private SizeInBytes storageFreeSpaceMin; public Builder setDirectory(File directory) { this.directory = directory; @@ -101,7 +107,7 @@ public Builder setOption(StartupOption option) { return this; } - public Builder setStorageFreeSpaceMin(long storageFreeSpaceMin) { + public Builder setStorageFreeSpaceMin(SizeInBytes storageFreeSpaceMin) { this.storageFreeSpaceMin = storageFreeSpaceMin; return this; } diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java index 7357f3dc14..37c6fcd3ba 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java @@ -116,7 +116,7 @@ class ServerState implements Closeable { .setDirectory(dir) .setOption(RaftStorage.StartupOption.RECOVER) .setLogCorruptionPolicy(RaftServerConfigKeys.Log.corruptionPolicy(prop)) - .setStorageFreeSpaceMin(RaftServerConfigKeys.storageFreeSpaceMin(prop).getSize()) + .setStorageFreeSpaceMin(RaftServerConfigKeys.storageFreeSpaceMin(prop)) .build(); storageFound = true; break; diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java index c899cdc77b..fdf1b0b184 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java @@ -42,11 +42,6 @@ public class RaftStorageImpl implements RaftStorage { private final CorruptionPolicy logCorruptionPolicy; private volatile RaftStorageMetadataFileImpl metaFile; - public RaftStorageImpl(File dir, CorruptionPolicy logCorruptionPolicy, - long storageFeeSpaceMin) throws IOException { - this(dir, logCorruptionPolicy, null, storageFeeSpaceMin); - } - RaftStorageImpl(File dir, CorruptionPolicy logCorruptionPolicy, StartupOption option, long storageFeeSpaceMin) throws IOException { this.storageDir = new RaftStorageDirectoryImpl(dir, storageFeeSpaceMin); diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java index 781e30741f..d22bc5c4e7 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/RaftStorageTestUtils.java @@ -37,7 +37,7 @@ static RaftStorage newRaftStorage(File dir) throws IOException { return RaftStorage.newBuilder() .setDirectory(dir) .setOption(RaftStorage.StartupOption.RECOVER) - .setStorageFreeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) + .setStorageFreeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT) .build(); } diff --git a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java index 3701bf9ac0..5a34264cc2 100644 --- a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java +++ b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java @@ -28,6 +28,7 @@ import org.apache.ratis.statemachine.impl.SimpleStateMachineStorage; import org.apache.ratis.statemachine.SnapshotRetentionPolicy; import org.apache.ratis.util.FileUtils; +import org.apache.ratis.util.SizeInBytes; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -51,7 +52,7 @@ static RaftStorageImpl newRaftStorage(File dir) throws IOException { return (RaftStorageImpl) RaftStorage.newBuilder() .setDirectory(dir) .setOption(RaftStorage.StartupOption.RECOVER) - .setStorageFreeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT.getSize()) + .setStorageFreeSpaceMin(RaftServerConfigKeys.STORAGE_FREE_SPACE_MIN_DEFAULT) .build(); } @@ -73,7 +74,7 @@ static RaftStorageImpl formatRaftStorage(File dir) throws IOException { return (RaftStorageImpl) RaftStorage.newBuilder() .setDirectory(dir) .setOption(RaftStorage.StartupOption.FORMAT) - .setStorageFreeSpaceMin(0) + .setStorageFreeSpaceMin(SizeInBytes.valueOf(0)) .build(); } From beb3ac981becd1a64c677d0e5991878802f723b2 Mon Sep 17 00:00:00 2001 From: dragonyliu Date: Thu, 16 Jun 2022 21:03:14 +0800 Subject: [PATCH 6/7] fix checkstyle error --- .../main/java/org/apache/ratis/server/storage/RaftStorage.java | 1 - 1 file changed, 1 deletion(-) diff --git a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java index c3eaef98d7..26bef93906 100644 --- a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java +++ b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java @@ -29,7 +29,6 @@ import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.nio.channels.OverlappingFileLockException; /** The storage of a raft server. */ public interface RaftStorage extends Closeable { From 046af2746f529afe7b61fe5c4d8696395c1f0f40 Mon Sep 17 00:00:00 2001 From: dragonyliu Date: Fri, 17 Jun 2022 11:08:14 +0800 Subject: [PATCH 7/7] fix ut error --- .../org/apache/ratis/server/storage/RaftStorage.java | 2 +- .../org/apache/ratis/grpc/TestRaftServerWithGrpc.java | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java index 26bef93906..bd2a59a2b8 100644 --- a/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java +++ b/ratis-server-api/src/main/java/org/apache/ratis/server/storage/RaftStorage.java @@ -79,7 +79,7 @@ private static RaftStorage newRaftStorage(File dir, CorruptionPolicy logCorrupti } catch (InvocationTargetException e) { Throwable t = e.getTargetException(); if (t.getCause() instanceof IOException) { - throw IOUtils.asIOException(t.getCause().getCause()); + throw IOUtils.asIOException(t.getCause()); } throw IOUtils.asIOException(e.getCause()); } diff --git a/ratis-test/src/test/java/org/apache/ratis/grpc/TestRaftServerWithGrpc.java b/ratis-test/src/test/java/org/apache/ratis/grpc/TestRaftServerWithGrpc.java index 8dcff758b4..16e11f7d30 100644 --- a/ratis-test/src/test/java/org/apache/ratis/grpc/TestRaftServerWithGrpc.java +++ b/ratis-test/src/test/java/org/apache/ratis/grpc/TestRaftServerWithGrpc.java @@ -125,9 +125,13 @@ void runTestServerRestartOnException(MiniRaftClusterWithGrpc cluster) throws Exc // the raft server proxy created earlier. Raft server proxy should close // the rpc server on failure. RaftServerConfigKeys.setStorageDir(p, Collections.singletonList(cluster.getStorageDir(leaderId))); - testFailureCase("start a new server with the same address", - () -> newRaftServer(cluster, leaderId, stateMachine, p).start(), - IOException.class, OverlappingFileLockException.class); + try { + LOG.info("start a new server with the same address"); + newRaftServer(cluster, leaderId, stateMachine, p).start(); + } catch (IOException e) { + Assert.assertTrue(e.getCause() instanceof OverlappingFileLockException); + Assert.assertTrue(e.getMessage().contains("directory is already locked")); + } // Try to start a raft server rpc at the leader address. cluster.getServerFactory(leaderId).newRaftServerRpc(cluster.getServer(leaderId)); }