Repository: incubator-ratis Updated Branches: refs/heads/master 9ce1783d3 -> 86383d689
RATIS-111. RaftLogWorker may throw IllegalStateException. Project: http://git-wip-us.apache.org/repos/asf/incubator-ratis/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ratis/commit/86383d68 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ratis/tree/86383d68 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ratis/diff/86383d68 Branch: refs/heads/master Commit: 86383d689389bb3ccc9336f97bac1b66caa3f867 Parents: 9ce1783 Author: Tsz-Wo Nicholas Sze <[email protected]> Authored: Thu Sep 7 14:17:30 2017 +0800 Committer: Tsz-Wo Nicholas Sze <[email protected]> Committed: Thu Sep 7 14:17:30 2017 +0800 ---------------------------------------------------------------------- .../ratis/util/AtomicFileOutputStream.java | 10 +- .../java/org/apache/ratis/util/FileUtils.java | 184 +++++++------------ .../java/org/apache/ratis/util/JavaUtils.java | 2 +- .../java/org/apache/ratis/util/LogUtils.java | 19 ++ .../test/java/org/apache/ratis/BaseTest.java | 2 + .../examples/arithmetic/TestArithmetic.java | 2 +- .../ratis/hadooprpc/TestRaftWithHadoopRpc.java | 2 +- .../ratis/server/impl/RaftServerProxy.java | 2 +- .../ratis/server/storage/RaftLogWorker.java | 48 ++--- .../ratis/server/storage/RaftStorage.java | 3 +- .../server/storage/RaftStorageDirectory.java | 21 +-- .../ratis/server/storage/SegmentedRaftLog.java | 3 +- .../ratis/server/storage/SnapshotManager.java | 4 +- .../java/org/apache/ratis/MiniRaftCluster.java | 15 +- .../java/org/apache/ratis/RaftBasicTests.java | 6 +- .../org/apache/ratis/RaftExceptionBaseTest.java | 2 - .../java/org/apache/ratis/RaftTestUtil.java | 16 +- .../server/storage/TestRaftLogReadWrite.java | 2 +- .../server/storage/TestRaftLogSegment.java | 2 +- .../ratis/server/storage/TestRaftStorage.java | 9 +- .../server/storage/TestSegmentedRaftLog.java | 2 +- 21 files changed, 162 insertions(+), 194 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-common/src/main/java/org/apache/ratis/util/AtomicFileOutputStream.java ---------------------------------------------------------------------- diff --git a/ratis-common/src/main/java/org/apache/ratis/util/AtomicFileOutputStream.java b/ratis-common/src/main/java/org/apache/ratis/util/AtomicFileOutputStream.java index e181e44..c0e5cec 100644 --- a/ratis-common/src/main/java/org/apache/ratis/util/AtomicFileOutputStream.java +++ b/ratis-common/src/main/java/org/apache/ratis/util/AtomicFileOutputStream.java @@ -17,8 +17,6 @@ */ package org.apache.ratis.util; -import org.apache.ratis.io.nativeio.NativeIO; -import org.apache.ratis.io.nativeio.NativeIOException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -72,13 +70,7 @@ public class AtomicFileOutputStream extends FilterOutputStream { if (origFile.exists() && !origFile.delete()) { throw new IOException("Could not delete original file " + origFile); } - try { - NativeIO.renameTo(tmpFile, origFile); - } catch (NativeIOException e) { - throw new IOException("Could not rename temporary file " + tmpFile - + " to " + origFile + " due to failure in native rename. " - + e.toString()); - } + FileUtils.move(tmpFile, origFile); } } else { if (!triedToClose) { http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java ---------------------------------------------------------------------- diff --git a/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java b/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java index 714a46c..38fdff3 100644 --- a/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java +++ b/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java @@ -17,143 +17,101 @@ */ package org.apache.ratis.util; -import org.apache.ratis.io.nativeio.NativeIO; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.nio.file.Files; +import java.nio.file.*; +import java.nio.file.attribute.BasicFileAttributes; -public class FileUtils { - public static final Logger LOG = LoggerFactory.getLogger(FileUtils.class); +public interface FileUtils { + Logger LOG = LoggerFactory.getLogger(FileUtils.class); - public static void truncateFile(File f, long target) throws IOException { - try (FileOutputStream out = new FileOutputStream(f, true)) { - out.getChannel().truncate(target); - } + static void truncateFile(File f, long target) throws IOException { + LogUtils.runAndLog(LOG, + () -> { + try (FileOutputStream out = new FileOutputStream(f, true)) { + out.getChannel().truncate(target); + }}, + () -> "FileOutputStream.getChannel().truncate " + f + " to target length " + target); } - public static void deleteFile(File f) throws IOException { - try { - Files.delete(f.toPath()); - } catch (IOException e) { - LOG.warn("Could not delete " + f); - throw e; - } + static void createDirectories(File dir) throws IOException { + createDirectories(dir.toPath()); } - /** - * Delete a directory and all its contents. If - * we return false, the directory may be partially-deleted. - * (1) If dir is symlink to a file, the symlink is deleted. The file pointed - * to by the symlink is not deleted. - * (2) If dir is symlink to a directory, symlink is deleted. The directory - * pointed to by symlink is not deleted. - * (3) If dir is a normal file, it is deleted. - * (4) If dir is a normal directory, then dir and all its contents recursively - * are deleted. - */ - public static boolean fullyDelete(final File dir) { - if (deleteImpl(dir, false)) { - // dir is (a) normal file, (b) symlink to a file, (c) empty directory or - // (d) symlink to a directory - return true; - } - // handle nonempty directory deletion - return fullyDeleteContents(dir) && deleteImpl(dir, true); + static void createDirectories(Path dir) throws IOException { + LogUtils.runAndLog(LOG, + () -> Files.createDirectories(dir), + () -> "Files.createDirectories " + dir); } - private static boolean deleteImpl(final File f, final boolean doLog) { - if (f == null) { - LOG.warn("null file argument."); - return false; - } - final boolean wasDeleted = f.delete(); - if (wasDeleted) { - LOG.debug("Deleted file or dir {}", f.getAbsolutePath()); - return true; - } - final boolean ex = f.exists(); - if (doLog && ex) { - LOG.warn("Failed to delete file or dir [" - + f.getAbsolutePath() + "]: it still exists."); - } - return !ex; + static void move(File src, File dst) throws IOException { + move(src.toPath(), dst.toPath()); } - /** - * Delete the contents of a directory, not the directory itself. If - * we return false, the directory may be partially-deleted. - * If dir is a symlink to a directory, all the contents of the actual - * directory pointed to by dir will be deleted. - */ - private static boolean fullyDeleteContents(final File dir) { - boolean deletionSucceeded = true; - final File[] contents = dir.listFiles(); - if (contents != null) { - for (File content : contents) { - if (content.isFile()) { - if (!deleteImpl(content, true)) { - deletionSucceeded = false; - } - } else { - // Either directory or symlink to another directory. - // Try deleting the directory as this might be a symlink - if (deleteImpl(content, false)) { - // this was indeed a symlink or an empty directory - continue; - } - // if not an empty directory or symlink let - // fullyDelete handle it. - if (!fullyDelete(content)) { - deletionSucceeded = false; - // continue deletion of other files/dirs under dir - } - } - } - } - return deletionSucceeded; + static void move(Path src, Path dst) throws IOException { + LogUtils.runAndLog(LOG, + () -> Files.move(src, dst), + () -> "Files.move " + src + " to " + dst); + } + + + /** The same as passing f.toPath() to {@link #delete(Path)}. */ + static void deleteFile(File f) throws IOException { + delete(f.toPath()); } /** - * A wrapper for {@link File#listFiles()}. This java.io API returns null - * when a dir is not a directory or for any I/O error. Instead of having - * null check everywhere File#listFiles() is used, we will add utility API - * to get around this problem. For the majority of cases where we prefer - * an IOException to be thrown. - * @param dir directory for which listing should be performed - * @return list of files or empty list - * @exception IOException for invalid directory or for a bad disk. + * Use {@link Files#delete(Path)} to delete the given path. + * + * This method may print log messages using {@link #LOG}. */ - public static File[] listFiles(File dir) throws IOException { - File[] files = dir.listFiles(); - if(files == null) { - throw new IOException("Invalid directory or I/O error occurred for dir: " - + dir.toString()); - } - return files; + static void delete(Path p) throws IOException { + LogUtils.runAndLog(LOG, + () -> Files.delete(p), + () -> "Files.delete " + p); + } + + /** The same as passing f.toPath() to {@link #deleteFully(Path)}. */ + static void deleteFully(File f) throws IOException { + LOG.trace("deleteFully {}", f); + deleteFully(f.toPath()); } /** - * Platform independent implementation for {@link File#canWrite()} - * @param f input file - * @return On Unix, same as {@link File#canWrite()} - * On Windows, true if process has write access on the path + * Delete fully the given path. + * + * (1) If it is a file, the file will be deleted. + * + * (2) If it is a directory, the directory and all its contents will be recursively deleted. + * If an exception is thrown, the directory may possibly be partially deleted.* + * + * (3) If it is a symlink, the symlink will be deleted but the symlink target will not be deleted. */ - public static boolean canWrite(File f) { - if (PlatformUtils.WINDOWS) { - try { - return NativeIO.Windows.access(f.getCanonicalPath(), - NativeIO.Windows.AccessRight.ACCESS_WRITE); - } catch (IOException e) { - return false; - } - } else { - return f.canWrite(); + static void deleteFully(Path p) throws IOException { + if (!Files.exists(p, LinkOption.NOFOLLOW_LINKS)) { + LOG.trace("deleteFully: {} does not exist."); + return; } + Files.walkFileTree(p, new SimpleFileVisitor<Path>() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + delete(file); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException e) throws IOException { + if (e != null) { + // directory iteration failed + throw e; + } + delete(dir); + return FileVisitResult.CONTINUE; + } + }); } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java ---------------------------------------------------------------------- diff --git a/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java b/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java index c78f999..9d4a6e5 100644 --- a/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java +++ b/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java @@ -124,7 +124,7 @@ public interface JavaUtils { } if (log != null && log.isWarnEnabled()) { log.warn("FAILED " + name + " attempt #" + i + "/" + numAttempts - + ": " + t + ", sleep " + sleepMs + "ms and then retry."); + + ": " + t + ", sleep " + sleepMs + "ms and then retry.", t); } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-common/src/main/java/org/apache/ratis/util/LogUtils.java ---------------------------------------------------------------------- diff --git a/ratis-common/src/main/java/org/apache/ratis/util/LogUtils.java b/ratis-common/src/main/java/org/apache/ratis/util/LogUtils.java index addc2ff..ebb61be 100644 --- a/ratis-common/src/main/java/org/apache/ratis/util/LogUtils.java +++ b/ratis-common/src/main/java/org/apache/ratis/util/LogUtils.java @@ -24,6 +24,8 @@ import org.apache.log4j.Level; import org.apache.log4j.LogManager; import org.slf4j.Logger; +import java.util.function.Supplier; + /** * Logging (as in log4j) related utility methods. */ @@ -33,4 +35,21 @@ public interface LogUtils { LogManager.getLogger(logger.getName()).setLevel(level); } + static <THROWABLE extends Throwable> void runAndLog( + Logger log, CheckedRunnable<THROWABLE> op, Supplier<String> opName) + throws THROWABLE { + try { + op.run(); + if (log.isTraceEnabled()) { + log.trace("Executed " + opName.get() + " successfully."); + } + } catch (Throwable t) { + if (log.isTraceEnabled()) { + log.trace("Failed to " + opName.get(), t); + } else if (log.isWarnEnabled()){ + log.warn("Failed to " + opName.get() + ": " + t); + } + throw t; + } + } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-common/src/test/java/org/apache/ratis/BaseTest.java ---------------------------------------------------------------------- diff --git a/ratis-common/src/test/java/org/apache/ratis/BaseTest.java b/ratis-common/src/test/java/org/apache/ratis/BaseTest.java index f77cc64..2b308b5 100644 --- a/ratis-common/src/test/java/org/apache/ratis/BaseTest.java +++ b/ratis-common/src/test/java/org/apache/ratis/BaseTest.java @@ -20,6 +20,7 @@ package org.apache.ratis; import org.apache.log4j.Level; import org.apache.ratis.conf.ConfUtils; import org.apache.ratis.util.CheckedRunnable; +import org.apache.ratis.util.FileUtils; import org.apache.ratis.util.JavaUtils; import org.apache.ratis.util.LogUtils; import org.junit.Assert; @@ -38,6 +39,7 @@ public abstract class BaseTest { { LogUtils.setLogLevel(ConfUtils.LOG, Level.WARN); + LogUtils.setLogLevel(FileUtils.LOG, Level.TRACE); } @Rule http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-examples/src/test/java/org/apache/ratis/examples/arithmetic/TestArithmetic.java ---------------------------------------------------------------------- diff --git a/ratis-examples/src/test/java/org/apache/ratis/examples/arithmetic/TestArithmetic.java b/ratis-examples/src/test/java/org/apache/ratis/examples/arithmetic/TestArithmetic.java index feff88f..45211d9 100644 --- a/ratis-examples/src/test/java/org/apache/ratis/examples/arithmetic/TestArithmetic.java +++ b/ratis-examples/src/test/java/org/apache/ratis/examples/arithmetic/TestArithmetic.java @@ -55,7 +55,7 @@ public class TestArithmetic extends BaseTest { try { RaftTestUtil.waitForLeader(cluster); try (final RaftClient client = cluster.createClient()) { - runTestPythagorean(client, 3, 100); + runTestPythagorean(client, 3, 10); } } finally { cluster.shutdown(); http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-hadoop/src/test/java/org/apache/ratis/hadooprpc/TestRaftWithHadoopRpc.java ---------------------------------------------------------------------- diff --git a/ratis-hadoop/src/test/java/org/apache/ratis/hadooprpc/TestRaftWithHadoopRpc.java b/ratis-hadoop/src/test/java/org/apache/ratis/hadooprpc/TestRaftWithHadoopRpc.java index 0519e52..124e7ee 100644 --- a/ratis-hadoop/src/test/java/org/apache/ratis/hadooprpc/TestRaftWithHadoopRpc.java +++ b/ratis-hadoop/src/test/java/org/apache/ratis/hadooprpc/TestRaftWithHadoopRpc.java @@ -31,7 +31,7 @@ import static org.apache.ratis.hadooprpc.MiniRaftClusterWithHadoopRpc.sendServer public class TestRaftWithHadoopRpc extends RaftBasicTests { static { - LogUtils.setLogLevel(RaftServerImpl.LOG, Level.TRACE); + LogUtils.setLogLevel(RaftServerImpl.LOG, Level.DEBUG); LogUtils.setLogLevel(RaftClient.LOG, Level.DEBUG); LogUtils.setLogLevel(MiniRaftClusterWithHadoopRpc.LOG, Level.DEBUG); } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java index 554372c..d2bf54c 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java @@ -59,8 +59,8 @@ public class RaftServerProxy implements RaftServer { final RpcType rpcType = RaftConfigKeys.Rpc.type(properties); this.factory = ServerFactory.cast(rpcType.newFactory(parameters)); - this.impl = CompletableFuture.completedFuture(initImpl(group)); this.serverRpc = initRaftServerRpc(factory, this, group); + this.impl = CompletableFuture.completedFuture(initImpl(group)); } private RaftServerImpl initImpl(RaftGroup group) throws IOException { http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLogWorker.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLogWorker.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLogWorker.java index 0a90d91..3d60c3d 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLogWorker.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLogWorker.java @@ -17,14 +17,8 @@ */ package org.apache.ratis.server.storage; -import java.io.File; -import java.io.IOException; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.TimeUnit; - import org.apache.ratis.conf.RaftProperties; -import org.apache.ratis.io.nativeio.NativeIO; +import org.apache.ratis.protocol.RaftPeerId; import org.apache.ratis.server.RaftServerConfigKeys; import org.apache.ratis.server.impl.RaftServerConstants; import org.apache.ratis.server.impl.RaftServerImpl; @@ -39,12 +33,20 @@ import org.apache.ratis.util.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; +import java.io.IOException; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.TimeUnit; + /** * This class takes the responsibility of all the raft log related I/O ops for a * raft peer. */ class RaftLogWorker implements Runnable { static final Logger LOG = LoggerFactory.getLogger(RaftLogWorker.class); + + private final String name; /** * The task queue accessed by rpc handler threads and the io worker thread. */ @@ -70,17 +72,21 @@ class RaftLogWorker implements Runnable { private final RaftProperties properties; - RaftLogWorker(RaftServerImpl raftServer, RaftStorage storage, + RaftLogWorker(RaftPeerId selfId, RaftServerImpl raftServer, RaftStorage storage, RaftProperties properties) { + this.name = selfId + "-" + getClass().getSimpleName(); + LOG.info("new {} for {}", name, storage); + this.raftServer = raftServer; + this.storage = storage; this.properties = properties; this.forceSyncNum = RaftServerConfigKeys.Log.forceSyncNum(properties); - workerThread = new Thread(this, - getClass().getSimpleName() + " for " + storage); + this.workerThread = new Thread(this, name); } void start(long latestIndex, File openSegmentFile) throws IOException { + LOG.trace("{} start(latestIndex={}, openSegmentFile={})", name, latestIndex, openSegmentFile); lastWrittenIndex = latestIndex; flushedIndex = latestIndex; if (openSegmentFile != null) { @@ -98,7 +104,7 @@ class RaftLogWorker implements Runnable { } catch (InterruptedException ignored) { } IOUtils.cleanup(LOG, out); - LOG.info("{} closes.", this.toString()); + LOG.info("{} close()", name); } /** @@ -114,16 +120,14 @@ class RaftLogWorker implements Runnable { @Override public String toString() { - return this.getClass().getSimpleName() + "-" - + (raftServer != null ? raftServer.getId() : ""); + return name; } /** * This is protected by the RaftServer and RaftLog's lock. */ private Task addIOTask(Task task) { - LOG.debug("{} adds IO task {}", - raftServer != null ? raftServer.getId() : "", task); + LOG.debug("{} adds IO task {}", name, task); try { if (!queue.offer(task, 1, TimeUnit.SECONDS)) { Preconditions.assertTrue(isAlive(), @@ -275,8 +279,7 @@ class RaftLogWorker implements Runnable { File openFile = storage.getStorageDir() .getOpenLogFile(segmentToClose.getStartIndex()); - LOG.info("{} finalizing log segment {}", RaftLogWorker.this.toString(), - openFile.getAbsolutePath()); + LOG.info("{} finalizing log segment {}", name, openFile); Preconditions.assertTrue(openFile.exists(), "File %s does not exist.", openFile); if (segmentToClose.numOfEntries() > 0) { @@ -285,7 +288,7 @@ class RaftLogWorker implements Runnable { segmentToClose.getStartIndex(), segmentToClose.getEndIndex()); Preconditions.assertTrue(!dstFile.exists()); - NativeIO.renameTo(openFile, dstFile); + FileUtils.move(openFile, dstFile); } else { // delete the file of the empty segment FileUtils.deleteFile(openFile); } @@ -308,12 +311,13 @@ class RaftLogWorker implements Runnable { @Override void execute() throws IOException { File openFile = storage.getStorageDir().getOpenLogFile(newStartIndex); - LOG.info("{} creating new log segment {}", RaftLogWorker.this.toString(), - openFile.getAbsolutePath()); + LOG.info("{} creating new log segment {}", name, openFile); Preconditions.assertTrue(!openFile.exists(), "open file %s exists for %s", - openFile.getAbsolutePath(), RaftLogWorker.this.toString()); + openFile, name); Preconditions.assertTrue(out == null && pendingFlushNum == 0); out = new LogOutputStream(openFile, false, properties); + Preconditions.assertTrue(openFile.exists(), "Failed to create file %s for %s", + openFile.getAbsolutePath(), name); } @Override @@ -346,7 +350,7 @@ class RaftLogWorker implements Runnable { File dstFile = storage.getStorageDir().getClosedLogFile( segments.toTruncate.startIndex, segments.toTruncate.newEndIndex); Preconditions.assertTrue(!dstFile.exists()); - NativeIO.renameTo(fileToTruncate, dstFile); + FileUtils.move(fileToTruncate, dstFile); // update lastWrittenIndex lastWrittenIndex = segments.toTruncate.newEndIndex; http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorage.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorage.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorage.java index 84363fe..16cd342 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorage.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorage.java @@ -86,7 +86,8 @@ public class RaftStorage implements Closeable { StorageState storageState = storageDir.analyzeStorage(toLock); if (storageState == StorageState.NORMAL) { metaFile = new MetaFile(storageDir.getMetaFile()); - assert metaFile.exists(); + Preconditions.assertTrue(metaFile.exists(), + () -> "Meta file " + metaFile + " does not exists."); metaFile.readFile(); // Existence of raft-meta.tmp means the change of votedFor/term has not // been committed. Thus we should delete the tmp file. http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageDirectory.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageDirectory.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageDirectory.java index 7f87e65..530f679 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageDirectory.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageDirectory.java @@ -117,13 +117,10 @@ public class RaftStorageDirectory { void clearDirectory(File dir) throws IOException { if (dir.exists()) { - File[] files = FileUtils.listFiles(dir); - LOG.info("Will remove files: " + Arrays.toString(files)); - if (!(FileUtils.fullyDelete(dir))) - throw new IOException("Cannot remove directory: " + dir); + LOG.info(dir + " already exists. Deleting it ..."); + FileUtils.deleteFully(dir); } - if (!dir.mkdirs()) - throw new IOException("Cannot create directory " + dir); + FileUtils.createDirectories(dir); } /** @@ -231,18 +228,16 @@ public class RaftStorageDirectory { String rootPath = root.getCanonicalPath(); try { // check that storage exists if (!root.exists()) { - LOG.info(rootPath + " does not exist. Creating ..."); - if (!root.mkdirs()) { - throw new IOException("Cannot create directory " + rootPath); - } + LOG.info("The storage directory " + rootPath + " does not exist. Creating ..."); + FileUtils.createDirectories(root); } // or is inaccessible if (!root.isDirectory()) { - LOG.warn(rootPath + "is not a directory"); + LOG.warn(rootPath + " is not a directory"); return StorageState.NON_EXISTENT; } - if (!FileUtils.canWrite(root)) { - LOG.warn("Cannot access storage directory " + rootPath); + if (!Files.isWritable(root.toPath())) { + LOG.warn("The storage directory " + rootPath + " is not writable."); return StorageState.NON_EXISTENT; } } catch(SecurityException ex) { http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java index d82550b..f964a77 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java @@ -67,7 +67,6 @@ import java.util.function.Consumer; public class SegmentedRaftLog extends RaftLog { static final String HEADER_STR = "RAFTLOG1"; static final byte[] HEADER_BYTES = HEADER_STR.getBytes(StandardCharsets.UTF_8); - static final LogSegment[] EMPTY_SEGMENT_ARRAY = new LogSegment[0]; /** * I/O task definitions. @@ -111,7 +110,7 @@ public class SegmentedRaftLog extends RaftLog { this.storage = storage; segmentMaxSize = RaftServerConfigKeys.Log.segmentSizeMax(properties).getSize(); cache = new RaftLogCache(storage, properties); - fileLogWorker = new RaftLogWorker(server, storage, properties); + fileLogWorker = new RaftLogWorker(selfId, server, storage, properties); lastCommitted.set(lastIndexInSnapshot); } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java index bd25d07..ccccf22 100644 --- a/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java +++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java @@ -58,7 +58,7 @@ public class SnapshotManager { final RaftStorageDirectory dir = storage.getStorageDir(); File tmpDir = dir.getNewTempDir(); - tmpDir.mkdirs(); + FileUtils.createDirectories(tmpDir); tmpDir.deleteOnExit(); LOG.info("Installing snapshot:{}, to tmp dir:{}", request, tmpDir); @@ -85,7 +85,7 @@ public class SnapshotManager { // same last index. if (chunk.getOffset() == 0) { if (tmpSnapshotFile.exists()) { - FileUtils.fullyDelete(tmpSnapshotFile); + FileUtils.deleteFully(tmpSnapshotFile); } // create the temp snapshot file and put padding inside out = new FileOutputStream(tmpSnapshotFile); http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java b/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java index a50c0d7..517af66 100644 --- a/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java +++ b/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java @@ -113,12 +113,6 @@ public abstract class MiniRaftCluster { + "/" + getClass().getSimpleName() + "/" + id); } - private static void formatDir(File dir) { - Preconditions.assertTrue(FileUtils.fullyDelete(dir), - "Failed to format directory %s", dir); - LOG.info("Formatted directory {}", dir); - } - public static String[] generateIds(int numServers, int base) { String[] ids = new String[numServers]; for (int i = 0; i < numServers; i++) { @@ -203,7 +197,8 @@ public abstract class MiniRaftCluster { try { final File dir = getStorageDir(id); if (format) { - formatDir(dir); + FileUtils.deleteFully(dir); + LOG.info("Formatted directory {}", dir); } final RaftProperties prop = new RaftProperties(properties); RaftServerConfigKeys.setStorageDir(prop, dir); @@ -444,7 +439,11 @@ public abstract class MiniRaftCluster { } public void shutdown() { - LOG.info("Stopping " + getClass().getSimpleName()); + LOG.info("************************************************************** "); + LOG.info("*** "); + LOG.info("*** Stopping " + getClass().getSimpleName()); + LOG.info("*** "); + LOG.info("************************************************************** "); getServerAliveStream().map(RaftServerImpl::getProxy).forEach(RaftServerProxy::close); if (ExitUtils.isTerminated()) { http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java b/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java index 7e08809..1c7ccd5 100644 --- a/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java +++ b/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java @@ -170,11 +170,7 @@ public abstract class RaftBasicTests extends BaseTest { RaftServerImpl leader = cluster.getLeader(); if (leader != null) { - final RaftPeerId oldLeader = leader.getId(); - LOG.info("Block all requests sent by leader " + oldLeader); - RaftPeerId newLeader = RaftTestUtil.changeLeader(cluster, oldLeader); - LOG.info("Changed leader from " + oldLeader + " to " + newLeader); - Assert.assertFalse(newLeader.equals(oldLeader)); + RaftTestUtil.changeLeader(cluster, leader.getId()); } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java b/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java index eceaf20..3877083 100644 --- a/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java +++ b/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java @@ -85,7 +85,6 @@ public abstract class RaftExceptionBaseTest<CLUSTER extends MiniRaftCluster> // enforce leader change RaftPeerId newLeader = RaftTestUtil.changeLeader(cluster, leaderId); - Assert.assertNotEquals(leaderId, newLeader); if (killNewLeader) { // kill the new leader @@ -124,7 +123,6 @@ public abstract class RaftExceptionBaseTest<CLUSTER extends MiniRaftCluster> // enforce leader change RaftPeerId newLeader = RaftTestUtil.changeLeader(cluster, leaderId); - Assert.assertNotEquals(leaderId, newLeader); // also add two new peers // add two more peers http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/RaftTestUtil.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/RaftTestUtil.java b/ratis-server/src/test/java/org/apache/ratis/RaftTestUtil.java index c3972a1..65cb5fa 100644 --- a/ratis-server/src/test/java/org/apache/ratis/RaftTestUtil.java +++ b/ratis-server/src/test/java/org/apache/ratis/RaftTestUtil.java @@ -32,6 +32,7 @@ import org.apache.ratis.shaded.com.google.protobuf.ByteString; import org.apache.ratis.shaded.proto.RaftProtos.LogEntryProto; import org.apache.ratis.shaded.proto.RaftProtos.SMLogEntryProto; import org.apache.ratis.util.JavaUtils; +import org.apache.ratis.util.Preconditions; import org.junit.Assert; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -257,12 +258,17 @@ public interface RaftTestUtil { static RaftPeerId changeLeader(MiniRaftCluster cluster, RaftPeerId oldLeader) throws InterruptedException { cluster.setBlockRequestsFrom(oldLeader.toString(), true); - RaftPeerId newLeader = oldLeader; - for(int i = 0; i < 10 && newLeader.equals(oldLeader); i++) { - newLeader = RaftTestUtil.waitForLeader(cluster).getId(); + try { + return JavaUtils.attempt(() -> { + final RaftPeerId newLeader = waitForLeader(cluster).getId(); + Preconditions.assertTrue(!newLeader.equals(oldLeader), + () -> "Failed to change leader: newLeader=" + newLeader + " equals oldLeader=" + oldLeader); + LOG.info("Changed leader from " + oldLeader + " to " + newLeader); + return newLeader; + }, 10, 100L, "changeLeader", LOG); + } finally { + cluster.setBlockRequestsFrom(oldLeader.toString(), false); } - cluster.setBlockRequestsFrom(oldLeader.toString(), false); - return newLeader; } static <SERVER extends RaftServer> void blockQueueAndSetDelay( http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogReadWrite.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogReadWrite.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogReadWrite.java index f742519..d05ffda 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogReadWrite.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogReadWrite.java @@ -63,7 +63,7 @@ public class TestRaftLogReadWrite extends BaseTest { @After public void tearDown() throws Exception { if (storageDir != null) { - FileUtils.fullyDelete(storageDir.getParentFile()); + FileUtils.deleteFully(storageDir.getParentFile()); } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogSegment.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogSegment.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogSegment.java index 0366f28..69d78f7 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogSegment.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftLogSegment.java @@ -63,7 +63,7 @@ public class TestRaftLogSegment extends BaseTest { @After public void tearDown() throws Exception { if (storageDir != null) { - FileUtils.fullyDelete(storageDir.getParentFile()); + FileUtils.deleteFully(storageDir.getParentFile()); } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java index 1673fe9..6c14123 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java @@ -18,7 +18,6 @@ package org.apache.ratis.server.storage; import org.apache.ratis.BaseTest; -import org.apache.ratis.io.nativeio.NativeIO; import org.apache.ratis.server.impl.RaftServerConstants.StartupOption; import org.apache.ratis.server.protocol.TermIndex; import org.apache.ratis.server.storage.RaftStorageDirectory.StorageState; @@ -48,13 +47,13 @@ public class TestRaftStorage extends BaseTest { @After public void tearDown() throws Exception { if (storageDir != null) { - FileUtils.fullyDelete(storageDir.getParentFile()); + FileUtils.deleteFully(storageDir.getParentFile()); } } @Test public void testNotExistent() throws IOException { - FileUtils.fullyDelete(storageDir); + FileUtils.deleteFully(storageDir); // we will format the empty directory RaftStorage storage = new RaftStorage(storageDir, StartupOption.REGULAR); @@ -68,7 +67,7 @@ public class TestRaftStorage extends BaseTest { } storage.close(); - FileUtils.fullyDelete(storageDir); + FileUtils.deleteFully(storageDir); Assert.assertTrue(storageDir.createNewFile()); try { new RaftStorage(storageDir, StartupOption.REGULAR); @@ -156,7 +155,7 @@ public class TestRaftStorage extends BaseTest { RaftStorageDirectory sd = new RaftStorageDirectory(storageDir); File metaFile = sd.getMetaFile(); - NativeIO.renameTo(metaFile, sd.getMetaTmpFile()); + FileUtils.move(metaFile, sd.getMetaTmpFile()); Assert.assertEquals(StorageState.NOT_FORMATTED, sd.analyzeStorage(false)); http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/86383d68/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java ---------------------------------------------------------------------- diff --git a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java index 9385d19..4c54922 100644 --- a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java +++ b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java @@ -82,7 +82,7 @@ public class TestSegmentedRaftLog extends BaseTest { @After public void tearDown() throws Exception { if (storageDir != null) { - FileUtils.fullyDelete(storageDir.getParentFile()); + FileUtils.deleteFully(storageDir.getParentFile()); } }
