cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jbel...@apache.org
Subject [7/10] git commit: fix compaction NPE when out of disk space and assertions disabled patch by jbellis; reviewed by xedin for CASSANDRA-3985
Date Thu, 03 May 2012 23:35:44 GMT
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 <jbellis@apache.org>
Authored: Thu May 3 17:27:53 2012 -0500
Committer: Jonathan Ellis <jbellis@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)
     {


Mime
View raw message