Repository: cassandra Updated Branches: refs/heads/trunk d478f45d1 -> f2a354763
Clean up the SSTableReader#getScanner API wrt removal of RateLimiter patch by agrasso reviewed by dbrosius for cassandra-12422 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/f2a35476 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f2a35476 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f2a35476 Branch: refs/heads/trunk Commit: f2a354763877cfeaf1dd017b84a7c8ee9eafd885 Parents: d478f45 Author: Anthony Grasso <anthony.gra...@gmail.com> Authored: Mon Oct 17 19:25:58 2016 -0400 Committer: Dave Brosius <dbros...@mebigfatguy.com> Committed: Mon Oct 17 19:25:58 2016 -0400 ---------------------------------------------------------------------- CHANGES.txt | 2 +- .../compaction/AbstractCompactionStrategy.java | 2 +- .../db/compaction/CompactionManager.java | 12 +++--- .../compaction/LeveledCompactionStrategy.java | 6 +-- .../io/sstable/format/SSTableReader.java | 32 +++------------- .../io/sstable/format/big/BigTableReader.java | 16 ++++---- .../io/sstable/format/big/BigTableScanner.java | 19 +++++----- .../db/compaction/AntiCompactionTest.java | 4 +- .../cassandra/io/sstable/SSTableReaderTest.java | 2 +- .../io/sstable/SSTableScannerTest.java | 40 ++++++++------------ 10 files changed, 51 insertions(+), 84 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 97bdd3b..c7b3206 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -3,7 +3,7 @@ * Add (automate) Nodetool Documentation (CASSANDRA-12672) * Update bundled cqlsh python driver to 3.7.0 (CASSANDRA-12736) * Reject invalid replication settings when creating or altering a keyspace (CASSANDRA-12681) - + * Clean up the SSTableReader#getScanner API wrt removal of RateLimiter (CASSANDRA-12422) 3.10 * Improve sum aggregate functions (CASSANDRA-12417) http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java index 8454147..fccad19 100644 --- a/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java +++ b/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java @@ -286,7 +286,7 @@ public abstract class AbstractCompactionStrategy try { for (SSTableReader sstable : sstables) - scanners.add(sstable.getScanner(ranges, null)); + scanners.add(sstable.getScanner(ranges)); } catch (Throwable t) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/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 a0dc8c9..2099a66 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java @@ -1084,7 +1084,7 @@ public class CompactionManager implements CompactionManagerMBean int nowInSec = FBUtilities.nowInSeconds(); try (SSTableRewriter writer = SSTableRewriter.construct(cfs, txn, false, sstable.maxDataAge); - ISSTableScanner scanner = cleanupStrategy.getScanner(sstable, null); + ISSTableScanner scanner = cleanupStrategy.getScanner(sstable); CompactionController controller = new CompactionController(cfs, txn.originals(), getDefaultGcBefore(cfs, nowInSec)); CompactionIterator ci = new CompactionIterator(OperationType.CLEANUP, Collections.singletonList(scanner), controller, nowInSec, UUIDGen.getTimeUUID(), metrics)) { @@ -1167,7 +1167,7 @@ public class CompactionManager implements CompactionManagerMBean : new Bounded(cfs, ranges, nowInSec); } - public abstract ISSTableScanner getScanner(SSTableReader sstable, RateLimiter limiter); + public abstract ISSTableScanner getScanner(SSTableReader sstable); public abstract UnfilteredRowIterator cleanup(UnfilteredRowIterator partition); private static final class Bounded extends CleanupStrategy @@ -1186,9 +1186,9 @@ public class CompactionManager implements CompactionManagerMBean } @Override - public ISSTableScanner getScanner(SSTableReader sstable, RateLimiter limiter) + public ISSTableScanner getScanner(SSTableReader sstable) { - return sstable.getScanner(ranges, limiter); + return sstable.getScanner(ranges); } @Override @@ -1209,9 +1209,9 @@ public class CompactionManager implements CompactionManagerMBean } @Override - public ISSTableScanner getScanner(SSTableReader sstable, RateLimiter limiter) + public ISSTableScanner getScanner(SSTableReader sstable) { - return sstable.getScanner(limiter); + return sstable.getScanner(); } @Override http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java index 0633251f..5d72aa5 100644 --- a/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java +++ b/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java @@ -275,7 +275,7 @@ public class LeveledCompactionStrategy extends AbstractCompactionStrategy { // L0 makes no guarantees about overlapping-ness. Just create a direct scanner for each for (SSTableReader sstable : byLevel.get(level)) - scanners.add(sstable.getScanner(ranges, null)); + scanners.add(sstable.getScanner(ranges)); } else { @@ -365,7 +365,7 @@ public class LeveledCompactionStrategy extends AbstractCompactionStrategy sstableIterator = this.sstables.iterator(); assert sstableIterator.hasNext(); // caller should check intersecting first SSTableReader currentSSTable = sstableIterator.next(); - currentScanner = currentSSTable.getScanner(ranges, null); + currentScanner = currentSSTable.getScanner(ranges); } @@ -419,7 +419,7 @@ public class LeveledCompactionStrategy extends AbstractCompactionStrategy return endOfData(); } SSTableReader currentSSTable = sstableIterator.next(); - currentScanner = currentSSTable.getScanner(ranges, null); + currentScanner = currentSSTable.getScanner(ranges); } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java index 45d4ace..b11f498 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java +++ b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java @@ -1664,37 +1664,17 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS return isSuspect.get(); } - - /** - * I/O SSTableScanner - * @return A Scanner for seeking over the rows of the SSTable. - */ - public ISSTableScanner getScanner() - { - return getScanner((RateLimiter) null); - } - - /** - * @param columns the columns to return. - * @param dataRange filter to use when reading the columns - * @return A Scanner for seeking over the rows of the SSTable. - */ - public ISSTableScanner getScanner(ColumnFilter columns, DataRange dataRange, boolean isForThrift) - { - return getScanner(columns, dataRange, null, isForThrift); - } - /** * Direct I/O SSTableScanner over a defined range of tokens. * * @param range the range of keys to cover * @return A Scanner for seeking over the rows of the SSTable. */ - public ISSTableScanner getScanner(Range<Token> range, RateLimiter limiter) + public ISSTableScanner getScanner(Range<Token> range) { if (range == null) - return getScanner(limiter); - return getScanner(Collections.singletonList(range), limiter); + return getScanner(); + return getScanner(Collections.singletonList(range)); } /** @@ -1702,7 +1682,7 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS * * @return A Scanner over the full content of the SSTable. */ - public abstract ISSTableScanner getScanner(RateLimiter limiter); + public abstract ISSTableScanner getScanner(); /** * Direct I/O SSTableScanner over a defined collection of ranges of tokens. @@ -1710,7 +1690,7 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS * @param ranges the range of keys to cover * @return A Scanner for seeking over the rows of the SSTable. */ - public abstract ISSTableScanner getScanner(Collection<Range<Token>> ranges, RateLimiter limiter); + public abstract ISSTableScanner getScanner(Collection<Range<Token>> ranges); /** * Direct I/O SSTableScanner over an iterator of bounds. @@ -1725,7 +1705,7 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS * @param dataRange filter to use when reading the columns * @return A Scanner for seeking over the rows of the SSTable. */ - public abstract ISSTableScanner getScanner(ColumnFilter columns, DataRange dataRange, RateLimiter limiter, boolean isForThrift); + public abstract ISSTableScanner getScanner(ColumnFilter columns, DataRange dataRange, boolean isForThrift); public FileDataInput getFileDataInput(long position) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/src/java/org/apache/cassandra/io/sstable/format/big/BigTableReader.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/format/big/BigTableReader.java b/src/java/org/apache/cassandra/io/sstable/format/big/BigTableReader.java index 8c64b01..8deb685 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/big/BigTableReader.java +++ b/src/java/org/apache/cassandra/io/sstable/format/big/BigTableReader.java @@ -17,7 +17,6 @@ */ package org.apache.cassandra.io.sstable.format.big; -import com.google.common.util.concurrent.RateLimiter; import org.apache.cassandra.cache.KeyCacheKey; import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.*; @@ -34,7 +33,6 @@ import org.apache.cassandra.io.sstable.*; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.io.sstable.metadata.StatsMetadata; import org.apache.cassandra.io.util.FileDataInput; -import org.apache.cassandra.io.util.RandomAccessReader; import org.apache.cassandra.tracing.Tracing; import org.apache.cassandra.utils.ByteBufferUtil; import org.slf4j.Logger; @@ -77,9 +75,9 @@ public class BigTableReader extends SSTableReader * @param dataRange filter to use when reading the columns * @return A Scanner for seeking over the rows of the SSTable. */ - public ISSTableScanner getScanner(ColumnFilter columns, DataRange dataRange, RateLimiter limiter, boolean isForThrift) + public ISSTableScanner getScanner(ColumnFilter columns, DataRange dataRange, boolean isForThrift) { - return BigTableScanner.getScanner(this, columns, dataRange, limiter, isForThrift); + return BigTableScanner.getScanner(this, columns, dataRange, isForThrift); } /** @@ -98,9 +96,9 @@ public class BigTableReader extends SSTableReader * * @return A Scanner for reading the full SSTable. */ - public ISSTableScanner getScanner(RateLimiter limiter) + public ISSTableScanner getScanner() { - return BigTableScanner.getScanner(this, limiter); + return BigTableScanner.getScanner(this); } /** @@ -109,12 +107,12 @@ public class BigTableReader extends SSTableReader * @param ranges the range of keys to cover * @return A Scanner for seeking over the rows of the SSTable. */ - public ISSTableScanner getScanner(Collection<Range<Token>> ranges, RateLimiter limiter) + public ISSTableScanner getScanner(Collection<Range<Token>> ranges) { if (ranges != null) - return BigTableScanner.getScanner(this, ranges, limiter); + return BigTableScanner.getScanner(this, ranges); else - return getScanner(limiter); + return getScanner(); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/src/java/org/apache/cassandra/io/sstable/format/big/BigTableScanner.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/format/big/BigTableScanner.java b/src/java/org/apache/cassandra/io/sstable/format/big/BigTableScanner.java index e465a02..5260b3a 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/big/BigTableScanner.java +++ b/src/java/org/apache/cassandra/io/sstable/format/big/BigTableScanner.java @@ -23,7 +23,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.apache.cassandra.utils.AbstractIterator; import com.google.common.collect.Iterators; -import com.google.common.util.concurrent.RateLimiter; import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.*; @@ -68,36 +67,36 @@ public class BigTableScanner implements ISSTableScanner protected Iterator<UnfilteredRowIterator> iterator; // Full scan of the sstables - public static ISSTableScanner getScanner(SSTableReader sstable, RateLimiter limiter) + public static ISSTableScanner getScanner(SSTableReader sstable) { - return new BigTableScanner(sstable, ColumnFilter.all(sstable.metadata), null, limiter, false, Iterators.singletonIterator(fullRange(sstable))); + return new BigTableScanner(sstable, ColumnFilter.all(sstable.metadata), null, false, Iterators.singletonIterator(fullRange(sstable))); } - public static ISSTableScanner getScanner(SSTableReader sstable, ColumnFilter columns, DataRange dataRange, RateLimiter limiter, boolean isForThrift) + public static ISSTableScanner getScanner(SSTableReader sstable, ColumnFilter columns, DataRange dataRange, boolean isForThrift) { - return new BigTableScanner(sstable, columns, dataRange, limiter, isForThrift, makeBounds(sstable, dataRange).iterator()); + return new BigTableScanner(sstable, columns, dataRange, isForThrift, makeBounds(sstable, dataRange).iterator()); } - public static ISSTableScanner getScanner(SSTableReader sstable, Collection<Range<Token>> tokenRanges, RateLimiter limiter) + public static ISSTableScanner getScanner(SSTableReader sstable, Collection<Range<Token>> tokenRanges) { // We want to avoid allocating a SSTableScanner if the range don't overlap the sstable (#5249) List<Pair<Long, Long>> positions = sstable.getPositionsForRanges(tokenRanges); if (positions.isEmpty()) return new EmptySSTableScanner(sstable); - return new BigTableScanner(sstable, ColumnFilter.all(sstable.metadata), null, limiter, false, makeBounds(sstable, tokenRanges).iterator()); + return new BigTableScanner(sstable, ColumnFilter.all(sstable.metadata), null, false, makeBounds(sstable, tokenRanges).iterator()); } public static ISSTableScanner getScanner(SSTableReader sstable, Iterator<AbstractBounds<PartitionPosition>> rangeIterator) { - return new BigTableScanner(sstable, ColumnFilter.all(sstable.metadata), null, null, false, rangeIterator); + return new BigTableScanner(sstable, ColumnFilter.all(sstable.metadata), null, false, rangeIterator); } - private BigTableScanner(SSTableReader sstable, ColumnFilter columns, DataRange dataRange, RateLimiter limiter, boolean isForThrift, Iterator<AbstractBounds<PartitionPosition>> rangeIterator) + private BigTableScanner(SSTableReader sstable, ColumnFilter columns, DataRange dataRange, boolean isForThrift, Iterator<AbstractBounds<PartitionPosition>> rangeIterator) { assert sstable != null; - this.dfile = limiter == null ? sstable.openDataReader() : sstable.openDataReader(limiter); + this.dfile = sstable.openDataReader(); this.ifile = sstable.openIndexReader(); this.sstable = sstable; this.columns = columns; http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/test/unit/org/apache/cassandra/db/compaction/AntiCompactionTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/compaction/AntiCompactionTest.java b/test/unit/org/apache/cassandra/db/compaction/AntiCompactionTest.java index db07eb8..a2e2754 100644 --- a/test/unit/org/apache/cassandra/db/compaction/AntiCompactionTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/AntiCompactionTest.java @@ -105,7 +105,7 @@ public class AntiCompactionTest assertEquals(2, store.getLiveSSTables().size()); for (SSTableReader sstable : store.getLiveSSTables()) { - try (ISSTableScanner scanner = sstable.getScanner((RateLimiter) null)) + try (ISSTableScanner scanner = sstable.getScanner()) { while (scanner.hasNext()) { @@ -240,7 +240,7 @@ public class AntiCompactionTest int nonRepairedKeys = 0; for (SSTableReader sstable : store.getLiveSSTables()) { - try (ISSTableScanner scanner = sstable.getScanner((RateLimiter) null)) + try (ISSTableScanner scanner = sstable.getScanner()) { while (scanner.hasNext()) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java index 0928ad4..982fc9c 100644 --- a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java @@ -416,7 +416,7 @@ public class SSTableReaderTest boolean foundScanner = false; for (SSTableReader s : store.getLiveSSTables()) { - try (ISSTableScanner scanner = s.getScanner(new Range<Token>(t(0), t(1)), null)) + try (ISSTableScanner scanner = s.getScanner(new Range<Token>(t(0), t(1)))) { scanner.next(); // throws exception pre 5407 foundScanner = true; http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2a35476/test/unit/org/apache/cassandra/io/sstable/SSTableScannerTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/io/sstable/SSTableScannerTest.java b/test/unit/org/apache/cassandra/io/sstable/SSTableScannerTest.java index d73c278..594efe9 100644 --- a/test/unit/org/apache/cassandra/io/sstable/SSTableScannerTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/SSTableScannerTest.java @@ -219,7 +219,7 @@ public class SSTableScannerTest SSTableReader sstable = store.getLiveSSTables().iterator().next(); // full range scan - ISSTableScanner scanner = sstable.getScanner(RateLimiter.create(Double.MAX_VALUE)); + ISSTableScanner scanner = sstable.getScanner(); for (int i = 2; i < 10; i++) assertEquals(toKey(i), new String(scanner.next().partitionKey().getKey().array())); @@ -325,7 +325,7 @@ public class SSTableScannerTest SSTableReader sstable = store.getLiveSSTables().iterator().next(); // full range scan - ISSTableScanner fullScanner = sstable.getScanner(RateLimiter.create(Double.MAX_VALUE)); + ISSTableScanner fullScanner = sstable.getScanner(); assertScanContainsRanges(fullScanner, 2, 9, 102, 109, @@ -335,8 +335,7 @@ public class SSTableScannerTest // scan all three ranges separately ISSTableScanner scanner = sstable.getScanner(makeRanges(1, 9, 101, 109, - 201, 209), - null); + 201, 209)); assertScanContainsRanges(scanner, 2, 9, 102, 109, @@ -344,16 +343,14 @@ public class SSTableScannerTest // skip the first range scanner = sstable.getScanner(makeRanges(101, 109, - 201, 209), - null); + 201, 209)); assertScanContainsRanges(scanner, 102, 109, 202, 209); // skip the second range scanner = sstable.getScanner(makeRanges(1, 9, - 201, 209), - null); + 201, 209)); assertScanContainsRanges(scanner, 2, 9, 202, 209); @@ -361,8 +358,7 @@ public class SSTableScannerTest // skip the last range scanner = sstable.getScanner(makeRanges(1, 9, - 101, 109), - null); + 101, 109)); assertScanContainsRanges(scanner, 2, 9, 102, 109); @@ -370,8 +366,7 @@ public class SSTableScannerTest // the first scanned range stops short of the actual data in the first range scanner = sstable.getScanner(makeRanges(1, 5, 101, 109, - 201, 209), - null); + 201, 209)); assertScanContainsRanges(scanner, 2, 5, 102, 109, @@ -380,8 +375,7 @@ public class SSTableScannerTest // the first scanned range requests data beyond actual data in the first range scanner = sstable.getScanner(makeRanges(1, 20, 101, 109, - 201, 209), - null); + 201, 209)); assertScanContainsRanges(scanner, 2, 9, 102, 109, @@ -391,8 +385,7 @@ public class SSTableScannerTest // the middle scan range splits the outside two data ranges scanner = sstable.getScanner(makeRanges(1, 5, 6, 205, - 206, 209), - null); + 206, 209)); assertScanContainsRanges(scanner, 2, 5, 7, 9, @@ -406,8 +399,7 @@ public class SSTableScannerTest 101, 109, 150, 159, 201, 209, - 1000, 1001), - null); + 1000, 1001)); assertScanContainsRanges(scanner, 3, 9, 102, 109, @@ -419,8 +411,7 @@ public class SSTableScannerTest 201, 209, 101, 109, 1000, 1001, - 150, 159), - null); + 150, 159)); assertScanContainsRanges(scanner, 2, 9, 102, 109, @@ -429,12 +420,11 @@ public class SSTableScannerTest // only empty ranges scanner = sstable.getScanner(makeRanges(0, 1, 150, 159, - 250, 259), - null); + 250, 259)); assertFalse(scanner.hasNext()); // no ranges is equivalent to a full scan - scanner = sstable.getScanner(new ArrayList<Range<Token>>(), null); + scanner = sstable.getScanner(new ArrayList<Range<Token>>()); assertFalse(scanner.hasNext()); } @@ -455,12 +445,12 @@ public class SSTableScannerTest SSTableReader sstable = store.getLiveSSTables().iterator().next(); // full range scan - ISSTableScanner fullScanner = sstable.getScanner(RateLimiter.create(Double.MAX_VALUE)); + ISSTableScanner fullScanner = sstable.getScanner(); assertScanContainsRanges(fullScanner, 205, 205); // scan three ranges separately ISSTableScanner scanner = sstable.getScanner(makeRanges(101, 109, - 201, 209), null); + 201, 209)); // this will currently fail assertScanContainsRanges(scanner, 205, 205);
