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()