Skip to content

Commit

Permalink
refactor: Initialize database in constructor (sourcenetwork#211)
Browse files Browse the repository at this point in the history 
* Explicitly return nil in case of no error

Is more obvious

* Initialize database in constructor

Makes it much harder to forget to initialize database (lots of functionality can partially work without doing so, leading to failures being detected later than need be)
  • Loading branch information
@AndrewSisley
AndrewSisley authored Feb 14, 2022
1 parent a337b44 commit e119260
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 95 deletions.
18 changes: 9 additions & 9 deletions bench/bench_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func SetupCollections(b *testing.B, ctx context.Context, db *defradb.DB, fixture
}

func SetupDBAndCollections(b *testing.B, ctx context.Context, fixture fixtures.Generator) (*defradb.DB, []client.Collection, error) {
db, err := NewTestDB(b)
db, err := NewTestDB(ctx, b)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -204,28 +204,28 @@ type dbInfo interface {
DB() *db.DB
}

func NewTestDB(t testing.TB) (*db.DB, error) {
func NewTestDB(ctx context.Context, t testing.TB) (*db.DB, error) {
//nolint
dbi, err := newBenchStoreInfo(t)
dbi, err := newBenchStoreInfo(ctx, t)
return dbi.DB(), err
}

func NewTestStorage(t testing.TB) (ds.Batching, error) {
dbi, err := newBenchStoreInfo(t)
func NewTestStorage(ctx context.Context, t testing.TB) (ds.Batching, error) {
dbi, err := newBenchStoreInfo(ctx, t)
return dbi.Rootstore(), err
}

func newBenchStoreInfo(t testing.TB) (dbInfo, error) {
func newBenchStoreInfo(ctx context.Context, t testing.TB) (dbInfo, error) {
var dbi dbInfo
var err error

switch storage {
case "memory":
dbi, err = testutils.NewBadgerMemoryDB()
dbi, err = testutils.NewBadgerMemoryDB(ctx)
case "badger":
dbi, err = testutils.NewBadgerFileDB(t)
dbi, err = testutils.NewBadgerFileDB(ctx, t)
case "memorymap":
dbi, err = testutils.NewMapDB()
dbi, err = testutils.NewMapDB(ctx)
default:
return nil, fmt.Errorf("invalid storage engine backend: %s", storage)
}
Expand Down
6 changes: 3 additions & 3 deletions bench/storage/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

func runStorageBenchGet(b *testing.B, ctx context.Context, valueSize, objCount, opCount int, doSync bool) error {
db, err := benchutils.NewTestStorage(b)
db, err := benchutils.NewTestStorage(ctx, b)
if err != nil {
return err
}
Expand Down Expand Up @@ -49,7 +49,7 @@ func runStorageBenchGet(b *testing.B, ctx context.Context, valueSize, objCount,
}

func runStorageBenchPut(b *testing.B, ctx context.Context, valueSize, objCount, opCount int, doSync bool) error {
db, err := benchutils.NewTestStorage(b)
db, err := benchutils.NewTestStorage(ctx, b)
if err != nil {
return err
}
Expand Down Expand Up @@ -85,7 +85,7 @@ func runStorageBenchPut(b *testing.B, ctx context.Context, valueSize, objCount,
}

func runStorageBenchPutMany(b *testing.B, ctx context.Context, valueSize, objCount, opCount int, doSync bool) error {
db, err := benchutils.NewTestStorage(b)
db, err := benchutils.NewTestStorage(ctx, b)
if err != nil {
return err
}
Expand Down
7 changes: 1 addition & 6 deletions cli/defradb/cmd/serverdump.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,11 @@ var srvDumpCmd = &cobra.Command{
os.Exit(1)
}

db, err := db.NewDB(rootstore)
db, err := db.NewDB(ctx, rootstore)
if err != nil {
log.Error("Failed to initiate database:", err)
os.Exit(1)
}
if err := db.Start(ctx); err != nil {
log.Error("Failed to start the database: ", err)
db.Close()
os.Exit(1)
}

log.Info("Dumping DB state:")
db.PrintDump(ctx)
Expand Down
7 changes: 1 addition & 6 deletions cli/defradb/cmd/start.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,11 @@ var startCmd = &cobra.Command{
options = append(options, db.WithBroadcaster(bs))
}

db, err := db.NewDB(rootstore, options...)
db, err := db.NewDB(ctx, rootstore, options...)
if err != nil {
log.Error("Failed to initiate database:", err)
os.Exit(1)
}
if err := db.Start(ctx); err != nil {
log.Error("Failed to start the database: ", err)
db.Close()
os.Exit(1)
}

// init the p2p node
var n *node.Node
Expand Down
26 changes: 13 additions & 13 deletions db/collection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func createNewTestCollection(ctx context.Context, db *DB) (client.Collection, er

func TestNewCollection_ReturnsError_GivenNoSchema(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

_, err = createNewTestCollection(ctx, db)
Expand All @@ -70,7 +70,7 @@ func TestNewCollection_ReturnsError_GivenNoSchema(t *testing.T) {

func TestNewCollectionWithSchema(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

col, err := newTestCollectionWithSchema(ctx, db)
Expand All @@ -95,7 +95,7 @@ func TestNewCollectionWithSchema(t *testing.T) {

func TestNewCollectionReturnsErrorGivenDuplicateSchema(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

_, err = newTestCollectionWithSchema(ctx, db)
Expand All @@ -107,7 +107,7 @@ func TestNewCollectionReturnsErrorGivenDuplicateSchema(t *testing.T) {

func TestNewCollectionReturnsErrorGivenNoFields(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

desc := base.CollectionDescription{
Expand All @@ -123,7 +123,7 @@ func TestNewCollectionReturnsErrorGivenNoFields(t *testing.T) {

func TestNewCollectionReturnsErrorGivenNoName(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

desc := base.CollectionDescription{
Expand All @@ -139,7 +139,7 @@ func TestNewCollectionReturnsErrorGivenNoName(t *testing.T) {

func TestNewCollectionReturnsErrorGivenNoKeyField(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

desc := base.CollectionDescription{
Expand All @@ -161,7 +161,7 @@ func TestNewCollectionReturnsErrorGivenNoKeyField(t *testing.T) {

func TestNewCollectionReturnsErrorGivenKeyFieldIsNotFirstField(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

desc := base.CollectionDescription{
Expand All @@ -187,7 +187,7 @@ func TestNewCollectionReturnsErrorGivenKeyFieldIsNotFirstField(t *testing.T) {

func TestNewCollectionReturnsErrorGivenFieldWithNoName(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

desc := base.CollectionDescription{
Expand All @@ -213,7 +213,7 @@ func TestNewCollectionReturnsErrorGivenFieldWithNoName(t *testing.T) {

func TestNewCollectionReturnsErrorGivenFieldWithNoKind(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

desc := base.CollectionDescription{
Expand All @@ -238,7 +238,7 @@ func TestNewCollectionReturnsErrorGivenFieldWithNoKind(t *testing.T) {

func TestNewCollectionReturnsErrorGivenFieldWithNoType(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

desc := base.CollectionDescription{
Expand All @@ -263,7 +263,7 @@ func TestNewCollectionReturnsErrorGivenFieldWithNoType(t *testing.T) {

func TestGetCollection(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

_, err = newTestCollectionWithSchema(ctx, db)
Expand Down Expand Up @@ -291,7 +291,7 @@ func TestGetCollection(t *testing.T) {

func TestGetCollectionReturnsErrorGivenNonExistantCollection(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

_, err = db.GetCollection(ctx, "doesNotExist")
Expand All @@ -300,7 +300,7 @@ func TestGetCollectionReturnsErrorGivenNonExistantCollection(t *testing.T) {

func TestGetCollectionReturnsErrorGivenEmptyString(t *testing.T) {
ctx := context.Background()
db, err := newMemoryDB()
db, err := newMemoryDB(ctx)
assert.NoError(t, err)

_, err = db.GetCollection(ctx, "")
Expand Down
23 changes: 7 additions & 16 deletions db/db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ type DB struct {
schema *schema.SchemaManager
queryExecutor *planner.QueryExecutor

// indicates if this references an initalized database
initialized bool

log logging.StandardLogger

// The options used to init the database
Expand All @@ -95,7 +92,7 @@ func WithBroadcaster(bs corenet.Broadcaster) Option {
}

// NewDB creates a new instance of the DB using the given options
func NewDB(rootstore ds.Batching, options ...Option) (*DB, error) {
func NewDB(ctx context.Context, rootstore ds.Batching, options ...Option) (*DB, error) {
log.Debug("loading: internal datastores")
systemstore := namespace.Wrap(rootstore, base.SystemStoreKey)
datastore := namespace.Wrap(rootstore, base.DataStoreKey)
Expand Down Expand Up @@ -147,12 +144,12 @@ func NewDB(rootstore ds.Batching, options ...Option) (*DB, error) {
opt(db)
}

return db, err
}
err = db.initialize(ctx)
if err != nil {
return nil, err
}

// Start runs all the initial sub-routines and initialization steps.
func (db *DB) Start(ctx context.Context) error {
return db.Initialize(ctx)
return db, nil
}

// Root
Expand Down Expand Up @@ -182,15 +179,10 @@ func (db *DB) DAGstore() core.DAGStore {

// Initialize is called when a database is first run and creates all the db global meta data
// like Collection ID counters
func (db *DB) Initialize(ctx context.Context) error {
func (db *DB) initialize(ctx context.Context) error {
db.glock.Lock()
defer db.glock.Unlock()

// if its already initialized, just load the schema and we're done
if db.initialized {
return nil
}

log.Debug("Checking if db has already been initialized...")
exists, err := db.systemstore.Has(ctx, ds.NewKey("init"))
if err != nil && err != ds.ErrNotFound {
Expand All @@ -216,7 +208,6 @@ func (db *DB) Initialize(ctx context.Context) error {
return err
}

db.initialized = true
return nil
}

Expand Down
Loading

0 comments on commit e119260

Please sign in to comment.