fix compaction NPE when out of disk space and assertions disabled patch by jbellis; reviewed by xedin for CASSANDRA-3985
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/abb39c2b Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/abb39c2b Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/abb39c2b Branch: refs/heads/trunk Commit: abb39c2bc4f985b848c07d181b45efd378b4795c Parents: f20badb Author: Jonathan Ellis <[email protected]> Authored: Thu May 3 17:27:53 2012 -0500 Committer: Jonathan Ellis <[email protected]> Committed: Thu May 3 18:00:49 2012 -0500 ---------------------------------------------------------------------- CHANGES.txt | 4 +- .../cassandra/config/DatabaseDescriptor.java | 29 ++++----------- src/java/org/apache/cassandra/db/Table.java | 15 ++++---- .../cassandra/db/compaction/CompactionTask.java | 22 ++++-------- 4 files changed, 24 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/abb39c2b/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index ad301db..1c1c184 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -12,6 +12,8 @@ * fix stress tool that hangs forever on timeout or error (CASSANDRA-4128) * Fix super columns bug where cache is not updated (CASSANDRA-4190) * stress tool to return appropriate exit code on failure (CASSANDRA-4188) + * fix compaction NPE when out of disk space and assertions disabled + (CASSANDRA-3985) 1.0.9 @@ -27,8 +29,6 @@ * don't change manifest level for cleanup, scrub, and upgradesstables operations under LeveledCompactionStrategy (CASSANDRA-3989, 4112) * fix race leading to super columns assertion failure (CASSANDRA-3957) - * ensure that directory is selected for compaction for user-defined - tasks and upgradesstables (CASSANDRA-3985) * fix NPE on invalid CQL delete command (CASSANDRA-3755) * allow custom types in CLI's assume command (CASSANDRA-4081) * fix totalBytes count for parallel compactions (CASSANDRA-3758) http://git-wip-us.apache.org/repos/asf/cassandra/blob/abb39c2b/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 09686d2..8213201 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -757,23 +757,19 @@ public class DatabaseDescriptor return Collections.unmodifiableSet(new HashSet(seedProvider.getSeeds())); } - public synchronized static String getDataFileLocationForTable(String table, long expectedCompactedFileSize) - { - return getDataFileLocationForTable(table, expectedCompactedFileSize, true); - } - /* * 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. * + * Should only be called by Table.getDataFileLocation, which knows how to free up extra space under + * some contitions to retry. (Left public because some test methods cheat and call this directly.) + * * @param table name of the table. * @param expectedCompactedSize expected file size in bytes. - * @param ensureFreeSpace Flag if the function should ensure enough free space exists for the expected file size. - * If False and there is not enough free space a warning is logged, and the dir with the most space is returned. */ - public synchronized static String getDataFileLocationForTable(String table, long expectedCompactedFileSize, boolean ensureFreeSpace) + public synchronized static String getDataFileLocationForTable(String table, long expectedCompactedFileSize) { long maxFreeDisk = 0; int maxDiskIndex = 0; @@ -791,22 +787,13 @@ public class DatabaseDescriptor } } - logger.debug("expected data files size is {}; largest free partition has {} bytes free", - expectedCompactedFileSize, - maxFreeDisk); - // Load factor of 0.9 we do not want to use the entire disk that is too risky. maxFreeDisk = (long) (0.9 * maxFreeDisk); - if (!ensureFreeSpace || expectedCompactedFileSize < maxFreeDisk) - { - dataFileDirectory = dataDirectoryForTable[maxDiskIndex]; + logger.debug("expected data files size is {}; largest free partition has {} bytes usable", + expectedCompactedFileSize, maxFreeDisk); - if (expectedCompactedFileSize >= maxFreeDisk) - logger.warn(String.format("Data file location %s only has %d free, expected size is %d", - dataFileDirectory, - maxFreeDisk, - expectedCompactedFileSize)); - } + if (expectedCompactedFileSize < maxFreeDisk) + dataFileDirectory = dataDirectoryForTable[maxDiskIndex]; return dataFileDirectory; } http://git-wip-us.apache.org/repos/asf/cassandra/blob/abb39c2b/src/java/org/apache/cassandra/db/Table.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/Table.java b/src/java/org/apache/cassandra/db/Table.java index 4c14f0f..d3a38db 100644 --- a/src/java/org/apache/cassandra/db/Table.java +++ b/src/java/org/apache/cassandra/db/Table.java @@ -556,17 +556,15 @@ public class Table public String getDataFileLocation(long expectedSize) { - return getDataFileLocation(expectedSize, true); - } + String path = DatabaseDescriptor.getDataFileLocationForTable(name, expectedSize); - public String getDataFileLocation(long expectedSize, boolean ensureFreeSpace) - { - String path = DatabaseDescriptor.getDataFileLocationForTable(name, expectedSize, ensureFreeSpace); // Requesting GC has a chance to free space only if we're using mmap and a non SUN jvm if (path == null - && (DatabaseDescriptor.getDiskAccessMode() == Config.DiskAccessMode.mmap || DatabaseDescriptor.getIndexAccessMode() == Config.DiskAccessMode.mmap) - && !MmappedSegmentedFile.isCleanerAvailable()) + && (DatabaseDescriptor.getDiskAccessMode() == Config.DiskAccessMode.mmap || DatabaseDescriptor.getIndexAccessMode() == Config.DiskAccessMode.mmap) + && !MmappedSegmentedFile.isCleanerAvailable()) { + logger.info("Forcing GC to free up disk space. Upgrade to the Oracle JVM to avoid this"); + StorageService.instance.requestGC(); // retry after GCing has forced unmap of compacted SSTables so they can be deleted // Note: GCInspector will do this already, but only sun JVM supports GCInspector so far @@ -579,8 +577,9 @@ public class Table { throw new AssertionError(e); } - path = DatabaseDescriptor.getDataFileLocationForTable(name, expectedSize, ensureFreeSpace); + path = DatabaseDescriptor.getDataFileLocationForTable(name, expectedSize); } + return path; } http://git-wip-us.apache.org/repos/asf/cassandra/blob/abb39c2b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java index 7f389bd..2a1b415 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java @@ -77,7 +77,7 @@ public class CompactionTask extends AbstractCompactionTask return 0; if (compactionFileLocation == null) - compactionFileLocation = cfs.table.getDataFileLocation(cfs.getExpectedCompactedFileSize(toCompact), ensureFreeSpace()); + compactionFileLocation = cfs.table.getDataFileLocation(cfs.getExpectedCompactedFileSize(toCompact)); if (compactionFileLocation == null && partialCompactionsAcceptable()) { @@ -89,17 +89,14 @@ public class CompactionTask extends AbstractCompactionTask // Note that we have removed files that are still marked as compacting. // This suboptimal but ok since the caller will unmark all the sstables at the end. toCompact.remove(cfs.getMaxSizeFile(toCompact)); - compactionFileLocation = cfs.table.getDataFileLocation(cfs.getExpectedCompactedFileSize(toCompact), - ensureFreeSpace()); - } - - if (compactionFileLocation == null) - { - logger.warn("insufficient space to compact even the two smallest files, aborting"); - return 0; + compactionFileLocation = cfs.table.getDataFileLocation(cfs.getExpectedCompactedFileSize(toCompact)); } } - assert compactionFileLocation != null; + if (compactionFileLocation == null) + { + logger.warn("insufficient space to compact; aborting compaction"); + return 0; + } if (DatabaseDescriptor.isSnapshotBeforeCompaction()) cfs.snapshotWithoutFlush(System.currentTimeMillis() + "-" + "compact-" + cfs.columnFamily); @@ -231,11 +228,6 @@ public class CompactionTask extends AbstractCompactionTask return !isUserDefined; } - protected boolean ensureFreeSpace() - { - return !isUserDefined; - } - //extensibility point for other strategies that may want to limit the upper bounds of the sstable segment size protected boolean newSSTableSegmentThresholdReached(SSTableWriter writer, long position) {
