From 11a44a94b11994a2e6383b2ef2971167e49bec15 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Fri, 25 Jun 2021 17:16:30 +0200 Subject: [PATCH 1/2] etcd: remove unnecessary iterator step from cursor Delete The etcd cursor Delete stepped to the next item in the range before Delete to not invalidate the iteation. This is unnecessary and not compatible with bbolt, resulting in an extra fetch too. --- kvdb/etcd/readwrite_cursor.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/kvdb/etcd/readwrite_cursor.go b/kvdb/etcd/readwrite_cursor.go index fb408b8f..123f2a5c 100644 --- a/kvdb/etcd/readwrite_cursor.go +++ b/kvdb/etcd/readwrite_cursor.go @@ -116,23 +116,11 @@ func (c *readWriteCursor) Seek(seek []byte) (key, value []byte) { // invalidating the cursor. Returns ErrIncompatibleValue if attempted // when the cursor points to a nested bucket. func (c *readWriteCursor) Delete() error { - // Get the next key after the current one. We could do this - // after deletion too but it's one step more efficient here. - nextKey, err := c.bucket.tx.stm.Next(c.prefix, c.currKey) - if err != nil { - return err - } - if isBucketKey(c.currKey) { c.bucket.DeleteNestedBucket(getKey(c.currKey)) } else { c.bucket.Delete(getKey(c.currKey)) } - if nextKey != nil { - // Set current key to the next one. - c.currKey = nextKey.key - } - return nil } From 96caa6f24211e90ce623de54a6d8a42ba32ba6fb Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Wed, 16 Jun 2021 16:56:38 +0200 Subject: [PATCH 2/2] etcd: remove assertion when creating bucket and value with the same key This commit removes an assertion which is not needed because with etcd we can safely create keys and values with the same key since they are stored under different keys in the DB. This saves us one unnecessary Get on every Put. --- kvdb/debug.go | 8 ++++++++ kvdb/etcd/debug.go | 8 ++++++++ kvdb/etcd/nodebug.go | 8 ++++++++ kvdb/etcd/readwrite_bucket.go | 29 +++++++++++++++++++++++------ kvdb/etcd_test.go | 10 ++++++++-- kvdb/nodebug.go | 8 ++++++++ 6 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 kvdb/debug.go create mode 100644 kvdb/etcd/debug.go create mode 100644 kvdb/etcd/nodebug.go create mode 100644 kvdb/nodebug.go diff --git a/kvdb/debug.go b/kvdb/debug.go new file mode 100644 index 00000000..07874bea --- /dev/null +++ b/kvdb/debug.go @@ -0,0 +1,8 @@ +// +build dev + +package kvdb + +const ( + // Switch on extra debug code. + etcdDebug = true +) diff --git a/kvdb/etcd/debug.go b/kvdb/etcd/debug.go new file mode 100644 index 00000000..5b67b0bb --- /dev/null +++ b/kvdb/etcd/debug.go @@ -0,0 +1,8 @@ +// +build dev + +package etcd + +const ( + // Switch on extra debug code. + etcdDebug = true +) diff --git a/kvdb/etcd/nodebug.go b/kvdb/etcd/nodebug.go new file mode 100644 index 00000000..9494f8cc --- /dev/null +++ b/kvdb/etcd/nodebug.go @@ -0,0 +1,8 @@ +// +build !dev + +package etcd + +const ( + // Switch off extra debug code. + etcdDebug = false +) diff --git a/kvdb/etcd/readwrite_bucket.go b/kvdb/etcd/readwrite_bucket.go index 0be2f88e..7591d7f9 100644 --- a/kvdb/etcd/readwrite_bucket.go +++ b/kvdb/etcd/readwrite_bucket.go @@ -118,6 +118,10 @@ func (b *readWriteBucket) NestedReadWriteBucket(key []byte) walletdb.ReadWriteBu // assertNoValue checks if the value for the passed key exists. func (b *readWriteBucket) assertNoValue(key []byte) error { + if !etcdDebug { + return nil + } + val, err := b.tx.stm.Get(string(makeValueKey(b.id, key))) if err != nil { return err @@ -130,6 +134,24 @@ func (b *readWriteBucket) assertNoValue(key []byte) error { return nil } +// assertNoBucket checks if the bucket for the passed key exists. +func (b *readWriteBucket) assertNoBucket(key []byte) error { + if !etcdDebug { + return nil + } + + val, err := b.tx.stm.Get(string(makeBucketKey(b.id, key))) + if err != nil { + return err + } + + if val != nil { + return walletdb.ErrIncompatibleValue + } + + return nil +} + // CreateBucket creates and returns a new nested bucket with the given // key. Returns ErrBucketExists if the bucket already exists, // ErrBucketNameRequired if the key is empty, or ErrIncompatibleValue @@ -272,15 +294,10 @@ func (b *readWriteBucket) Put(key, value []byte) error { return walletdb.ErrKeyRequired } - val, err := b.tx.stm.Get(string(makeBucketKey(b.id, key))) - if err != nil { + if err := b.assertNoBucket(key); err != nil { return err } - if val != nil { - return walletdb.ErrIncompatibleValue - } - // Update the transaction with the new value. b.tx.stm.Put(string(makeValueKey(b.id, key)), string(value)) diff --git a/kvdb/etcd_test.go b/kvdb/etcd_test.go index fc81bb93..6d96d284 100644 --- a/kvdb/etcd_test.go +++ b/kvdb/etcd_test.go @@ -19,6 +19,7 @@ var ( func TestEtcd(t *testing.T) { tests := []struct { name string + debugOnly bool test func(*testing.T, walletdb.DB) expectedDb map[string]string }{ @@ -103,8 +104,9 @@ func TestEtcd(t *testing.T) { test: testBucketSequence, }, { - name: "key clash", - test: testKeyClash, + name: "key clash", + debugOnly: true, + test: testKeyClash, expectedDb: map[string]string{ bkey("apple"): bval("apple"), bkey("apple", "banana"): bval("apple", "banana"), @@ -137,6 +139,10 @@ func TestEtcd(t *testing.T) { for _, test := range tests { test := test + if test.debugOnly && !etcdDebug { + continue + } + t.Run(test.name, func(t *testing.T) { t.Parallel() diff --git a/kvdb/nodebug.go b/kvdb/nodebug.go new file mode 100644 index 00000000..76225182 --- /dev/null +++ b/kvdb/nodebug.go @@ -0,0 +1,8 @@ +// +build !dev + +package kvdb + +const ( + // Switch off extra debug code. + etcdDebug = false +)