You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jb...@apache.org on 2012/05/04 01:35:44 UTC
[8/10] git commit: fix compaction NPE when out of disk space and
assertions disabled patch by jbellis; reviewed by xedin for CASSANDRA-3985
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/cassandra-1.1
Commit: abb39c2bc4f985b848c07d181b45efd378b4795c
Parents: f20badb
Author: Jonathan Ellis <jb...@apache.org>
Authored: Thu May 3 17:27:53 2012 -0500
Committer: Jonathan Ellis <jb...@apache.org>
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)
{