Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cursor): Last method needs skip empty pages #341

Merged
merged 1 commit into from
Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Note that we start to track changes starting from v1.3.7.

### BoltDB
- [Add support for loong64 arch](https://github.com/etcd-io/bbolt/pull/303).
- Fix [the "Last" method might return no data due to not skipping the empty pages](https://github.com/etcd-io/bbolt/pull/341)

### CMD
- [Open db file readonly in compact CLI command](https://github.com/etcd-io/bbolt/pull/292).
Expand Down
56 changes: 36 additions & 20 deletions cursor.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@ func (c *Cursor) Last() (key []byte, value []byte) {
ref.index = ref.count() - 1
c.stack = append(c.stack, ref)
c.last()

// If this is an empty page (calling Delete may result in empty pages)
// we call prev to find the last page that is not empty
for len(c.stack) > 0 && c.stack[len(c.stack)-1].count() == 0 {
c.prev()
dchaofei marked this conversation as resolved.
Show resolved Hide resolved
}

if len(c.stack) == 0 {
return nil, nil
}

k, v, flags := c.keyValue()
if (flags & uint32(bucketLeafFlag)) != 0 {
return k, nil
Expand All @@ -84,26 +95,7 @@ func (c *Cursor) Next() (key []byte, value []byte) {
// The returned key and value are only valid for the life of the transaction.
func (c *Cursor) Prev() (key []byte, value []byte) {
_assert(c.bucket.tx.db != nil, "tx closed")

// Attempt to move back one element until we're successful.
// Move up the stack as we hit the beginning of each page in our stack.
for i := len(c.stack) - 1; i >= 0; i-- {
elem := &c.stack[i]
if elem.index > 0 {
elem.index--
break
}
c.stack = c.stack[:i]
}

// If we've hit the end then return nil.
if len(c.stack) == 0 {
return nil, nil
}

// Move down the stack to find the last element of the last leaf under this branch.
c.last()
k, v, flags := c.keyValue()
k, v, flags := c.prev()
if (flags & uint32(bucketLeafFlag)) != 0 {
return k, nil
}
Expand Down Expand Up @@ -243,6 +235,30 @@ func (c *Cursor) next() (key []byte, value []byte, flags uint32) {
}
}

// prev moves the cursor to the previous item in the bucket and returns its key and value.
// If the cursor is at the beginning of the bucket then a nil key and value are returned.
func (c *Cursor) prev() (key []byte, value []byte, flags uint32) {
// Attempt to move back one element until we're successful.
// Move up the stack as we hit the beginning of each page in our stack.
for i := len(c.stack) - 1; i >= 0; i-- {
elem := &c.stack[i]
if elem.index > 0 {
elem.index--
break
}
c.stack = c.stack[:i]
}

// If we've hit the end then return nil.
if len(c.stack) == 0 {
return nil, nil, 0
}

// Move down the stack to find the last element of the last leaf under this branch.
c.last()
return c.keyValue()
}

// search recursively performs a binary search against a given page/node until it finds a given key.
func (c *Cursor) search(key []byte, pgid pgid) {
p, n := c.bucket.pageNode(pgid)
Expand Down
47 changes: 47 additions & 0 deletions cursor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,53 @@ func TestCursor_First_EmptyPages(t *testing.T) {
}
}

// Ensure that a cursor can skip over empty pages that have been deleted.
func TestCursor_Last_EmptyPages(t *testing.T) {
db := MustOpenDB()
defer db.MustClose()

// Create 1000 keys in the "widgets" bucket.
if err := db.Update(func(tx *bolt.Tx) error {
b, err := tx.CreateBucket([]byte("widgets"))
if err != nil {
t.Fatal(err)
}

for i := 0; i < 1000; i++ {
if err := b.Put(u64tob(uint64(i)), []byte{}); err != nil {
t.Fatal(err)
}
}

return nil
}); err != nil {
t.Fatal(err)
}

// Delete last 800 elements to ensure last page is empty
if err := db.Update(func(tx *bolt.Tx) error {
b := tx.Bucket([]byte("widgets"))
for i := 200; i < 1000; i++ {
if err := b.Delete(u64tob(uint64(i))); err != nil {
t.Fatal(err)
}
}

c := b.Cursor()
var n int
for k, _ := c.Last(); k != nil; k, _ = c.Prev() {
n++
}
if n != 200 {
t.Fatalf("unexpected key count: %d", n)
}

return nil
}); err != nil {
t.Fatal(err)
}
}

// Ensure that a Tx can iterate over all elements in a bucket.
func TestCursor_QuickCheck(t *testing.T) {
f := func(items testdata) bool {
Expand Down