[DRAFT] feat!: do not invalidate data block if bloomfilter returns false

sstable/reader_iter_single_lvl.go

@@ -809,7 +809,6 @@ func (i *singleLevelIterator[I, PI, D, PD]) seekPrefixGE(
 	// NOTE: prefix is only used for bloom filter checking and not later work in
 	// this method. Hence, we can use the existing iterator position if the last
 	// SeekPrefixGE did not fail bloom filter matching.
-
 	err := i.err
 	i.err = nil // clear cached iteration error
 	if i.useFilterBlock {
@@ -821,15 +820,18 @@ func (i *singleLevelIterator[I, PI, D, PD]) seekPrefixGE(
 		// Check prefix bloom filter.
 		var mayContain bool
 		mayContain, i.err = i.bloomFilterMayContain(prefix)
-		if i.err != nil || !mayContain {
-			// In the i.err == nil case, this invalidation may not be necessary for
-			// correctness, and may be a place to optimize later by reusing the
-			// already loaded block. It was necessary in earlier versions of the code
-			// since the caller was allowed to call Next when SeekPrefixGE returned
-			// nil. This is no longer allowed.
+
+		if i.err != nil {
+			// fmt.Println("[DEBUG] invalidate the block due to bloomFilterMayContain error:", i.err)
 			PD(&i.data).Invalidate()
 			return nil
 		}
+		if !mayContain {
+			// Fast-path: key definitely not in table. Do NOT invalidate the block
+			// i.data can be left in place for potential reuse.
+			// fmt.Println("[DEBUG] do not invalidate the block if bloomFilterMayContain returned false only")
+			return nil
+		}
 		i.lastBloomFilterMatched = true
 	}
 	if flags.TrySeekUsingNext() {

sstable/reader_iter_two_lvl.go

@@ -437,15 +437,16 @@ func (i *twoLevelIterator[I, PI, D, PD]) SeekPrefixGE(
 		i.lastBloomFilterMatched = false
 		var mayContain bool
 		mayContain, i.secondLevel.err = i.secondLevel.bloomFilterMayContain(prefix)
-		if i.secondLevel.err != nil || !mayContain {
-			// In the i.secondLevel.err == nil case, this invalidation may not be necessary for
-			// correctness, and may be a place to optimize later by reusing the
-			// already loaded block. It was necessary in earlier versions of the code
-			// since the caller was allowed to call Next when SeekPrefixGE returned
-			// nil. This is no longer allowed.
+
+		if i.secondLevel.err != nil {
 			PD(&i.secondLevel.data).Invalidate()
 			return nil
 		}
+		if !mayContain {
+			// Fast-path: key definitely not in table. Do NOT invalidate the block
+			// i.secondLevel.data can be left in place for potential reuse.
+			return nil
+		}
 		i.lastBloomFilterMatched = true
 	}