fix FD leak in slice queries patch by jbellis; reviewed by Sam Tunnicliffe and tested by brandonwilliams
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/e1b10590 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/e1b10590 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/e1b10590 Branch: refs/heads/trunk Commit: e1b10590e84189b92af168e33a63c14c3ca1f5fa Parents: 5c91bd1 Author: Jonathan Ellis <jbel...@apache.org> Authored: Tue Sep 4 11:12:28 2012 -0500 Committer: Jonathan Ellis <jbel...@apache.org> Committed: Tue Sep 4 13:55:36 2012 -0500 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/db/CollationController.java | 19 +++++++++++---- .../db/columniterator/ISSTableColumnIterator.java | 8 ++++++ .../db/columniterator/SSTableNamesIterator.java | 10 +++++++- .../db/columniterator/SSTableSliceIterator.java | 10 +++++++- .../org/apache/cassandra/db/filter/IFilter.java | 5 ++- .../cassandra/db/filter/NamesQueryFilter.java | 5 ++- .../apache/cassandra/db/filter/QueryFilter.java | 5 ++- .../cassandra/db/filter/SliceQueryFilter.java | 5 ++- 9 files changed, 53 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index b23e9ba..6c504e8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 1.1.5 + * fix FD leak in slice queries (CASSANDRA-4571) * avoid recursion in leveled compaction (CASSANDRA-4587) * increase stack size under Java7 to 180K * Log(info) schema changes (CASSANDRA-4547) http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/CollationController.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/CollationController.java b/src/java/org/apache/cassandra/db/CollationController.java index 8121062..fb9674f 100644 --- a/src/java/org/apache/cassandra/db/CollationController.java +++ b/src/java/org/apache/cassandra/db/CollationController.java @@ -29,6 +29,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.cassandra.db.columniterator.IColumnIterator; +import org.apache.cassandra.db.columniterator.ISSTableColumnIterator; import org.apache.cassandra.db.columniterator.SimpleAbstractColumnIterator; import org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy; import org.apache.cassandra.db.filter.NamesQueryFilter; @@ -245,7 +246,6 @@ public class CollationController } long mostRecentRowTombstone = Long.MIN_VALUE; - Map<IColumnIterator, Long> iteratorMaxTimes = Maps.newHashMapWithExpectedSize(view.sstables.size()); for (SSTableReader sstable : view.sstables) { // if we've already seen a row tombstone with a timestamp greater @@ -254,7 +254,7 @@ public class CollationController continue; IColumnIterator iter = filter.getSSTableColumnIterator(sstable); - iteratorMaxTimes.put(iter, sstable.getMaxTimestamp()); + iterators.add(iter); if (iter.getColumnFamily() != null) { ColumnFamily cf = iter.getColumnFamily(); @@ -269,10 +269,19 @@ public class CollationController // If we saw a row tombstone, do a second pass through the iterators we // obtained from the sstables and drop any whose maxTimestamp < that of the // row tombstone - for (Map.Entry<IColumnIterator, Long> entry : iteratorMaxTimes.entrySet()) + if (mostRecentRowTombstone > Long.MIN_VALUE) { - if (entry.getValue() >= mostRecentRowTombstone) - iterators.add(entry.getKey()); + Iterator<IColumnIterator> it = iterators.iterator(); + while (it.hasNext()) + { + IColumnIterator iter = it.next(); + if ((iter instanceof ISSTableColumnIterator) + && ((ISSTableColumnIterator) iter).getSStable().getMaxTimestamp() < mostRecentRowTombstone) + { + FileUtils.closeQuietly(iter); + it.remove(); + } + } } // we need to distinguish between "there is no data at all for this row" (BF will let us rebuild that efficiently) http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/columniterator/ISSTableColumnIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/columniterator/ISSTableColumnIterator.java b/src/java/org/apache/cassandra/db/columniterator/ISSTableColumnIterator.java new file mode 100644 index 0000000..4da4c0a --- /dev/null +++ b/src/java/org/apache/cassandra/db/columniterator/ISSTableColumnIterator.java @@ -0,0 +1,8 @@ +package org.apache.cassandra.db.columniterator; + +import org.apache.cassandra.io.sstable.SSTableReader; + +public interface ISSTableColumnIterator extends IColumnIterator +{ + public SSTableReader getSStable(); +} http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/columniterator/SSTableNamesIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableNamesIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableNamesIterator.java index d5d999f..23036df 100644 --- a/src/java/org/apache/cassandra/db/columniterator/SSTableNamesIterator.java +++ b/src/java/org/apache/cassandra/db/columniterator/SSTableNamesIterator.java @@ -43,7 +43,7 @@ import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.Filter; -public class SSTableNamesIterator extends SimpleAbstractColumnIterator implements IColumnIterator +public class SSTableNamesIterator extends SimpleAbstractColumnIterator implements ISSTableColumnIterator { private static Logger logger = LoggerFactory.getLogger(SSTableNamesIterator.class); @@ -51,9 +51,11 @@ public class SSTableNamesIterator extends SimpleAbstractColumnIterator implement private Iterator<IColumn> iter; public final SortedSet<ByteBuffer> columns; public final DecoratedKey key; + private final SSTableReader sstable; public SSTableNamesIterator(SSTableReader sstable, DecoratedKey key, SortedSet<ByteBuffer> columns) { + this.sstable = sstable; assert columns != null; this.columns = columns; this.key = key; @@ -84,6 +86,7 @@ public class SSTableNamesIterator extends SimpleAbstractColumnIterator implement public SSTableNamesIterator(SSTableReader sstable, FileDataInput file, DecoratedKey key, SortedSet<ByteBuffer> columns) { + this.sstable = sstable; assert columns != null; this.columns = columns; this.key = key; @@ -99,6 +102,11 @@ public class SSTableNamesIterator extends SimpleAbstractColumnIterator implement } } + public SSTableReader getSStable() + { + return sstable; + } + private void read(SSTableReader sstable, FileDataInput file) throws IOException { http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/columniterator/SSTableSliceIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableSliceIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableSliceIterator.java index 8e4fc06..38335f4 100644 --- a/src/java/org/apache/cassandra/db/columniterator/SSTableSliceIterator.java +++ b/src/java/org/apache/cassandra/db/columniterator/SSTableSliceIterator.java @@ -36,14 +36,16 @@ import org.apache.cassandra.utils.ByteBufferUtil; /** * A Column Iterator over SSTable */ -public class SSTableSliceIterator implements IColumnIterator +public class SSTableSliceIterator implements ISSTableColumnIterator { private final FileDataInput fileToClose; private IColumnIterator reader; + private final SSTableReader sstable; private DecoratedKey key; public SSTableSliceIterator(SSTableReader sstable, DecoratedKey key, ByteBuffer startColumn, ByteBuffer finishColumn, boolean reversed) { + this.sstable = sstable; this.key = key; fileToClose = sstable.getFileDataInput(this.key); if (fileToClose == null) @@ -81,6 +83,7 @@ public class SSTableSliceIterator implements IColumnIterator */ public SSTableSliceIterator(SSTableReader sstable, FileDataInput file, DecoratedKey key, ByteBuffer startColumn, ByteBuffer finishColumn, boolean reversed) { + this.sstable = sstable; this.key = key; fileToClose = null; reader = createReader(sstable, file, startColumn, finishColumn, reversed); @@ -93,6 +96,11 @@ public class SSTableSliceIterator implements IColumnIterator : new IndexedSliceReader(sstable, file, startColumn, finishColumn, reversed); } + public SSTableReader getSStable() + { + return sstable; + } + public DecoratedKey getKey() { return key; http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/filter/IFilter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/filter/IFilter.java b/src/java/org/apache/cassandra/db/filter/IFilter.java index de26e83..355d3c6 100644 --- a/src/java/org/apache/cassandra/db/filter/IFilter.java +++ b/src/java/org/apache/cassandra/db/filter/IFilter.java @@ -25,6 +25,7 @@ import java.util.Iterator; import org.apache.cassandra.db.*; import org.apache.cassandra.db.columniterator.IColumnIterator; +import org.apache.cassandra.db.columniterator.ISSTableColumnIterator; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.io.sstable.SSTableReader; import org.apache.cassandra.io.util.FileDataInput; @@ -51,13 +52,13 @@ public interface IFilter * @param file Already opened file data input, saves us opening another one * @param key The key of the row we are about to iterate over */ - public abstract IColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key); + public abstract ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key); /** * returns an iterator that returns columns from the given SSTable * matching the Filter criteria in sorted order. */ - public abstract IColumnIterator getSSTableColumnIterator(SSTableReader sstable, DecoratedKey<?> key); + public abstract ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable, DecoratedKey<?> key); /** * collects columns from reducedColumns into returnCF. Termination is determined http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java b/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java index d6ab552..6db0aee 100644 --- a/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java @@ -30,6 +30,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.cassandra.db.*; import org.apache.cassandra.db.columniterator.IColumnIterator; +import org.apache.cassandra.db.columniterator.ISSTableColumnIterator; import org.apache.cassandra.db.columniterator.SSTableNamesIterator; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.io.sstable.SSTableReader; @@ -55,12 +56,12 @@ public class NamesQueryFilter implements IFilter return Memtable.getNamesIterator(key, cf, this); } - public IColumnIterator getSSTableColumnIterator(SSTableReader sstable, DecoratedKey<?> key) + public ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable, DecoratedKey<?> key) { return new SSTableNamesIterator(sstable, key, columns); } - public IColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key) + public ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key) { return new SSTableNamesIterator(sstable, file, key, columns); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/filter/QueryFilter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/filter/QueryFilter.java b/src/java/org/apache/cassandra/db/filter/QueryFilter.java index 1e83bf4..08a4f70 100644 --- a/src/java/org/apache/cassandra/db/filter/QueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/QueryFilter.java @@ -29,6 +29,7 @@ import org.slf4j.LoggerFactory; import org.apache.cassandra.db.*; import org.apache.cassandra.db.columniterator.IColumnIterator; +import org.apache.cassandra.db.columniterator.ISSTableColumnIterator; import org.apache.cassandra.db.columniterator.IdentityQueryFilter; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.io.sstable.SSTableReader; @@ -72,14 +73,14 @@ public class QueryFilter } // TODO move gcBefore into a field - public IColumnIterator getSSTableColumnIterator(SSTableReader sstable) + public ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable) { if (path.superColumnName == null) return filter.getSSTableColumnIterator(sstable, key); return superFilter.getSSTableColumnIterator(sstable, key); } - public IColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key) + public ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key) { if (path.superColumnName == null) return filter.getSSTableColumnIterator(sstable, file, key); http://git-wip-us.apache.org/repos/asf/cassandra/blob/e1b10590/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java index 1a4a912..e749719 100644 --- a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java @@ -34,6 +34,7 @@ import org.slf4j.LoggerFactory; import org.apache.cassandra.db.*; import org.apache.cassandra.db.columniterator.IColumnIterator; +import org.apache.cassandra.db.columniterator.ISSTableColumnIterator; import org.apache.cassandra.db.columniterator.SSTableSliceIterator; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.io.sstable.SSTableReader; @@ -61,12 +62,12 @@ public class SliceQueryFilter implements IFilter return Memtable.getSliceIterator(key, cf, this); } - public IColumnIterator getSSTableColumnIterator(SSTableReader sstable, DecoratedKey<?> key) + public ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable, DecoratedKey<?> key) { return new SSTableSliceIterator(sstable, key, start, finish, reversed); } - public IColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key) + public ISSTableColumnIterator getSSTableColumnIterator(SSTableReader sstable, FileDataInput file, DecoratedKey<?> key) { return new SSTableSliceIterator(sstable, file, key, start, finish, reversed); }