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.

Sign up for GitHub

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

Jump to bottom

database: add applicable dbtests for linkeddb #3486

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+63 −17
Open

database: add applicable dbtests for linkeddb #3486

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c417191
database: add applicable dbtests for linkeddb
darioush Oct 21, 2024
b7bf31d
fix test name in map
darioush Oct 21, 2024
66f6cbd
test modifications
darioush Oct 21, 2024
5a4a240
reduce diff
darioush Oct 21, 2024
9a8e7eb
Update database/dbtest/dbtest.go
darioush Oct 21, 2024
fff8a8b
update linkeddb.go
darioush Oct 22, 2024
79d350f
Merge branch 'test-db-iface' of github.com:ava-labs/avalanchego into …
darioush Oct 22, 2024
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
53 changes: 36 additions & 17 deletions database/dbtest/dbtest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,20 @@ import (
"github.com/ava-labs/avalanchego/utils/units"
)

// TestsBasic is a list of all basic database tests that require only
// a KeyValueReaderWriterDeleter.
var TestsBasic = map[string]func(t *testing.T, db database.KeyValueReaderWriterDeleter){
"SimpleKeyValue": TestSimpleKeyValue,
"OverwriteKeyValue": TestOverwriteKeyValue,
"EmptyKey": TestEmptyKey,
"KeyEmptyValue": TestKeyEmptyValue,
"MemorySafetyDatabase": TestMemorySafetyDatabase,
"ModifyValueAfterPut": TestModifyValueAfterPut,
"PutGetEmpty": TestPutGetEmpty,
}

// Tests is a list of all database tests
var Tests = map[string]func(t *testing.T, db database.Database){
"SimpleKeyValue": TestSimpleKeyValue,
"OverwriteKeyValue": TestOverwriteKeyValue,
"EmptyKey": TestEmptyKey,
"KeyEmptyValue": TestKeyEmptyValue,
"SimpleKeyValueClosed": TestSimpleKeyValueClosed,
"NewBatchClosed": TestNewBatchClosed,
"BatchPut": TestBatchPut,
Expand All @@ -49,23 +57,29 @@ var Tests = map[string]func(t *testing.T, db database.Database){
"IteratorError": TestIteratorError,
"IteratorErrorAfterRelease": TestIteratorErrorAfterRelease,
"CompactNoPanic": TestCompactNoPanic,
"MemorySafetyDatabase": TestMemorySafetyDatabase,
"MemorySafetyBatch": TestMemorySafetyBatch,
"AtomicClear": TestAtomicClear,
"Clear": TestClear,
"AtomicClearPrefix": TestAtomicClearPrefix,
"ClearPrefix": TestClearPrefix,
"ModifyValueAfterPut": TestModifyValueAfterPut,
"ModifyValueAfterBatchPut": TestModifyValueAfterBatchPut,
"ModifyValueAfterBatchPutReplay": TestModifyValueAfterBatchPutReplay,
"ConcurrentBatches": TestConcurrentBatches,
"ManySmallConcurrentKVPairBatches": TestManySmallConcurrentKVPairBatches,
"PutGetEmpty": TestPutGetEmpty,
}

func init() {
// Add all basic database tests to the database tests
for name, test := range TestsBasic {
Tests[name] = func(t *testing.T, db database.Database) {
test(t, db)
}
}
}

// TestSimpleKeyValue tests to make sure that simple Put + Get + Delete + Has
// calls return the expected values.
func TestSimpleKeyValue(t *testing.T, db database.Database) {
func TestSimpleKeyValue(t *testing.T, db database.KeyValueReaderWriterDeleter) {
require := require.New(t)

key := []byte("hello")
Expand Down Expand Up @@ -101,7 +115,7 @@ func TestSimpleKeyValue(t *testing.T, db database.Database) {
require.NoError(db.Delete(key))
}

func TestOverwriteKeyValue(t *testing.T, db database.Database) {
func TestOverwriteKeyValue(t *testing.T, db database.KeyValueReaderWriterDeleter) {
require := require.New(t)

key := []byte("hello")
Expand All @@ -117,7 +131,7 @@ func TestOverwriteKeyValue(t *testing.T, db database.Database) {
require.Equal(value2, gotValue)
}

func TestKeyEmptyValue(t *testing.T, db database.Database) {
func TestKeyEmptyValue(t *testing.T, db database.KeyValueReaderWriterDeleter) {
require := require.New(t)

key := []byte("hello")
Expand All @@ -133,7 +147,7 @@ func TestKeyEmptyValue(t *testing.T, db database.Database) {
require.Empty(value)
}

func TestEmptyKey(t *testing.T, db database.Database) {
func TestEmptyKey(t *testing.T, db database.KeyValueReaderWriterDeleter) {
require := require.New(t)

var (
Expand Down Expand Up @@ -202,7 +216,7 @@ func TestSimpleKeyValueClosed(t *testing.T, db database.Database) {

// TestMemorySafetyDatabase ensures it is safe to modify a key after passing it
// to Database.Put and Database.Get.
func TestMemorySafetyDatabase(t *testing.T, db database.Database) {
func TestMemorySafetyDatabase(t *testing.T, db database.KeyValueReaderWriterDeleter) {
require := require.New(t)

key := []byte("1key")
Expand All @@ -211,9 +225,14 @@ func TestMemorySafetyDatabase(t *testing.T, db database.Database) {
key2 := []byte("2key")
value2 := []byte("value2")

// Put both K/V pairs in the database
// Put key in the database directly
require.NoError(db.Put(key, value))
require.NoError(db.Put(key2, value2))

// Put key2 in the database by modifying key, which should be safe
// to modify after the Put call
key[0] = key2[0]
require.NoError(db.Put(key, value2))
key[0] = keyCopy[0]

// Get the value for [key]
gotVal, err := db.Get(key)
Expand Down Expand Up @@ -1042,7 +1061,7 @@ func testClearPrefix(t *testing.T, db database.Database, clearF func(database.Da
require.NoError(db.Close())
}

func TestModifyValueAfterPut(t *testing.T, db database.Database) {
func TestModifyValueAfterPut(t *testing.T, db database.KeyValueReaderWriterDeleter) {
require := require.New(t)

key := []byte{1}
Expand Down Expand Up @@ -1166,7 +1185,7 @@ func runConcurrentBatches(
return eg.Wait()
}

func TestPutGetEmpty(t *testing.T, db database.Database) {
func TestPutGetEmpty(t *testing.T, db database.KeyValueReaderWriterDeleter) {
require := require.New(t)

key := []byte("hello")
Expand All @@ -1184,7 +1203,7 @@ func TestPutGetEmpty(t *testing.T, db database.Database) {
require.Empty(value) // May be nil or empty byte slice.
}

func FuzzKeyValue(f *testing.F, db database.Database) {
func FuzzKeyValue(f *testing.F, db database.KeyValueReaderWriterDeleter) {
f.Fuzz(func(t *testing.T, key []byte, value []byte) {
require := require.New(t)

Expand Down
25 changes: 25 additions & 0 deletions database/linkeddb/db_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package linkeddb

import (
"testing"

"github.com/ava-labs/avalanchego/database/dbtest"
"github.com/ava-labs/avalanchego/database/memdb"
)

func TestInterface(t *testing.T) {
for name, test := range dbtest.TestsBasic {
t.Run(name, func(t *testing.T) {
db := NewDefault(memdb.New())
test(t, db)
})
}
}

func FuzzKeyValue(f *testing.F) {
db := NewDefault(memdb.New())
dbtest.FuzzKeyValue(f, db)
}
2 changes: 1 addition & 1 deletion database/linkeddb/linkeddb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (ldb *linkedDB) getHeadKey() ([]byte, error) {
func (ldb *linkedDB) putHeadKey(key []byte) error {
ldb.headKeyIsUpdated = true
ldb.updatedHeadKeyExists = true
ldb.updatedHeadKey = key
ldb.updatedHeadKey = slices.Clone(key)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On mobile right now - but I think there are other instances in the linkeddb that we need to perform a copy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Put we hold a direct reference to key which can be problematic:

oldHead.Previous = key

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think putting the copy in putHeadKey results in an unnecessary copy during Delete as well. Maybe we could copy the key in Put once we know we are going to hold a reference to it (aka after we check that we aren't overwriting an existing node)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

return ldb.batch.Put(headKey, key)
}

Expand Down
Loading