Skip to content

Commit

Permalink
Remove use of cursor.delete due to this bug: etcd-io/bbolt#146
Browse files Browse the repository at this point in the history
  • Loading branch information
@plorenz
plorenz committed Apr 12, 2021
1 parent 6c36ac4 commit 4b7efa0
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 26 deletions.
108 changes: 108 additions & 0 deletions storage/boltz/crud_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,114 @@ func TestCrud(t *testing.T) {
t.Run("composite symbol", test.testCompositeSymbol)
}

func TestCursorDeleteNotBuggy(t *testing.T) {
test := &crudTest{}
test.Assertions = require.New(t)
test.init(false)
defer test.cleanup()

err := test.db.Update(func(tx *bbolt.Tx) error {
bucket, err := tx.CreateBucket([]byte("test-delete"))
if err != nil {
return err
}
if err := bucket.Put([]byte("a"), nil); err != nil {
return err
}
if err := bucket.Put([]byte("b"), nil); err != nil {
return err
}
return nil
})
test.NoError(err)

var vals []string
err = test.db.View(func(tx *bbolt.Tx) error {
bucket := tx.Bucket([]byte("test-delete"))
cursor := bucket.Cursor()
for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() {
vals = append(vals, string(key))
}
return nil
})
test.NoError(err)
test.Equal([]string{"a", "b"}, vals)

err = test.db.Update(func(tx *bbolt.Tx) error {
bucket := tx.Bucket([]byte("test-delete"))
cursor := bucket.Cursor()
for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() {
if err := cursor.Delete(); err != nil {
return nil
}
}
return nil
})
test.NoError(err)

vals = nil
err = test.db.View(func(tx *bbolt.Tx) error {
bucket := tx.Bucket([]byte("test-delete"))
cursor := bucket.Cursor()
for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() {
vals = append(vals, string(key))
}
return nil
})
test.NoError(err)
test.Nil(vals)
}

func TestCursorDeleteBuggy(t *testing.T) {
test := &crudTest{}
test.Assertions = require.New(t)
test.init(false)
defer test.cleanup()

err := test.db.Update(func(tx *bbolt.Tx) error {
bucket, err := tx.CreateBucket([]byte("test-delete"))
if err != nil {
return err
}
if err := bucket.Put([]byte("a"), nil); err != nil {
return err
}
if err := bucket.Put([]byte("b"), nil); err != nil {
return err
}

var vals []string
bucket = tx.Bucket([]byte("test-delete"))
cursor := bucket.Cursor()
for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() {
vals = append(vals, string(key))
}

test.NoError(err)
test.Equal([]string{"a", "b"}, vals)

bucket = tx.Bucket([]byte("test-delete"))
cursor = bucket.Cursor()
for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() {
if err := cursor.Delete(); err != nil {
return nil
}
}

vals = nil
bucket = tx.Bucket([]byte("test-delete"))
cursor = bucket.Cursor()
for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() {
vals = append(vals, string(key))
}

test.NoError(err)
test.Equal([]string{"b"}, vals)
return nil
})
test.NoError(err)
}

func TestLinkCollectionImpl_CheckIntegrity(t *testing.T) {
test := &crudTest{}
test.Assertions = require.New(t)
Expand Down
25 changes: 10 additions & 15 deletions storage/boltz/link_collection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ func (collection *linkCollectionImpl) RemoveLink(tx *bbolt.Tx, id []byte, key []

func (collection *linkCollectionImpl) SetLinks(tx *bbolt.Tx, id string, keys []string) error {
sort.Strings(keys)
bId := []byte(id)
fieldBucket := collection.getFieldBucketForStringId(tx, id)

var toAdd []string
var toRemove []string

if !fieldBucket.HasError() {
cursor := fieldBucket.Cursor()
Expand All @@ -129,9 +129,7 @@ func (collection *linkCollectionImpl) SetLinks(tx *bbolt.Tx, id string, keys []s
keys = keys[1:]
}
} else if compare > cursorCurrent {
if err := collection.unlinkCursor(tx, cursor, bId, val); err != nil {
return err
}
toRemove = append(toRemove, string(val))
rowHandled = true
break
} else {
Expand All @@ -142,9 +140,7 @@ func (collection *linkCollectionImpl) SetLinks(tx *bbolt.Tx, id string, keys []s
}

if !rowHandled {
if err := collection.unlinkCursor(tx, cursor, bId, val); err != nil {
return err
}
toRemove = append(toRemove, string(val))
continue
}
}
Expand All @@ -153,6 +149,11 @@ func (collection *linkCollectionImpl) SetLinks(tx *bbolt.Tx, id string, keys []s
if fieldBucket.HasError() {
return fieldBucket.Err
}

if err := collection.RemoveLinks(tx, id, toRemove...); err != nil {
return err
}

toAdd = append(toAdd, keys...)
return collection.AddLinks(tx, id, toAdd...)
}
Expand All @@ -165,7 +166,8 @@ func (collection *linkCollectionImpl) EntityDeleted(tx *bbolt.Tx, id string) err
cursor := fieldBucket.Cursor()
for val, _ := cursor.First(); val != nil; val, _ = cursor.Next() {
_, key := GetTypeAndValue(val)
if err := collection.unlinkCursor(tx, cursor, bId, key); err != nil {
// We don't need to delete the local entry b/c the parent bucket is getting deleted
if err := collection.otherField.RemoveLink(tx, key, bId); err != nil {
return err
}
}
Expand Down Expand Up @@ -277,13 +279,6 @@ func (collection *linkCollectionImpl) unlink(tx *bbolt.Tx, fieldBucket *TypedBuc
return collection.otherField.RemoveLink(tx, associatedId, id)
}

func (collection *linkCollectionImpl) unlinkCursor(tx *bbolt.Tx, cursor *bbolt.Cursor, id []byte, associatedId []byte) error {
if err := cursor.Delete(); err != nil {
return err
}
return collection.otherField.RemoveLink(tx, associatedId, id)
}

const MaxLinkedSetKeySize = 4096

type LinkedSetSymbol struct {
Expand Down
10 changes: 2 additions & 8 deletions storage/boltz/link_collection_rc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ func (collection *rcLinkCollectionImpl) EntityDeleted(tx *bbolt.Tx, id string) e
cursor := fieldBucket.Cursor()
for val, _ := cursor.First(); val != nil; val, _ = cursor.Next() {
_, key := GetTypeAndValue(val)
if err := collection.unlinkCursor(tx, cursor, bId, key); err != nil {
// We don't need to remove the local entry because the parent bucket is getting deleted
if err := collection.otherField.unlink(tx, key, bId); err != nil {
return err
}
}
Expand Down Expand Up @@ -177,13 +178,6 @@ func (collection *rcLinkCollectionImpl) decrementLinkCount(tx *bbolt.Tx, fieldBu
return newVal, nil
}

func (collection *rcLinkCollectionImpl) unlinkCursor(tx *bbolt.Tx, cursor *bbolt.Cursor, id []byte, associatedId []byte) error {
if err := cursor.Delete(); err != nil {
return err
}
return collection.otherField.unlink(tx, associatedId, id)
}

type RefCountedLinkedSetSymbol struct {
EntitySymbol
}
Expand Down
13 changes: 10 additions & 3 deletions storage/boltz/query_symbols.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,16 @@ func (symbol *entitySetSymbolImpl) Map(tx *bbolt.Tx, key []byte, f func(ctx *Map
var newValues []*FieldTypeAndValue
cursor := listBucket.Cursor()
ctx := &MapContext{}

var toDelete [][]byte
for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() {
ctx.next(GetTypeAndValue(key))
f(ctx)
if ctx.HasError() {
return ctx.GetError()
}
if ctx.replace {
if err := cursor.Delete(); err != nil {
return err
}
toDelete = append(toDelete, key)
if ctx.newVal != nil {
newValues = append(newValues, &FieldTypeAndValue{FieldType: ctx.newType, Value: ctx.newVal})
}
Expand All @@ -192,6 +192,13 @@ func (symbol *entitySetSymbolImpl) Map(tx *bbolt.Tx, key []byte, f func(ctx *Map
break
}
}

for _, key := range toDelete {
if err := listBucket.Delete(key); err != nil {
return err
}
}

for _, val := range newValues {
listBucket.SetListEntry(val.FieldType, val.Value)
}
Expand Down

0 comments on commit 4b7efa0

Please sign in to comment.