Avoid second-guessing out-of-space state patch by jbellis; reviewed by yukim for CASSANDRA-5605
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/7161aec4 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/7161aec4 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/7161aec4 Branch: refs/heads/cassandra-1.2 Commit: 7161aec42c5bdb9e007587e20bc71603a505a95d Parents: d28cf3e Author: Jonathan Ellis <[email protected]> Authored: Wed Sep 18 12:12:51 2013 -0500 Committer: Jonathan Ellis <[email protected]> Committed: Wed Sep 18 12:13:23 2013 -0500 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/db/Directories.java | 62 +++++--------------- .../db/compaction/CompactionManager.java | 2 +- .../cassandra/db/compaction/Scrubber.java | 2 +- .../cassandra/io/util/DiskAwareRunnable.java | 2 +- .../apache/cassandra/streaming/StreamIn.java | 2 +- .../cassandra/db/ColumnFamilyStoreTest.java | 2 +- .../apache/cassandra/db/DirectoriesTest.java | 4 +- .../unit/org/apache/cassandra/db/ScrubTest.java | 2 +- .../cassandra/io/sstable/SSTableReaderTest.java | 3 +- .../io/sstable/SSTableSimpleWriterTest.java | 2 +- 11 files changed, 27 insertions(+), 57 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 47ff752..fb9915e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 1.2.10 + * Avoid second-guessing out-of-space state (CASSANDRA-5605) * Tuning knobs for dealing with large blobs and many CFs (CASSANDRA-5982) * (Hadoop) Fix CQLRW for thrift tables (CASSANDRA-6002) * Fix possible divide-by-zero in HHOM (CASSANDRA-5990) http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/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 0890d29..351c0c0 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -19,6 +19,7 @@ package org.apache.cassandra.db; import java.io.File; import java.io.FileFilter; +import java.io.IOError; import java.io.IOException; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; @@ -132,9 +133,9 @@ public class Directories return null; } - public File getDirectoryForNewSSTables(long estimatedSize) + public File getDirectoryForNewSSTables() { - File path = getLocationWithMaximumAvailableSpace(estimatedSize); + File path = getWriteableLocationAsFile(); // Requesting GC has a chance to free space only if we're using mmap and a non SUN jvm if (path == null @@ -154,68 +155,37 @@ public class Directories { throw new AssertionError(e); } - path = getLocationWithMaximumAvailableSpace(estimatedSize); + path = getWriteableLocationAsFile(); } return path; } - /* - * Loop through all the disks to see which disk has the max free space - * return the disk with max free space for compactions. If the size of the expected - * compacted file is greater than the max disk space available return null, we cannot - * do compaction in this case. - */ - public File getLocationWithMaximumAvailableSpace(long estimatedSize) + public File getWriteableLocationAsFile() { - long maxFreeDisk = 0; - File maxLocation = null; - - for (File dir : sstableDirectories) - { - if (BlacklistedDirectories.isUnwritable(dir)) - continue; - - long usableSpace = dir.getUsableSpace(); - if (maxFreeDisk < usableSpace) - { - maxFreeDisk = usableSpace; - maxLocation = dir; - } - } - // Load factor of 0.9 we do not want to use the entire disk that is too risky. - maxFreeDisk = (long) (0.9 * maxFreeDisk); - logger.debug(String.format("expected data files size is %d; largest free partition (%s) has %d bytes free", - estimatedSize, maxLocation, maxFreeDisk)); - - return estimatedSize < maxFreeDisk ? maxLocation : null; + return getLocationForDisk(getWriteableLocation()); } /** - * Finds location which is capable of holding given {@code estimatedSize}. - * Picks a non-blacklisted directory with most free space and least current tasks. - * If no directory can hold given {@code estimatedSize}, then returns null. + * @return a non-blacklisted directory with the most free space and least current tasks. * - * @param estimatedSize estimated size you need to find location to fit - * @return directory capable of given estimated size, or null if none found + * @throws IOError if all directories are blacklisted. */ - public DataDirectory getLocationCapableOfSize(long estimatedSize) + public DataDirectory getWriteableLocation() { List<DataDirectory> candidates = new ArrayList<DataDirectory>(); // pick directories with enough space and so that resulting sstable dirs aren't blacklisted for writes. for (DataDirectory dataDir : dataFileLocations) { - File sstableDir = getLocationForDisk(dataDir); - - if (BlacklistedDirectories.isUnwritable(sstableDir)) + if (BlacklistedDirectories.isUnwritable(getLocationForDisk(dataDir))) continue; - - // need a separate check for sstableDir itself - could be a mounted separate disk or SSD just for this CF. - if (dataDir.getEstimatedAvailableSpace() > estimatedSize && sstableDir.getUsableSpace() * 0.9 > estimatedSize) - candidates.add(dataDir); + candidates.add(dataDir); } + if (candidates.isEmpty()) + throw new IOError(new IOException("All configured data directories have been blacklisted as unwritable for erroring out")); + // sort directories by free space, in _descending_ order. Collections.sort(candidates); @@ -228,7 +198,7 @@ public class Directories } }); - return candidates.isEmpty() ? null : candidates.get(0); + return candidates.get(0); } @@ -265,7 +235,7 @@ public class Directories public long getEstimatedAvailableSpace() { // Load factor of 0.9 we do not want to use the entire disk that is too risky. - return (long)(0.9 * location.getUsableSpace()) - estimatedWorkingSize.get(); + return location.getUsableSpace() - estimatedWorkingSize.get(); } public int compareTo(DataDirectory o) http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/src/java/org/apache/cassandra/db/compaction/CompactionManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java index 4c9c707..93f3108 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java @@ -584,7 +584,7 @@ public class CompactionManager implements CompactionManagerMBean logger.info("Cleaning up " + sstable); // Calculate the expected compacted filesize long expectedRangeFileSize = cfs.getExpectedCompactedFileSize(Arrays.asList(sstable), OperationType.CLEANUP); - File compactionFileLocation = cfs.directories.getDirectoryForNewSSTables(expectedRangeFileSize); + File compactionFileLocation = cfs.directories.getDirectoryForNewSSTables(); if (compactionFileLocation == null) throw new IOException("disk full"); http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/src/java/org/apache/cassandra/db/compaction/Scrubber.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/Scrubber.java b/src/java/org/apache/cassandra/db/compaction/Scrubber.java index cb529cb..7b2178b 100644 --- a/src/java/org/apache/cassandra/db/compaction/Scrubber.java +++ b/src/java/org/apache/cassandra/db/compaction/Scrubber.java @@ -76,7 +76,7 @@ public class Scrubber implements Closeable this.outputHandler = outputHandler; // Calculate the expected compacted filesize - this.destination = cfs.directories.getDirectoryForNewSSTables(sstable.onDiskLength()); + this.destination = cfs.directories.getDirectoryForNewSSTables(); if (destination == null) throw new IOException("disk full"); http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/src/java/org/apache/cassandra/io/util/DiskAwareRunnable.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/util/DiskAwareRunnable.java b/src/java/org/apache/cassandra/io/util/DiskAwareRunnable.java index 1be4803..198a88d 100644 --- a/src/java/org/apache/cassandra/io/util/DiskAwareRunnable.java +++ b/src/java/org/apache/cassandra/io/util/DiskAwareRunnable.java @@ -34,7 +34,7 @@ public abstract class DiskAwareRunnable extends WrappedRunnable while (true) { writeSize = getExpectedWriteSize(); - directory = getDirectories().getLocationCapableOfSize(writeSize); + directory = getDirectories().getWriteableLocation(); if (directory != null || !reduceScopeForLimitedSpace()) break; } http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/src/java/org/apache/cassandra/streaming/StreamIn.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/streaming/StreamIn.java b/src/java/org/apache/cassandra/streaming/StreamIn.java index 740b430..85ea7fa 100644 --- a/src/java/org/apache/cassandra/streaming/StreamIn.java +++ b/src/java/org/apache/cassandra/streaming/StreamIn.java @@ -80,7 +80,7 @@ public class StreamIn // new local sstable Table table = Table.open(remotedesc.ksname); ColumnFamilyStore cfStore = table.getColumnFamilyStore(remotedesc.cfname); - Directories.DataDirectory localDir = cfStore.directories.getLocationCapableOfSize(remote.size); + Directories.DataDirectory localDir = cfStore.directories.getWriteableLocation(); if (localDir == null) throw new RuntimeException("Insufficient disk space to store " + remote.size + " bytes"); Descriptor localdesc = Descriptor.fromFilename(cfStore.getTempSSTablePath(cfStore.directories.getLocationForDisk(localDir))); http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index a394644..abe3f05 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -837,7 +837,7 @@ public class ColumnFamilyStoreTest extends SchemaLoader for (int version = 1; version <= 2; ++version) { - Descriptor existing = new Descriptor(cfs.directories.getDirectoryForNewSSTables(1), "Keyspace2", "Standard1", version, false); + Descriptor existing = new Descriptor(cfs.directories.getDirectoryForNewSSTables(), "Keyspace2", "Standard1", version, false); Descriptor desc = new Descriptor(Directories.getBackupsDirectory(existing), "Keyspace2", "Standard1", version, false); for (Component c : new Component[]{ Component.DATA, Component.PRIMARY_INDEX, Component.FILTER, Component.STATS }) assertTrue("can not find backedup file:" + desc.filenameFor(c), new File(desc.filenameFor(c)).exists()); http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/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 21e183c..dce6f87 100644 --- a/test/unit/org/apache/cassandra/db/DirectoriesTest.java +++ b/test/unit/org/apache/cassandra/db/DirectoriesTest.java @@ -107,7 +107,7 @@ public class DirectoriesTest for (String cf : CFS) { Directories directories = Directories.create(KS, cf); - Assert.assertEquals(cfDir(cf), directories.getDirectoryForNewSSTables(0)); + Assert.assertEquals(cfDir(cf), directories.getDirectoryForNewSSTables()); Descriptor desc = new Descriptor(cfDir(cf), KS, cf, 1, false); File snapshotDir = new File(cfDir(cf), File.separator + Directories.SNAPSHOT_SUBDIR + File.separator + "42"); @@ -180,7 +180,7 @@ public class DirectoriesTest { /* files not matching the pattern should just be ignored, with a log warning */ Directories directories = Directories.create(KS, "bad"); - File dir = directories.getDirectoryForNewSSTables(1); + File dir = directories.getDirectoryForNewSSTables(); File f = File.createTempFile("bad", "file", dir.getParentFile()); Directories.migrateSSTables(); Assert.assertTrue(f.isFile()); http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/test/unit/org/apache/cassandra/db/ScrubTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/ScrubTest.java b/test/unit/org/apache/cassandra/db/ScrubTest.java index 26f0e78..c26939a 100644 --- a/test/unit/org/apache/cassandra/db/ScrubTest.java +++ b/test/unit/org/apache/cassandra/db/ScrubTest.java @@ -55,7 +55,7 @@ public class ScrubTest extends SchemaLoader File rootDir = new File(root); assert rootDir.isDirectory(); - File destDir = Directories.create(TABLE, cf).getDirectoryForNewSSTables(1); + File destDir = Directories.create(TABLE, cf).getDirectoryForNewSSTables(); String corruptSSTableName = null; http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java index d0670a0..02b6855 100644 --- a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java @@ -28,7 +28,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.ExecutionException; -import java.util.*; import org.junit.Test; import org.junit.runner.RunWith; @@ -230,7 +229,7 @@ public class SSTableReaderTest extends SchemaLoader File rootDir = new File(root + File.separator + "hb" + File.separator + "Keyspace1"); assert rootDir.isDirectory(); - File destDir = Directories.create("Keyspace1", "Indexed1").getDirectoryForNewSSTables(0); + File destDir = Directories.create("Keyspace1", "Indexed1").getDirectoryForNewSSTables(); assert destDir != null; FileUtils.createDirectory(destDir); http://git-wip-us.apache.org/repos/asf/cassandra/blob/7161aec4/test/unit/org/apache/cassandra/io/sstable/SSTableSimpleWriterTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/io/sstable/SSTableSimpleWriterTest.java b/test/unit/org/apache/cassandra/io/sstable/SSTableSimpleWriterTest.java index 6efdc9b..ce569b9 100644 --- a/test/unit/org/apache/cassandra/io/sstable/SSTableSimpleWriterTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/SSTableSimpleWriterTest.java @@ -43,7 +43,7 @@ public class SSTableSimpleWriterTest extends SchemaLoader String cfname = "StandardInteger1"; Table t = Table.open(tablename); // make sure we create the directory - File dir = Directories.create(tablename, cfname).getDirectoryForNewSSTables(0); + File dir = Directories.create(tablename, cfname).getDirectoryForNewSSTables(); assert dir.exists(); IPartitioner partitioner = StorageService.getPartitioner();
