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

User persister factory in storage unit #32

Merged
merged 4 commits into from
Dec 7, 2023
Merged

User persister factory in storage unit #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f0f3a4
added persister factory interface
ssd04 Dec 4, 2023
53909e7
update storage unit tests
ssd04 Dec 5, 2023
0943b1e
added todo comment
ssd04 Dec 5, 2023
873df9f
fixes after review
ssd04 Dec 5, 2023
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
42 changes: 20 additions & 22 deletions storageUnit/storageunit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package storageUnit
import (
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"sync"
"time"
Expand All @@ -16,9 +17,7 @@ import (
logger "github.com/multiversx/mx-chain-logger-go"
"github.com/multiversx/mx-chain-storage-go/common"
"github.com/multiversx/mx-chain-storage-go/fifocache"
"github.com/multiversx/mx-chain-storage-go/leveldb"
"github.com/multiversx/mx-chain-storage-go/lrucache"
"github.com/multiversx/mx-chain-storage-go/memorydb"
"github.com/multiversx/mx-chain-storage-go/monitoring"
"github.com/multiversx/mx-chain-storage-go/types"
)
Expand Down Expand Up @@ -75,6 +74,9 @@ const MaxRetriesToCreateDB = 10
// SleepTimeBetweenCreateDBRetries represents the number of seconds to sleep between DB creates
const SleepTimeBetweenCreateDBRetries = 5 * time.Second

// ErrNilPersisterFactory signals that a nil persister factory handler has been provided
var ErrNilPersisterFactory = errors.New("nil persister factory")

// UnitConfig holds the configurable elements of the storage unit
type UnitConfig struct {
CacheConf CacheConfig
Expand Down Expand Up @@ -289,8 +291,14 @@ func NewStorageUnit(c types.Cacher, p types.Persister) (*Unit, error) {
return sUnit, nil
}

// PersisterFactoryHandler defines the behaviour of a component which is able to create persisters
type PersisterFactoryHandler interface {
Create(path string) (types.Persister, error)
IsInterfaceNil() bool
}

// NewStorageUnitFromConf creates a new storage unit from a storage unit config
func NewStorageUnitFromConf(cacheConf CacheConfig, dbConf DBConfig) (*Unit, error) {
func NewStorageUnitFromConf(cacheConf CacheConfig, dbConf DBConfig, persisterFactory PersisterFactoryHandler) (*Unit, error) {
var cache types.Cacher
var db types.Persister
var err error
Expand All @@ -307,14 +315,7 @@ func NewStorageUnitFromConf(cacheConf CacheConfig, dbConf DBConfig) (*Unit, erro
return nil, err
}

argDB := ArgDB{
DBType: dbConf.Type,
Path: dbConf.FilePath,
BatchDelaySeconds: dbConf.BatchDelaySeconds,
MaxBatchSize: dbConf.MaxBatchSize,
MaxOpenFiles: dbConf.MaxOpenFiles,
}
db, err = NewDB(argDB)
db, err = NewDB(persisterFactory, dbConf.FilePath)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -378,21 +379,18 @@ type ArgDB struct {
}

// NewDB creates a new database from database config
func NewDB(argDB ArgDB) (types.Persister, error) {
// TODO: refactor to integrate retries loop into persister factory; maybe implement persister
// factory separatelly in storage repo
func NewDB(persisterFactory PersisterFactoryHandler, path string) (types.Persister, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

for extra protection (even if this is temporary) I would add a nil check for the provided persisterFactory. Maybe make the interface implement NilInterfaceChecker interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added nil check

if check.IfNil(persisterFactory) {
return nil, ErrNilPersisterFactory
}

var db types.Persister
var err error

for i := 0; i < MaxRetriesToCreateDB; i++ {
switch argDB.DBType {
case LvlDB:
db, err = leveldb.NewDB(argDB.Path, argDB.BatchDelaySeconds, argDB.MaxBatchSize, argDB.MaxOpenFiles)
case LvlDBSerial:
db, err = leveldb.NewSerialDB(argDB.Path, argDB.BatchDelaySeconds, argDB.MaxBatchSize, argDB.MaxOpenFiles)
case MemoryDB:
db = memorydb.New()
default:
return nil, common.ErrNotSupportedDBType
}
db, err = persisterFactory.Create(path)

if err == nil {
return db, nil
Expand Down
70 changes: 41 additions & 29 deletions storageUnit/storageunit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/multiversx/mx-chain-storage-go/lrucache"
"github.com/multiversx/mx-chain-storage-go/memorydb"
"github.com/multiversx/mx-chain-storage-go/storageUnit"
"github.com/multiversx/mx-chain-storage-go/testscommon"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -254,14 +255,13 @@ func TestCreateCacheFromConfOK(t *testing.T) {
}

func TestCreateDBFromConfWrongType(t *testing.T) {
arg := storageUnit.ArgDB{
DBType: "NotLvlDB",
Path: "test",
BatchDelaySeconds: 10,
MaxBatchSize: 10,
MaxOpenFiles: 10,
}
persister, err := storageUnit.NewDB(arg)
persisterFactory := testscommon.NewPersisterFactoryHandlerMock(
"NotLvlDB",
10,
10,
10,
)
persister, err := storageUnit.NewDB(persisterFactory, "test")

assert.NotNil(t, err, "error expected")
assert.Nil(t, persister, "persister expected to be nil, but got %s", persister)
Expand All @@ -272,27 +272,29 @@ func TestCreateDBFromConfWrongFileNameLvlDB(t *testing.T) {
t.Skip("this is not a short test")
}

arg := storageUnit.ArgDB{
DBType: storageUnit.LvlDB,
Path: "",
BatchDelaySeconds: 10,
MaxBatchSize: 10,
MaxOpenFiles: 10,
}
persister, err := storageUnit.NewDB(arg)
path := ""
persisterFactory := testscommon.NewPersisterFactoryHandlerMock(
storageUnit.LvlDB,
10,
10,
10,
)

persister, err := storageUnit.NewDB(persisterFactory, path)
assert.NotNil(t, err, "error expected")
assert.Nil(t, persister, "persister expected to be nil, but got %s", persister)
}

func TestCreateDBFromConfLvlDBOk(t *testing.T) {
arg := storageUnit.ArgDB{
DBType: storageUnit.LvlDB,
Path: t.TempDir(),
BatchDelaySeconds: 10,
MaxBatchSize: 10,
MaxOpenFiles: 10,
}
persister, err := storageUnit.NewDB(arg)
path := t.TempDir()
persisterFactory := testscommon.NewPersisterFactoryHandlerMock(
storageUnit.LvlDB,
10,
10,
10,
)

persister, err := storageUnit.NewDB(persisterFactory, path)
assert.Nil(t, err, "no error expected")
assert.NotNil(t, persister, "valid persister expected but got nil")

Expand All @@ -311,7 +313,9 @@ func TestNewStorageUnit_FromConfWrongCacheSizeVsBatchSize(t *testing.T) {
MaxBatchSize: 11,
BatchDelaySeconds: 1,
MaxOpenFiles: 10,
})
},
testscommon.NewPersisterFactoryHandlerMock(storageUnit.LvlDB, 11, 1, 10),
)

assert.NotNil(t, err, "error expected")
assert.Nil(t, storer, "storer expected to be nil but got %s", storer)
Expand All @@ -328,7 +332,9 @@ func TestNewStorageUnit_FromConfWrongCacheConfig(t *testing.T) {
BatchDelaySeconds: 1,
MaxBatchSize: 1,
MaxOpenFiles: 10,
})
},
testscommon.NewPersisterFactoryHandlerMock(storageUnit.LvlDB, 1, 1, 10),
)

assert.NotNil(t, err, "error expected")
assert.Nil(t, storer, "storer expected to be nil but got %s", storer)
Expand All @@ -341,7 +347,9 @@ func TestNewStorageUnit_FromConfWrongDBConfig(t *testing.T) {
}, storageUnit.DBConfig{
FilePath: "Blocks",
Type: "NotLvlDB",
})
},
testscommon.NewPersisterFactoryHandlerMock("NotLvlDB", 0, 0, 0),
)

assert.NotNil(t, err, "error expected")
assert.Nil(t, storer, "storer expected to be nil but got %s", storer)
Expand All @@ -357,7 +365,9 @@ func TestNewStorageUnit_FromConfLvlDBOk(t *testing.T) {
MaxBatchSize: 1,
BatchDelaySeconds: 1,
MaxOpenFiles: 10,
})
},
testscommon.NewPersisterFactoryHandlerMock(storageUnit.LvlDB, 1, 1, 10),
)

assert.Nil(t, err, "no error expected but got %s", err)
assert.NotNil(t, storer, "valid storer expected but got nil")
Expand All @@ -375,7 +385,9 @@ func TestNewStorageUnit_ShouldWorkLvlDB(t *testing.T) {
BatchDelaySeconds: 1,
MaxBatchSize: 1,
MaxOpenFiles: 10,
})
},
testscommon.NewPersisterFactoryHandlerMock(storageUnit.LvlDB, 1, 1, 10),
)

assert.Nil(t, err, "no error expected but got %s", err)
assert.NotNil(t, storer, "valid storer expected but got nil")
Expand Down
45 changes: 45 additions & 0 deletions testscommon/persisterFactoryHandlerMock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package testscommon

import (
"github.com/multiversx/mx-chain-storage-go/common"
"github.com/multiversx/mx-chain-storage-go/leveldb"
"github.com/multiversx/mx-chain-storage-go/memorydb"
"github.com/multiversx/mx-chain-storage-go/storageUnit"
"github.com/multiversx/mx-chain-storage-go/types"
)

type persisterFactoryHandlerMock struct {
dbType storageUnit.DBType
batchDelaySeconds int
maxBatchSize int
maxOpenFiles int
}

// NewPersisterFactoryHandlerMock -
func NewPersisterFactoryHandlerMock(dbType storageUnit.DBType, batchDelaySeconds int, maxBatchSize int, maxOpenFiles int) *persisterFactoryHandlerMock {
return &persisterFactoryHandlerMock{
dbType: dbType,
batchDelaySeconds: batchDelaySeconds,
maxBatchSize: maxBatchSize,
maxOpenFiles: maxOpenFiles,
}
}

// Create -
func (mock *persisterFactoryHandlerMock) Create(path string) (types.Persister, error) {
switch mock.dbType {
case storageUnit.LvlDB:
return leveldb.NewDB(path, mock.batchDelaySeconds, mock.maxBatchSize, mock.maxOpenFiles)
case storageUnit.LvlDBSerial:
return leveldb.NewSerialDB(path, mock.batchDelaySeconds, mock.maxBatchSize, mock.maxOpenFiles)
case storageUnit.MemoryDB:
return memorydb.New(), nil
default:
return nil, common.ErrNotSupportedDBType
}
}

// IsInterfaceNil -
func (mock *persisterFactoryHandlerMock) IsInterfaceNil() bool {
return mock == nil
}