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.

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

[db] KvWithVersion to handle both versioned and non-versioned namespace #4518

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 3 additions & 0 deletions db/db_versioned.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type (

// Version returns the key's most recent version
Version(string, []byte) (uint64, error)

// CommitToDB writes a batch to the underlying DB
CommitToDB(uint64, map[string]bool, batch.KVStoreBatch) error
}

// BoltDBVersioned is KvVersioned implementation based on bolt DB
Expand Down
108 changes: 108 additions & 0 deletions db/kvstore_versioned.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
package db

import (
"context"

"github.com/pkg/errors"

"github.com/iotexproject/iotex-core/v2/db/batch"
"github.com/iotexproject/iotex-core/v2/pkg/lifecycle"
)

Expand Down Expand Up @@ -47,4 +52,107 @@ type (
// SetVersion sets the version, and returns a KVStore to call Put()/Get()
SetVersion(uint64) KVStore
}

// KvWithVersion wraps the versioned DB implementation with a certain version
KvWithVersion struct {
db VersionedDB
kvBase KVStore
versioned map[string]bool // map of versioned buckets
version uint64 // the current version
}
)

// Option sets an option
type Option func(*KvWithVersion)

func VersionedNamespaceOption(ns ...string) Option {
return func(k *KvWithVersion) {
k.versioned = make(map[string]bool)
for _, ns := range ns {
k.versioned[ns] = true
}
}
}

// NewKVStoreWithVersion implements a KVStore that can handle both versioned
// and non-versioned namespace
func NewKVStoreWithVersion(cfg Config, opts ...Option) *KvWithVersion {
db := NewBoltDBVersioned(cfg)
kv := KvWithVersion{
db: db,
kvBase: db.Base(),
Comment on lines +82 to +83
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may need to review the implementation of BoltDBWithVersion. From the name, BoltDBWithVersion sounds like an implementation of KVStoreWithVersion. Moreover, db.Base() is a weird interface.

}
for _, opt := range opts {
opt(&kv)
}
return &kv
}

// Start starts the DB
func (b *KvWithVersion) Start(ctx context.Context) error {
return b.kvBase.Start(ctx)
}

// Stop stops the DB
func (b *KvWithVersion) Stop(ctx context.Context) error {
return b.kvBase.Stop(ctx)
}

// Put writes a <key, value> record
func (b *KvWithVersion) Put(ns string, key, value []byte) error {
if b.versioned[ns] {
return b.db.Put(b.version, ns, key, value)
}
return b.kvBase.Put(ns, key, value)
Copy link
Member

Choose a reason for hiding this comment

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

Using kvBase in kvWithVersion is strange, I would recommend splitting it up. for example:

KvVersioned interface{
    Version(string, []byte) (uint64, error)
    SetVersion(uint64) KVStore
    Plain() KVStore
}

Copy link
Member Author

Choose a reason for hiding this comment

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

do you mean add a Plain() interface?
this is internal implementation, for non-versioned namespace, it is a regular Put/Get/Delete, same as now

}

// Get retrieves a key's value
func (b *KvWithVersion) Get(ns string, key []byte) ([]byte, error) {
if b.versioned[ns] {
return b.db.Get(b.version, ns, key)
}
return b.kvBase.Get(ns, key)
}

// Delete deletes a key
func (b *KvWithVersion) Delete(ns string, key []byte) error {
if b.versioned[ns] {
return b.db.Delete(b.version, ns, key)
}
return b.kvBase.Delete(ns, key)
}

// Filter returns <k, v> pair in a bucket that meet the condition
func (b *KvWithVersion) Filter(ns string, cond Condition, minKey, maxKey []byte) ([][]byte, [][]byte, error) {
if b.versioned[ns] {
panic("Filter not supported for versioned DB")
}
return b.kvBase.Filter(ns, cond, minKey, maxKey)
}

// WriteBatch commits a batch
func (b *KvWithVersion) WriteBatch(kvsb batch.KVStoreBatch) error {
return b.db.CommitToDB(b.version, b.versioned, kvsb)
}

// Version returns the key's most recent version
func (b *KvWithVersion) Version(ns string, key []byte) (uint64, error) {
if b.versioned[ns] {
return b.db.Version(ns, key)
}
return 0, errors.Errorf("namespace %s is non-versioned", ns)
}

// SetVersion sets the version, and returns a KVStore to call Put()/Get()
func (b *KvWithVersion) SetVersion(v uint64) KVStore {
kv := KvWithVersion{
db: b.db,
kvBase: b.kvBase,
versioned: make(map[string]bool),
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as db and kvBase, if versioned will not be updated, passing pointer is fine.

version: v,
}
for k := range b.versioned {
kv.versioned[k] = true
}
return &kv
}
Loading
Loading