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

Cursor.Delete followed by Next skips the next k/v pair #146

Closed
dnldd opened this issue Feb 7, 2019 · 7 comments
Closed

Cursor.Delete followed by Next skips the next k/v pair #146

dnldd opened this issue Feb 7, 2019 · 7 comments

Comments

@dnldd
Copy link

dnldd commented Feb 7, 2019

The bug is described in detail here: boltdb/bolt#620.

@asdine
Copy link
Contributor

asdine commented Oct 21, 2019

Actually, even bucket.Delete during cursor iteration makes the cursor skip the next pair.

@WBare
Copy link

WBare commented Jun 23, 2020

Is there an update on this? This seems to be working for me on both 1.3.3 and 1.3.4

In my testing, the combination of a cursor.Seek(key) then either a bucket.Delete(key) or a cursor.Delete() followed by a cursor.Next() does seem to correct advance to the next key right after the deleted item.

Perhaps I'm not reproducing the bug correctly. The original boltdb (not bbolt) issue linked above has a link to a test for this. Maybe it has already been fixed in bbolt and this issue should be closed?

@jrick
Copy link
Contributor

jrick commented Jun 26, 2020

Still a bug.

package main

import (
	"bytes"
	"os"
	"testing"
	
	"go.etcd.io/bbolt"
)

func TestCursorDelete(t *testing.T) {
	db, err := bbolt.Open("bbolt.db", 0666, nil)
	if err != nil {
		t.Fatal(err)
	}
	defer os.Remove("bbolt.db")
	defer db.Close()

	cursorKeys := make(map[string]struct{})
	err = db.Update(func(tx *bbolt.Tx) (err error) {
		b, err := tx.CreateBucket([]byte("b"))
		if err != nil {
			return err
		}
		put := func(s []byte) {
			if err == nil {
				err = b.Put(s, s)
			}
		}
		put([]byte("a"))
		put([]byte("b"))
		put([]byte("c"))
		put([]byte("d"))
		if err != nil {
			return err
		}

		c := b.Cursor()
		for k, _ := c.First(); k != nil; k, _ = c.Next() {
			t.Logf("inspecting key %s", k)
			cursorKeys[string(k)] = struct{}{}
			
			if bytes.Equal(k, []byte("a")) {
				err = c.Delete()
				if err != nil {
					return err
				}
				continue
			}
		}
		return nil
	})
	if err != nil {
		t.Fatal(err)
	}

	_, ok := cursorKeys["b"]
	if !ok {
		t.Errorf("cursor never saw key b")
	}
}
$ go test -v              
=== RUN   TestCursorDelete
    cursor_test.go:40: inspecting key a
    cursor_test.go:40: inspecting key c
    cursor_test.go:40: inspecting key d
    cursor_test.go:59: cursor never saw key b
--- FAIL: TestCursorDelete (0.00s)
FAIL
exit status 1
FAIL    bboltcursor     0.012s

@WBare
Copy link

WBare commented Jun 29, 2020

OK, perfect. Thanks, @jrick. Wondering why mine was working, I modified your test below. This may help narrow down the bug.

The test below does not produce the error. Based on this one test (not conclusive) it looks like the error does not occur if the key/doc being deleted is not also created in the same transaction.

This is definitely bad behavior.

func TestCursorDelete2Transactions(t *testing.T) {
	db, err := bbolt.Open("bbolt.db", 0666, nil)
	if err != nil {
		t.Fatal(err)
	}
	defer os.Remove("bbolt.db")
	defer db.Close()

	cursorKeys := make(map[string]struct{})
	err = db.Update(func(tx *bbolt.Tx) (err error) {
		b, err := tx.CreateBucket([]byte("b"))
		if err != nil {
			return err
		}
		put := func(s []byte) {
			if err == nil {
				err = b.Put(s, s)
			}
		}
		put([]byte("a"))
		put([]byte("b"))
		put([]byte("c"))
		put([]byte("d"))
		if err != nil {
			return err
		}
		return
	})

        // NOTICE THIS BREAK IN THE TRANSACTION

	err = db.Update(func(tx *bbolt.Tx) (err error) {
		b := tx.Bucket([]byte("b"))
		c := b.Cursor()
		for k, _ := c.First(); k != nil; k, _ = c.Next() {
			t.Logf("inspecting key %s", k)
			cursorKeys[string(k)] = struct{}{}

			if bytes.Equal(k, []byte("a")) {
				err = c.Delete()
				if err != nil {
					return err
				}
				continue
			}
		}
		return nil
	})
	if err != nil {
		t.Fatal(err)
	}

	_, ok := cursorKeys["b"]
	if !ok {
		t.Errorf("cursor never saw key b")
	}
}

@missinglink
Copy link
Contributor

I can confirm this is still broken on master, it makes the database completely unusable for any find-and-delete operations.

@missinglink
Copy link
Contributor

missinglink commented Sep 14, 2021

Okay, so hacking around a bit, there is a workaround if you rewrite the range query from the example as such:

for k, _ := cursor.Seek(min); k != nil && bytes.Compare(k, max) <= 0; {
	if shouldDelete(k) && cursor.Delete() == nil {
		k, _ = cursor.Seek(k)
	} else {
		k, _ = cursor.Next()
	}
}

plorenz added a commit to openziti/storage that referenced this issue Mar 30, 2022
@github-actions github-actions bot added the stale label May 11, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 1, 2024
@ahrtr
Copy link
Member

ahrtr commented Jun 1, 2024

Added a known issue for this behaviour, please refer to #614

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

7 participants