Skip to content

Commit

Permalink
manifest: assert that we don't use Seek on L0 LevelMetadata
Browse files Browse the repository at this point in the history
The L0 `LevelMetadata` is ordered by `LargestSeqNum` so using
`SeekGE/SeekLT` is not valid. This addresses a TODO to verify this.

The assertion failed on some test code that is also fixed.
  • Loading branch information
RaduBerinde committed Mar 29, 2024
1 parent 4f4be42 commit a9087af
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 9 deletions.
14 changes: 13 additions & 1 deletion ingest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,19 @@ func TestExcise(t *testing.T) {
d.mu.versions.logLock()
d.mu.Unlock()
current := d.mu.versions.currentVersion()
for level := range current.Levels {
for _, sublevel := range current.L0Sublevels.Levels {
iter := sublevel.Iter()
for m := iter.SeekGE(d.cmp, exciseSpan.Start); m != nil && d.cmp(m.Smallest.UserKey, exciseSpan.End) < 0; m = iter.Next() {
_, err := d.excise(exciseSpan.UserKeyBounds(), m, ve, 0 /* level */)
if err != nil {
d.mu.Lock()
d.mu.versions.logUnlock()
d.mu.Unlock()
return fmt.Sprintf("error when excising %s: %s", m.FileNum, err.Error())
}
}
}
for level := 1; level < manifest.NumLevels; level++ {
iter := current.Levels[level].Iter()
for m := iter.SeekGE(d.cmp, exciseSpan.Start); m != nil && d.cmp(m.Smallest.UserKey, exciseSpan.End) < 0; m = iter.Next() {
_, err := d.excise(exciseSpan.UserKeyBounds(), m, ve, level)
Expand Down
34 changes: 26 additions & 8 deletions internal/manifest/level_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,13 +551,15 @@ func (i *LevelIterator) Prev() *FileMetadata {

// SeekGE seeks to the first file in the iterator's file set with a largest
// user key greater than or equal to the provided user key. The iterator must
// have been constructed from L1+, because it requires the underlying files to
// be sorted by user keys and non-overlapping.
// have been constructed from L1+ or from a single sublevel of L0, because it
// requires the underlying files to be sorted by user keys and non-overlapping.
func (i *LevelIterator) SeekGE(cmp Compare, userKey []byte) *FileMetadata {
// TODO(jackson): Assert that i.iter.cmp == btreeCmpSmallestKey.
if i.iter.r == nil {
return nil
}
if invariants.Enabled {
i.assertNotL0Cmp(userKey)
}
m := i.seek(func(m *FileMetadata) bool {
return cmp(m.Largest.UserKey, userKey) >= 0
})
Expand All @@ -574,15 +576,31 @@ func (i *LevelIterator) SeekGE(cmp Compare, userKey []byte) *FileMetadata {
return i.skipFilteredForward(m)
}

// SeekLT seeks to the last file in the iterator's file set with a smallest
// user key less than the provided user key. The iterator must have been
// constructed from L1+, because it requires the underlying files to be sorted
// by user keys and non-overlapping.
// assertNotL0Cmp verifies that the btree associated with the iterator is
// ordered by Smallest key (i.e. L1+ or L0 sublevel) and not by LargestSeqNum
// (L0).
//
// userKey is an arbitrary valid key.
func (i *LevelIterator) assertNotL0Cmp(userKey []byte) {
k := base.MakeInternalKey(userKey, 1, base.InternalKeyKindSet)
a := FileMetadata{Smallest: k, LargestSeqNum: 10}
b := FileMetadata{Smallest: k, LargestSeqNum: 100}
if i.iter.cmp(&a, &b) != 0 {
panic("SeekGE used with btreeCmpSeqNum")
}
}

// SeekLT seeks to the last file in the iterator's file set with a smallest user
// key less than the provided user key. The iterator must have been constructed
// from L1+ or from a single sublevel of L0, because it requires the underlying
// files to be sorted by user keys and non-overlapping.
func (i *LevelIterator) SeekLT(cmp Compare, userKey []byte) *FileMetadata {
// TODO(jackson): Assert that i.iter.cmp == btreeCmpSmallestKey.
if i.iter.r == nil {
return nil
}
if invariants.Enabled {
i.assertNotL0Cmp(userKey)
}
i.seek(func(m *FileMetadata) bool {
return cmp(m.Smallest.UserKey, userKey) >= 0
})
Expand Down

0 comments on commit a9087af

Please sign in to comment.