Skip to content

Commit

Permalink
db: use invalidating iterators with low probability under invariants
Browse files Browse the repository at this point in the history
In invariants builds, with 10% probability wrap various internal iterators with
an invalidating iterator. This helps ensure we're not accidentally depending on
key byte slice stability where we shouldn't.

Close cockroachdb#2896.
  • Loading branch information
jbowens committed Sep 14, 2023
1 parent bb9d6ab commit 529d256
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
2 changes: 2 additions & 0 deletions compaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/cockroachdb/errors"
"github.com/cockroachdb/pebble/internal/base"
"github.com/cockroachdb/pebble/internal/invalidating"
"github.com/cockroachdb/pebble/internal/keyspan"
"github.com/cockroachdb/pebble/internal/manifest"
"github.com/cockroachdb/pebble/internal/private"
Expand Down Expand Up @@ -2847,6 +2848,7 @@ func (d *DB) runCompaction(
return nil, pendingOutputs, stats, err
}
c.allowedZeroSeqNum = c.allowZeroSeqNum()
iiter = invalidating.MaybeWrapIfInvariants(iiter)
iter := newCompactionIter(c.cmp, c.equal, c.formatKey, d.merge, iiter, snapshots,
&c.rangeDelFrag, &c.rangeKeyFrag, c.allowedZeroSeqNum, c.elideTombstone,
c.elideRangeTombstone, d.FormatMajorVersion())
Expand Down
6 changes: 4 additions & 2 deletions db.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/cockroachdb/errors"
"github.com/cockroachdb/pebble/internal/arenaskl"
"github.com/cockroachdb/pebble/internal/base"
"github.com/cockroachdb/pebble/internal/invalidating"
"github.com/cockroachdb/pebble/internal/invariants"
"github.com/cockroachdb/pebble/internal/keyspan"
"github.com/cockroachdb/pebble/internal/manifest"
Expand Down Expand Up @@ -1149,6 +1150,7 @@ func finishInitializingIter(ctx context.Context, buf *iterAlloc) *Iterator {
},
}
dbi.iter = &dbi.lazyCombinedIter
dbi.iter = invalidating.MaybeWrapIfInvariants(dbi.iter)
} else {
dbi.lazyCombinedIter.combinedIterState = combinedIterState{
initialized: true,
Expand Down Expand Up @@ -1426,8 +1428,8 @@ func (i *Iterator) constructPointIter(
li.initRangeDel(&mlevels[mlevelsIndex].rangeDelIter)
li.initBoundaryContext(&mlevels[mlevelsIndex].levelIterBoundaryContext)
li.initCombinedIterState(&i.lazyCombinedIter.combinedIterState)
mlevels[mlevelsIndex].iter = li
mlevels[mlevelsIndex].levelIter = li
mlevels[mlevelsIndex].iter = invalidating.MaybeWrapIfInvariants(li)

levelsIndex++
mlevelsIndex++
Expand All @@ -1453,7 +1455,7 @@ func (i *Iterator) constructPointIter(
buf.merging.snapshot = i.seqNum
buf.merging.batchSnapshot = i.batchSeqNum
buf.merging.combinedIterState = &i.lazyCombinedIter.combinedIterState
i.pointIter = &buf.merging
i.pointIter = invalidating.MaybeWrapIfInvariants(&buf.merging)
i.merging = &buf.merging
}

Expand Down
17 changes: 16 additions & 1 deletion internal/invalidating/iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,22 @@

package invalidating

import "github.com/cockroachdb/pebble/internal/base"
import (
"github.com/cockroachdb/pebble/internal/base"
"github.com/cockroachdb/pebble/internal/fastrand"
"github.com/cockroachdb/pebble/internal/invariants"
)

// MaybeWrapIfInvariants wraps some iterators with an invalidating iterator.
// MaybeWrapIfInvariants does nothing in non-invariant builds.
func MaybeWrapIfInvariants(iter base.InternalIterator) base.InternalIterator {
if invariants.Enabled {
if fastrand.Uint32n(10) == 1 {
return NewIter(iter)
}
}
return iter
}

// iter tests unsafe key/value slice reuse by modifying the last
// returned key/value to all 1s.
Expand Down

0 comments on commit 529d256

Please sign in to comment.