apply disk_failure_policy to bad disks on initial directory creation patch by Kirk True; reviewed by jbellis for CASSANDRA-4847
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/15837696 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/15837696 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/15837696 Branch: refs/heads/cassandra-1.2 Commit: 158376963b10f2cba75d76f9d0f4f330dfeb53e7 Parents: 40762ae Author: Jonathan Ellis <jbel...@apache.org> Authored: Thu Dec 13 10:19:12 2012 -0600 Committer: Jonathan Ellis <jbel...@apache.org> Committed: Thu Dec 13 10:19:22 2012 -0600 ---------------------------------------------------------------------- CHANGES.txt | 2 + .../cassandra/config/DatabaseDescriptor.java | 5 ++ .../cassandra/db/BlacklistedDirectories.java | 4 +- src/java/org/apache/cassandra/db/Directories.java | 15 +++++- .../org/apache/cassandra/io/util/FileUtils.java | 31 ++++++++++++ .../apache/cassandra/service/CassandraDaemon.java | 31 +----------- .../org/apache/cassandra/db/DirectoriesTest.java | 38 +++++++++++++++ 7 files changed, 94 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/15837696/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index ba0177b..8a93921 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,6 @@ 1.2.1 + * apply disk_failure_policy to bad disks on initial directory creation + (CASSANDRA-4847) * Optimize name-based queries to use ArrayBackedSortedColumns (CASSANDRA-5043) * Fall back to old manifest if most recent is unparseable (CASSANDRA-5041) * pool [Compressed]RandomAccessReader objects on the partitioned read path http://git-wip-us.apache.org/repos/asf/cassandra/blob/15837696/src/java/org/apache/cassandra/config/DatabaseDescriptor.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 0a8d67c..1319093 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -1019,6 +1019,11 @@ public class DatabaseDescriptor return indexAccessMode; } + public static void setDiskFailurePolicy(Config.DiskFailurePolicy policy) + { + conf.disk_failure_policy = policy; + } + public static Config.DiskFailurePolicy getDiskFailurePolicy() { return conf.disk_failure_policy; http://git-wip-us.apache.org/repos/asf/cassandra/blob/15837696/src/java/org/apache/cassandra/db/BlacklistedDirectories.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/BlacklistedDirectories.java b/src/java/org/apache/cassandra/db/BlacklistedDirectories.java index 78f1f0c..999483f 100644 --- a/src/java/org/apache/cassandra/db/BlacklistedDirectories.java +++ b/src/java/org/apache/cassandra/db/BlacklistedDirectories.java @@ -127,6 +127,8 @@ public class BlacklistedDirectories implements BlacklistedDirectoriesMBean if (file.getPath().endsWith(".db")) return file.getParentFile(); - throw new IllegalStateException("Unable to parse directory from path " + file); + // We may not be able to determine if it's a file or a directory if + // we were called because we couldn't create the file/directory. + return file; } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/15837696/src/java/org/apache/cassandra/db/Directories.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java index 9e53ab9..bf1b695 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -33,6 +33,7 @@ import org.slf4j.LoggerFactory; import org.apache.cassandra.config.*; import org.apache.cassandra.db.compaction.LeveledManifest; +import org.apache.cassandra.io.FSError; import org.apache.cassandra.io.FSWriteError; import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.io.sstable.*; @@ -100,8 +101,18 @@ public class Directories if (!StorageService.instance.isClientMode()) { - for (File dir : sstableDirectories) - FileUtils.createDirectory(dir); + for (File dir : sstableDirectories) + { + try + { + FileUtils.createDirectory(dir); + } + catch (FSError e) + { + // don't just let the default exception handler do this, we need the create loop to continue + FileUtils.handleFSError(e); + } + } } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/15837696/src/java/org/apache/cassandra/io/util/FileUtils.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/util/FileUtils.java b/src/java/org/apache/cassandra/io/util/FileUtils.java index 24464f7..ec0aeb6 100644 --- a/src/java/org/apache/cassandra/io/util/FileUtils.java +++ b/src/java/org/apache/cassandra/io/util/FileUtils.java @@ -27,6 +27,10 @@ import java.util.Arrays; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.BlacklistedDirectories; +import org.apache.cassandra.db.Table; +import org.apache.cassandra.io.FSError; import org.apache.cassandra.io.FSReadError; import org.apache.cassandra.io.FSWriteError; import org.apache.cassandra.service.StorageService; @@ -360,4 +364,31 @@ public class FileUtils n += skipped; } } + + public static void handleFSError(FSError e) + { + switch (DatabaseDescriptor.getDiskFailurePolicy()) + { + case stop: + logger.error("Stopping the gossiper and the RPC server"); + StorageService.instance.stopGossiping(); + StorageService.instance.stopRPCServer(); + break; + case best_effort: + // for both read and write errors mark the path as unwritable. + BlacklistedDirectories.maybeMarkUnwritable(e.path); + if (e instanceof FSReadError) + { + File directory = BlacklistedDirectories.maybeMarkUnreadable(e.path); + if (directory != null) + Table.removeUnreadableSSTables(directory); + } + break; + case ignore: + // already logged, so left nothing to do + break; + default: + throw new IllegalStateException(); + } + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/15837696/src/java/org/apache/cassandra/service/CassandraDaemon.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/CassandraDaemon.java b/src/java/org/apache/cassandra/service/CassandraDaemon.java index 8aeffd8..fd32f34 100644 --- a/src/java/org/apache/cassandra/service/CassandraDaemon.java +++ b/src/java/org/apache/cassandra/service/CassandraDaemon.java @@ -40,7 +40,7 @@ import org.apache.cassandra.db.*; import org.apache.cassandra.db.commitlog.CommitLog; import org.apache.cassandra.db.compaction.CompactionManager; import org.apache.cassandra.io.FSError; -import org.apache.cassandra.io.FSReadError; +import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.thrift.ThriftServer; import org.apache.cassandra.utils.CLibrary; import org.apache.cassandra.utils.Mx4jTool; @@ -141,37 +141,10 @@ public class CassandraDaemon { if (e2 != e) // make sure FSError gets logged exactly once. logger.error("Exception in thread " + t, e2); - handleFSError((FSError) e2); + FileUtils.handleFSError((FSError) e2); } } } - - private void handleFSError(FSError e) - { - switch (DatabaseDescriptor.getDiskFailurePolicy()) - { - case stop: - logger.error("Stopping the gossiper and the RPC server"); - StorageService.instance.stopGossiping(); - StorageService.instance.stopRPCServer(); - break; - case best_effort: - // for both read and write errors mark the path as unwritable. - BlacklistedDirectories.maybeMarkUnwritable(e.path); - if (e instanceof FSReadError) - { - File directory = BlacklistedDirectories.maybeMarkUnreadable(e.path); - if (directory != null) - Table.removeUnreadableSSTables(directory); - } - break; - case ignore: - // already logged, so left nothing to do - break; - default: - throw new IllegalStateException(); - } - } }); // check all directories(data, commitlog, saved cache) for existence and permission http://git-wip-us.apache.org/repos/asf/cassandra/blob/15837696/test/unit/org/apache/cassandra/db/DirectoriesTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/DirectoriesTest.java b/test/unit/org/apache/cassandra/db/DirectoriesTest.java index ba6576d..21e183c 100644 --- a/test/unit/org/apache/cassandra/db/DirectoriesTest.java +++ b/test/unit/org/apache/cassandra/db/DirectoriesTest.java @@ -26,6 +26,9 @@ import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.config.Config.DiskFailurePolicy; +import org.apache.cassandra.db.Directories.DataDirectory; import org.apache.cassandra.db.compaction.LeveledManifest; import org.apache.cassandra.io.sstable.Component; import org.apache.cassandra.io.sstable.Descriptor; @@ -198,4 +201,39 @@ public class DirectoriesTest Assert.assertTrue(e.getMessage().contains(f.getPath())); } } + + @Test + public void testDiskFailurePolicy_best_effort() throws IOException + { + DiskFailurePolicy origPolicy = DatabaseDescriptor.getDiskFailurePolicy(); + + try + { + DatabaseDescriptor.setDiskFailurePolicy(DiskFailurePolicy.best_effort); + + for (DataDirectory dd : Directories.dataFileLocations) + { + dd.location.setExecutable(false); + dd.location.setWritable(false); + } + + Directories.create(KS, "bad"); + + for (DataDirectory dd : Directories.dataFileLocations) + { + File file = new File(dd.location, new File(KS, "bad").getPath()); + Assert.assertTrue(BlacklistedDirectories.isUnwritable(file)); + } + } + finally + { + for (DataDirectory dd : Directories.dataFileLocations) + { + dd.location.setExecutable(true); + dd.location.setWritable(true); + } + + DatabaseDescriptor.setDiskFailurePolicy(origPolicy); + } + } }