Fix memory leak in snapshot repair patch by yukim; reviewed by jbellis for CASSANDRA-6047
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/9d7bb1e0 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/9d7bb1e0 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/9d7bb1e0 Branch: refs/heads/trunk Commit: 9d7bb1e0c27fd5076afc750150ab4b86f228efb5 Parents: 7161aec Author: Yuki Morishita <yu...@apache.org> Authored: Wed Sep 18 14:19:45 2013 -0500 Committer: Yuki Morishita <yu...@apache.org> Committed: Wed Sep 18 14:19:45 2013 -0500 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../db/compaction/CompactionManager.java | 14 +++++++--- .../cassandra/io/sstable/SSTableReader.java | 28 +++++++++++++++----- 3 files changed, 33 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/9d7bb1e0/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index fb9915e..c6e1169 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -20,6 +20,7 @@ * Add SSTableDeletingNotification to DataTracker (CASSANDRA-6010) * Fix snapshots in use get deleted during snapshot repair (CASSANDRA-6011) * Move hints and exception count to o.a.c.metrics (CASSANDRA-6017) + * Fix memory leak in snapshot repair (CASSANDRA-6047) 1.2.9 http://git-wip-us.apache.org/repos/asf/cassandra/blob/9d7bb1e0/src/java/org/apache/cassandra/db/compaction/CompactionManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java index 93f3108..5c17b0a 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java @@ -714,7 +714,8 @@ public class CompactionManager implements CompactionManagerMBean Collection<SSTableReader> sstables; int gcBefore; - if (cfs.snapshotExists(validator.request.sessionid)) + boolean isSnapshotValidation = cfs.snapshotExists(validator.request.sessionid); + if (isSnapshotValidation) { // If there is a snapshot created for the session then read from there. sstables = cfs.getSnapshotSSTableReader(validator.request.sessionid); @@ -768,10 +769,17 @@ public class CompactionManager implements CompactionManagerMBean } finally { - SSTableReader.releaseReferences(sstables); iter.close(); - if (cfs.snapshotExists(validator.request.sessionid)) + if (isSnapshotValidation) + { + for (SSTableReader sstable : sstables) + FileUtils.closeQuietly(sstable); cfs.clearSnapshot(validator.request.sessionid); + } + else + { + SSTableReader.releaseReferences(sstables); + } metrics.finishCompaction(ci); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/9d7bb1e0/src/java/org/apache/cassandra/io/sstable/SSTableReader.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/SSTableReader.java b/src/java/org/apache/cassandra/io/sstable/SSTableReader.java index 6327273..ed221d9 100644 --- a/src/java/org/apache/cassandra/io/sstable/SSTableReader.java +++ b/src/java/org/apache/cassandra/io/sstable/SSTableReader.java @@ -62,7 +62,7 @@ import static org.apache.cassandra.db.Directories.SECONDARY_INDEX_NAME_SEPARATOR * SSTableReaders are open()ed by Table.onStart; after that they are created by SSTableWriter.renameAndOpen. * Do not re-call open() on existing SSTable files; use the references kept by ColumnFamilyStore post-start instead. */ -public class SSTableReader extends SSTable +public class SSTableReader extends SSTable implements Closeable { private static final Logger logger = LoggerFactory.getLogger(SSTableReader.class); @@ -345,6 +345,20 @@ public class SSTableReader extends SSTable this.bf = bloomFilter; } + /** + * Clean up all opened resources. + * + * @throws IOException + */ + public void close() throws IOException + { + // Force finalizing mmapping if necessary + ifile.cleanup(); + dfile.cleanup(); + // close the BF so it can be opened later. + bf.close(); + } + public void setTrackedBy(DataTracker tracker) { deletingTask.setTracker(tracker); @@ -969,17 +983,17 @@ public class SSTableReader extends SSTable } } + /** + * Release reference to this SSTableReader. + * If there is no one referring to this SSTable, and is marked as compacted, + * all resources are cleaned up and files are deleted eventually. + */ public void releaseReference() { if (references.decrementAndGet() == 0 && isCompacted.get()) { - // Force finalizing mmapping if necessary - ifile.cleanup(); - dfile.cleanup(); - + FileUtils.closeQuietly(this); deletingTask.schedule(); - // close the BF so it can be opened later. - FileUtils.closeQuietly(bf); } assert references.get() >= 0 : "Reference counter " + references.get() + " for " + dfile.path; }