This is an automated email from the ASF dual-hosted git repository.
brandonwilliams pushed a commit to branch cassandra-3.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-3.0 by this push:
new 87424da Count bloom filter misses correctly.
87424da is described below
commit 87424dabd20acfdbb9a4489f289ec075ccfb87b0
Author: Aleksei Zotov <azotc...@gmail.com>
AuthorDate: Thu Jul 15 21:40:10 2021 +0400
Count bloom filter misses correctly.
Patch by Aleksei Zotov; reviewed by blerer and blambov for
CASSANDRA-12922
---
CHANGES.txt | 1 +
.../io/sstable/format/big/BigTableReader.java | 14 ++-
.../cassandra/io/sstable/SSTableReaderTest.java | 105 ++++++++++++++++-----
3 files changed, 96 insertions(+), 24 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index de54aad..d4e9322 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.25:
+ * Count bloom filter misses correctly (CASSANDRA-12922)
* Reject token() in MV WHERE clause (CASSANDRA-13464)
* Ensure java executable is on the path (CASSANDRA-14325)
* Make speculative retry parameter case-insensitive for backward
compatibility with 2.1 (CASSANDRA-16467)
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 598f757..080cf04 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
@@ -138,6 +138,13 @@ public class BigTableReader extends SSTableReader
boolean permitMatchPastLast,
SSTableReadsListener listener)
{
+ // Having no index file is impossible in a normal operation. The only
way it might happen is running
+ // Scrubber that does not really rely onto this method.
+ if (ifile == null)
+ {
+ return null;
+ }
+
if (op == Operator.EQ)
{
assert key instanceof DecoratedKey; // EQ only make sense if the
key is a valid row key
@@ -158,6 +165,8 @@ public class BigTableReader extends SSTableReader
RowIndexEntry cachedPosition = getCachedPosition(cacheKey,
updateCacheAndStats);
if (cachedPosition != null)
{
+ // we do not need to track "true positive" for Bloom Filter
here because it has been already tracked
+ // inside getCachedPosition method
listener.onSSTableSelected(this, cachedPosition,
SelectionReason.KEY_CACHE_HIT);
Tracing.trace("Key cache hit for sstable {}",
descriptor.generation);
return cachedPosition;
@@ -198,9 +207,6 @@ public class BigTableReader extends SSTableReader
int effectiveInterval =
indexSummary.getEffectiveIndexIntervalAfterIndex(sampledIndex);
- if (ifile == null)
- return null;
-
// scan the on-disk index, starting at the nearest sampled position.
// The check against IndexInterval is to be exit the loop in the EQ
case when the key looked for is not present
// (bloom filter false positive). But note that for non-EQ cases, we
might need to check the first key of the
@@ -235,6 +241,8 @@ public class BigTableReader extends SSTableReader
exactMatch = (comparison == 0);
if (v < 0)
{
+ if (op == SSTableReader.Operator.EQ &&
updateCacheAndStats)
+ bloomFilterTracker.addFalsePositive();
listener.onSSTableSkipped(this,
SkippingReason.PARTITION_INDEX_LOOKUP);
Tracing.trace("Partition index lookup allows skipping
sstable {}", descriptor.generation);
return null;
diff --git a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
index 4ca6ec0..66155e5 100644
--- a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
+++ b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
@@ -70,7 +70,8 @@ public class SSTableReaderTest
public static final String CF_STANDARD = "Standard1";
public static final String CF_STANDARD2 = "Standard2";
public static final String CF_INDEXED = "Indexed1";
- public static final String CF_STANDARDLOWINDEXINTERVAL =
"StandardLowIndexInterval";
+ public static final String CF_STANDARD_LOW_INDEX_INTERVAL =
"StandardLowIndexInterval";
+ public static final String CF_STANDARD_SMALL_BLOOM_FILTER =
"StandardSmallBloomFilter";
private IPartitioner partitioner;
@@ -88,17 +89,21 @@ public class SSTableReaderTest
SchemaLoader.standardCFMD(KEYSPACE1,
CF_STANDARD),
SchemaLoader.standardCFMD(KEYSPACE1,
CF_STANDARD2),
SchemaLoader.compositeIndexCFMD(KEYSPACE1,
CF_INDEXED, true),
- SchemaLoader.standardCFMD(KEYSPACE1,
CF_STANDARDLOWINDEXINTERVAL)
+ SchemaLoader.standardCFMD(KEYSPACE1,
CF_STANDARD_LOW_INDEX_INTERVAL)
.minIndexInterval(8)
.maxIndexInterval(256)
-
.caching(CachingParams.CACHE_NOTHING));
+
.caching(CachingParams.CACHE_NOTHING),
+ SchemaLoader.standardCFMD(KEYSPACE1,
CF_STANDARD_SMALL_BLOOM_FILTER)
+ .minIndexInterval(4)
+ .maxIndexInterval(4)
+ .bloomFilterFpChance(0.99));
}
@Test
public void testGetPositionsForRanges()
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Standard2");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_STANDARD2);
partitioner = store.getPartitioner();
// insert data and compact to a single sstable
@@ -144,7 +149,7 @@ public class SSTableReaderTest
try
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store =
keyspace.getColumnFamilyStore("Standard1");
+ ColumnFamilyStore store =
keyspace.getColumnFamilyStore(CF_STANDARD);
partitioner = store.getPartitioner();
// insert a bunch of data and compact to a single sstable
@@ -188,7 +193,7 @@ public class SSTableReaderTest
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Standard1");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_STANDARD);
partitioner = store.getPartitioner();
for (int j = 0; j < 100; j += 2)
@@ -216,7 +221,7 @@ public class SSTableReaderTest
{
// try to make sure CASSANDRA-8239 never happens again
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Standard1");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_STANDARD);
partitioner = store.getPartitioner();
for (int j = 0; j < 10; j++)
@@ -245,7 +250,7 @@ public class SSTableReaderTest
public void testGetPositionsForRangesWithKeyCache()
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Standard2");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_STANDARD2);
partitioner = store.getPartitioner();
CacheService.instance.keyCache.setCapacity(100);
@@ -298,10 +303,12 @@ public class SSTableReaderTest
// check if opening and querying works
assertIndexQueryWorks(store);
}
+
+ @Test
public void testGetPositionsKeyCacheStats()
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Standard2");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_STANDARD2);
partitioner = store.getPartitioner();
CacheService.instance.keyCache.setCapacity(1000);
@@ -319,24 +326,80 @@ public class SSTableReaderTest
CompactionManager.instance.performMaximal(store, false);
SSTableReader sstable = store.getLiveSSTables().iterator().next();
+ // existing, non-cached key
sstable.getPosition(k(2), SSTableReader.Operator.EQ);
+ assertEquals(1, sstable.getKeyCacheRequest());
assertEquals(0, sstable.getKeyCacheHit());
- assertEquals(1, sstable.getBloomFilterTruePositiveCount());
+ // existing, cached key
sstable.getPosition(k(2), SSTableReader.Operator.EQ);
+ assertEquals(2, sstable.getKeyCacheRequest());
assertEquals(1, sstable.getKeyCacheHit());
- assertEquals(2, sstable.getBloomFilterTruePositiveCount());
- sstable.getPosition(k(15), SSTableReader.Operator.EQ);
+ // non-existing key (it is specifically chosen to not be rejected by
Bloom Filter check)
+ sstable.getPosition(k(14), SSTableReader.Operator.EQ);
+ assertEquals(3, sstable.getKeyCacheRequest());
assertEquals(1, sstable.getKeyCacheHit());
- assertEquals(2, sstable.getBloomFilterTruePositiveCount());
-
}
+ @Test
+ public void testGetPositionsBloomFilterStats()
+ {
+ Keyspace keyspace = Keyspace.open(KEYSPACE1);
+ ColumnFamilyStore store =
keyspace.getColumnFamilyStore(CF_STANDARD_SMALL_BLOOM_FILTER);
+ partitioner = store.getPartitioner();
+ CacheService.instance.keyCache.setCapacity(1000);
+
+ // insert data and compact to a single sstable
+ CompactionManager.instance.disableAutoCompaction();
+ for (int j = 0; j < 10; j++)
+ {
+ new RowUpdateBuilder(store.metadata, j, String.valueOf(j))
+ .clustering("0")
+ .add("val", ByteBufferUtil.EMPTY_BYTE_BUFFER)
+ .build()
+ .applyUnsafe();
+ }
+ store.forceBlockingFlush();
+ CompactionManager.instance.performMaximal(store, false);
+
+ SSTableReader sstable = store.getLiveSSTables().iterator().next();
+ // the keys are specifically chosen to cover certain use cases
+ // existing key is read from index
+ sstable.getPosition(k(2), SSTableReader.Operator.EQ);
+ assertEquals(1, sstable.getBloomFilterTruePositiveCount());
+ assertEquals(0, sstable.getBloomFilterTrueNegativeCount());
+ assertEquals(0, sstable.getBloomFilterFalsePositiveCount());
+ // existing key is read from Cache Key
+ sstable.getPosition(k(2), SSTableReader.Operator.EQ);
+ assertEquals(2, sstable.getBloomFilterTruePositiveCount());
+ assertEquals(0, sstable.getBloomFilterTrueNegativeCount());
+ assertEquals(0, sstable.getBloomFilterFalsePositiveCount());
+ // non-existing key is rejected by Bloom Filter check
+ sstable.getPosition(k(10), SSTableReader.Operator.EQ);
+ assertEquals(2, sstable.getBloomFilterTruePositiveCount());
+ assertEquals(1, sstable.getBloomFilterTrueNegativeCount());
+ assertEquals(0, sstable.getBloomFilterFalsePositiveCount());
+ // non-existing key is rejected by sstable keys range check
+ sstable.getPosition(k(99), SSTableReader.Operator.EQ);
+ assertEquals(2, sstable.getBloomFilterTruePositiveCount());
+ assertEquals(1, sstable.getBloomFilterTrueNegativeCount());
+ assertEquals(1, sstable.getBloomFilterFalsePositiveCount());
+ // non-existing key is rejected by index interval check
+ sstable.getPosition(k(14), SSTableReader.Operator.EQ);
+ assertEquals(2, sstable.getBloomFilterTruePositiveCount());
+ assertEquals(1, sstable.getBloomFilterTrueNegativeCount());
+ assertEquals(2, sstable.getBloomFilterFalsePositiveCount());
+ // non-existing key is rejected by index lookup check
+ sstable.getPosition(k(807), SSTableReader.Operator.EQ);
+ assertEquals(2, sstable.getBloomFilterTruePositiveCount());
+ assertEquals(1, sstable.getBloomFilterTrueNegativeCount());
+ assertEquals(3, sstable.getBloomFilterFalsePositiveCount());
+ }
@Test
public void testOpeningSSTable() throws Exception
{
String ks = KEYSPACE1;
- String cf = "Standard1";
+ String cf = CF_STANDARD;
// clear and create just one sstable for this test
Keyspace keyspace = Keyspace.open(ks);
@@ -460,7 +523,7 @@ public class SSTableReaderTest
public void testLoadingSummaryUsesCorrectPartitioner() throws Exception
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Indexed1");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_INDEXED);
new RowUpdateBuilder(store.metadata, System.currentTimeMillis(), "k1")
.clustering("0")
@@ -494,7 +557,7 @@ public class SSTableReaderTest
public void testGetScannerForNoIntersectingRanges() throws Exception
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Standard1");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_STANDARD);
partitioner = store.getPartitioner();
new RowUpdateBuilder(store.metadata, 0, "k1")
@@ -520,7 +583,7 @@ public class SSTableReaderTest
public void testGetPositionsForRangesFromTableOpenedForBulkLoading()
throws IOException
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- ColumnFamilyStore store = keyspace.getColumnFamilyStore("Standard2");
+ ColumnFamilyStore store = keyspace.getColumnFamilyStore(CF_STANDARD2);
partitioner = store.getPartitioner();
// insert data and compact to a single sstable. The
@@ -563,7 +626,7 @@ public class SSTableReaderTest
public void testIndexSummaryReplacement() throws IOException,
ExecutionException, InterruptedException
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- final ColumnFamilyStore store =
keyspace.getColumnFamilyStore("StandardLowIndexInterval"); // index interval of
8, no key caching
+ final ColumnFamilyStore store =
keyspace.getColumnFamilyStore(CF_STANDARD_LOW_INDEX_INTERVAL); // index
interval of 8, no key caching
CompactionManager.instance.disableAutoCompaction();
final int NUM_PARTITIONS = 512;
@@ -642,7 +705,7 @@ public class SSTableReaderTest
private void testIndexSummaryUpsampleAndReload0() throws Exception
{
Keyspace keyspace = Keyspace.open(KEYSPACE1);
- final ColumnFamilyStore store =
keyspace.getColumnFamilyStore("StandardLowIndexInterval"); // index interval of
8, no key caching
+ final ColumnFamilyStore store =
keyspace.getColumnFamilyStore(CF_STANDARD_LOW_INDEX_INTERVAL); // index
interval of 8, no key caching
CompactionManager.instance.disableAutoCompaction();
final int NUM_PARTITIONS = 512;
@@ -674,7 +737,7 @@ public class SSTableReaderTest
private void assertIndexQueryWorks(ColumnFamilyStore indexedCFS)
{
- assert "Indexed1".equals(indexedCFS.name);
+ assert CF_INDEXED.equals(indexedCFS.name);
// make sure all sstables including 2ary indexes load from disk
for (ColumnFamilyStore cfs : indexedCFS.concatWithIndexes())
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
