Author: slebresne
Date: Wed Aug 31 14:50:35 2011
New Revision: 1163652

URL: http://svn.apache.org/viewvc?rev=1163652&view=rev
Log:
Fix closing sstable iterators before using them
patch by slebresne; reviewed by jbellis for CASSANDRA-3110

Modified:
    cassandra/trunk/CHANGES.txt
    cassandra/trunk/src/java/org/apache/cassandra/db/CollationController.java

Modified: cassandra/trunk/CHANGES.txt
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/CHANGES.txt?rev=1163652&r1=1163651&r2=1163652&view=diff
==============================================================================
--- cassandra/trunk/CHANGES.txt (original)
+++ cassandra/trunk/CHANGES.txt Wed Aug 31 14:50:35 2011
@@ -50,6 +50,7 @@
  * Expose gossip/FD info to JMX (CASSANDRA-2806)
  * Fix streaming over SSL when compressed SSTable involved (CASSANDRA-3051)
  * Add support for pluggable secondary index implementations (CASSANDRA-3078)
+ * Fix closing sstable iterators before using them (CASSANDRA-3110)
 
 0.8.5
  * fix NPE when encryption_options is unspecified (CASSANDRA-3007)

Modified: 
cassandra/trunk/src/java/org/apache/cassandra/db/CollationController.java
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/CollationController.java?rev=1163652&r1=1163651&r2=1163652&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/CollationController.java 
(original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/CollationController.java 
Wed Aug 31 14:50:35 2011
@@ -116,6 +116,38 @@ public class CollationController
                         container.addColumn(iter.next());
                 }
             }
+
+            // we need to distinguish between "there is no data at all for 
this row" (BF will let us rebuild that efficiently)
+            // and "there used to be data, but it's gone now" (we should cache 
the empty CF so we don't need to rebuild that slower)
+            if (iterators.isEmpty())
+                return null;
+
+            // do a final collate.  toCollate is boilerplate required to 
provide a CloseableIterator
+            final ColumnFamily c2 = container;
+            CloseableIterator<IColumn> toCollate = new 
SimpleAbstractColumnIterator()
+            {
+                final Iterator<IColumn> iter = c2.iterator();
+
+                protected IColumn computeNext()
+                {
+                    return iter.hasNext() ? iter.next() : endOfData();
+                }
+
+                public ColumnFamily getColumnFamily()
+                {
+                    return c2;
+                }
+
+                public DecoratedKey getKey()
+                {
+                    return filter.key;
+                }
+            };
+            ColumnFamily returnCF = container.cloneMeShallow();
+            filter.collateColumns(returnCF, 
Collections.singletonList(toCollate), cfs.metadata.comparator, gcBefore);
+
+            // Caller is responsible for final removeDeletedCF.  This is 
important for cacheRow to work correctly:
+            return returnCF;
         }
         finally
         {
@@ -123,38 +155,6 @@ public class CollationController
             for (IColumnIterator iter : iterators)
                 FileUtils.closeQuietly(iter);
         }
-
-        // we need to distinguish between "there is no data at all for this 
row" (BF will let us rebuild that efficiently)
-        // and "there used to be data, but it's gone now" (we should cache the 
empty CF so we don't need to rebuild that slower)
-        if (iterators.isEmpty())
-            return null;
-
-        // do a final collate.  toCollate is boilerplate required to provide a 
CloseableIterator
-        final ColumnFamily c2 = container;
-        CloseableIterator<IColumn> toCollate = new 
SimpleAbstractColumnIterator()
-        {
-            final Iterator<IColumn> iter = c2.iterator();
-
-            protected IColumn computeNext()
-            {
-                return iter.hasNext() ? iter.next() : endOfData();
-            }
-
-            public ColumnFamily getColumnFamily()
-            {
-                return c2;
-            }
-
-            public DecoratedKey getKey()
-            {
-                return filter.key;
-            }
-        };
-        ColumnFamily returnCF = container.cloneMeShallow();
-        filter.collateColumns(returnCF, Collections.singletonList(toCollate), 
cfs.metadata.comparator, gcBefore);
-
-        // Caller is responsible for final removeDeletedCF.  This is important 
for cacheRow to work correctly:
-        return returnCF;
     }
 
     /**
@@ -210,6 +210,16 @@ public class CollationController
                     sstablesIterated++;
                 }
             }
+
+            // we need to distinguish between "there is no data at all for 
this row" (BF will let us rebuild that efficiently)
+            // and "there used to be data, but it's gone now" (we should cache 
the empty CF so we don't need to rebuild that slower)
+            if (iterators.isEmpty())
+                return null;
+
+            filter.collateColumns(returnCF, iterators, 
cfs.metadata.comparator, gcBefore);
+
+            // Caller is responsible for final removeDeletedCF.  This is 
important for cacheRow to work correctly:
+            return returnCF;
         }
         finally
         {
@@ -217,16 +227,6 @@ public class CollationController
             for (IColumnIterator iter : iterators)
                 FileUtils.closeQuietly(iter);
         }
-
-        // we need to distinguish between "there is no data at all for this 
row" (BF will let us rebuild that efficiently)
-        // and "there used to be data, but it's gone now" (we should cache the 
empty CF so we don't need to rebuild that slower)
-        if (iterators.isEmpty())
-            return null;
-
-        filter.collateColumns(returnCF, iterators, cfs.metadata.comparator, 
gcBefore);
-
-        // Caller is responsible for final removeDeletedCF.  This is important 
for cacheRow to work correctly:
-        return returnCF;
     }
 
     public int getSstablesIterated()


Reply via email to