sstable: do not invalidate data block if bloomfilter returns false

[xxmplus]

Do not invalidate data block if bloomfilter returns false.

Fixes #4788

[cockroach-teamcity]

This change is 

[RaduBerinde]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @sumeerbhola)

---

sstable/reader_iter_two_lvl.go line 447 at r1 (raw file):

		}
		if !mayContain {
			// In the no-error bloom filter miss case, the key definitely not in table.

[nit] is definitely not

[jbowens]

@jbowens reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @sumeerbhola and @xxmplus)

---

-- commits line 2 at r1:
nit: s/feat:/sstable:/

---

sstable/reader_iter_two_lvl.go line 449 at r1 (raw file):

			// In the no-error bloom filter miss case, the key definitely not in table.
			// We can avoid invalidating the already loaded block since the caller is
			// not allowed to call Next when SeekPrefixGE returns nil.

I wonder if we could add an assertion for this in invariants builds. We'd need to remember the previous operation (or at least whether it was a SeekPrefixGE). We could store this information in an invariants.Value[T] type so that it compiles away on production builds, and then in Next() assert that the block is not invalidated in invariants.Enabled builds if the previous op was a SeekPrefixGE

[xxmplus]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @jbowens, @RaduBerinde, and @sumeerbhola)

---

-- commits line 2 at r1:

Previously, jbowens (Jackson Owens) wrote…

nit: s/feat:/sstable:/

updated.

---

sstable/reader_iter_two_lvl.go line 447 at r1 (raw file):

Previously, RaduBerinde wrote…

[nit] is definitely not

fixed.

---

sstable/reader_iter_two_lvl.go line 449 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

I wonder if we could add an assertion for this in invariants builds. We'd need to remember the previous operation (or at least whether it was a SeekPrefixGE). We could store this information in an invariants.Value[T] type so that it compiles away on production builds, and then in Next() assert that the block is not invalidated in invariants.Enabled builds if the previous op was a SeekPrefixGE

added.

[xxmplus]

@jbowens TFTR! I could use another review.