Skip to content

Commit

Permalink
sstable: clarify BufferHandle ownership by iterators
Browse files Browse the repository at this point in the history
  • Loading branch information
RaduBerinde committed Oct 18, 2024
1 parent 5a84fdd commit d3b36cf
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
16 changes: 12 additions & 4 deletions sstable/block/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,17 @@ const MetadataSize = 328
const _ uint = -(MetadataSize % 8)

// DataBlockIterator is a type constraint for implementations of block iterators
// over data blocks.
// over data blocks. It's implemented by *rowblk.Iter and *colblk.DataBlockIter.
type DataBlockIterator interface {
base.InternalIterator

// Handle returns the handle to the block.
Handle() BufferHandle
// InitHandle initializes the block from the provided buffer handle.
//
// The iterator takes ownership of the BufferHandle and releases it when it is
// closed (or re-initialized with another handle). This happens even in error
// cases.
InitHandle(base.Compare, base.Split, BufferHandle, IterTransforms) error
// Valid returns true if the iterator is currently positioned at a valid KV.
Valid() bool
Expand Down Expand Up @@ -198,13 +202,17 @@ type DataBlockIterator interface {
IsDataInvalidated() bool
}

// IndexBlockIterator is an interface for implementations of block
// iterators over index blocks. It's currently satisifed by the
// *rowblk.IndexIter type.
// IndexBlockIterator is an interface for implementations of block iterators
// over index blocks. It's implemented by *rowblk.IndexIter and
// *colblk.IndexBlockIter.
type IndexBlockIterator interface {
// Init initializes the block iterator from the provided block.
Init(base.Compare, base.Split, []byte, IterTransforms) error
// InitHandle initializes an iterator from the provided block handle.
//
// The iterator takes ownership of the BufferHandle and releases it when it is
// closed (or re-initialized with another handle). This happens even in error
// cases.
InitHandle(base.Compare, base.Split, BufferHandle, IterTransforms) error
// Valid returns true if the iterator is currently positioned at a valid
// block handle.
Expand Down
3 changes: 3 additions & 0 deletions sstable/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,9 @@ func estimateDiskUsage[I any, PI indexBlockIterator[I]](
if err != nil {
return 0, err
}
// We are using InitHandle below but we never Close those iterators, which
// allows us to release the index handle ourselves.
// TODO(radu): clean this up.
defer indexH.Release()

// Iterators over the bottom-level index blocks containing start and end.
Expand Down
7 changes: 5 additions & 2 deletions sstable/reader_iter_two_lvl.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,13 @@ func (i *twoLevelIterator[I, PI, D, PD]) loadSecondLevelIndexBlock(dir int8) loa
// blockIntersects
}
indexBlock, err := i.secondLevel.reader.readIndexBlock(i.secondLevel.ctx, i.secondLevel.readBlockEnv, i.secondLevel.indexFilterRH, bhp.Handle)
if err == nil {
err = PI(&i.secondLevel.index).InitHandle(i.secondLevel.cmp, i.secondLevel.reader.Split, indexBlock, i.secondLevel.transforms)
if err != nil {
i.secondLevel.err = err
return loadBlockFailed
}
err = PI(&i.secondLevel.index).InitHandle(i.secondLevel.cmp, i.secondLevel.reader.Split, indexBlock, i.secondLevel.transforms)
if err != nil {
PI(&i.secondLevel.index).Invalidate()
i.secondLevel.err = err
return loadBlockFailed
}
Expand Down

0 comments on commit d3b36cf

Please sign in to comment.