From dfdbfa04c254262f90774bd22cd53e89cc9085a3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 27 Jul 2017 15:20:43 -0400 Subject: [PATCH] Use KVMemCache for checkpointing IAVL tree as they are not go-routine safe... --- app/store.go | 7 +++- state/bonsai.go | 44 ++++++++++++++++---- state/chainstate_test.go | 88 ---------------------------------------- state/kvcache.go | 3 ++ state/kvstore.go | 3 +- state/merkle.go | 35 ++++++++++------ 6 files changed, 68 insertions(+), 112 deletions(-) delete mode 100644 state/chainstate_test.go diff --git a/app/store.go b/app/store.go index f6ae197513..4c3bc8adac 100644 --- a/app/store.go +++ b/app/store.go @@ -120,8 +120,13 @@ func (s *Store) Info() abci.ResponseInfo { // Commit implements abci.Application func (s *Store) Commit() abci.Result { - s.hash = s.State.Hash() + var err error s.height++ + s.hash, err = s.State.Hash() + if err != nil { + return abci.NewError(abci.CodeType_InternalError, err.Error()) + } + s.logger.Debug("Commit synced", "height", s.height, "hash", fmt.Sprintf("%X", s.hash)) diff --git a/state/bonsai.go b/state/bonsai.go index 57abe714ed..2a98732985 100644 --- a/state/bonsai.go +++ b/state/bonsai.go @@ -73,24 +73,50 @@ func (b *Bonsai) Last(start, end []byte) Model { } func (b *Bonsai) Checkpoint() SimpleDB { - return &Bonsai{ - id: b.id, - Tree: b.Tree.Copy(), - } + return NewMemKVCache(b) } -// Commit will take all changes from the checkpoint and write -// them to the parent. -// Returns an error if this is not a child of this one func (b *Bonsai) Commit(sub SimpleDB) error { - bb, ok := sub.(*Bonsai) + cache, ok := sub.(*MemKVCache) + if !ok { + return ErrNotASubTransaction() + } + // see if it was wrapping this struct + bb, ok := cache.store.(*Bonsai) if !ok || (b.id != bb.id) { return ErrNotASubTransaction() } - b.Tree = bb.Tree + + // apply the cached data to the Bonsai + cache.applyCache() return nil } +//---------------------------------------- +// This is the checkpointing I want, but apparently iavl-tree is not +// as immutable as I hoped... paniced in multiple go-routines :( +// +// FIXME: use this code when iavltree is improved + +// func (b *Bonsai) Checkpoint() SimpleDB { +// return &Bonsai{ +// id: b.id, +// Tree: b.Tree.Copy(), +// } +// } + +// // Commit will take all changes from the checkpoint and write +// // them to the parent. +// // Returns an error if this is not a child of this one +// func (b *Bonsai) Commit(sub SimpleDB) error { +// bb, ok := sub.(*Bonsai) +// if !ok || (b.id != bb.id) { +// return ErrNotASubTransaction() +// } +// b.Tree = bb.Tree +// return nil +// } + // Discard will remove reference to this func (b *Bonsai) Discard() { b.id = 0 diff --git a/state/chainstate_test.go b/state/chainstate_test.go deleted file mode 100644 index 096d27d197..0000000000 --- a/state/chainstate_test.go +++ /dev/null @@ -1,88 +0,0 @@ -package state - -// TODO: something similar in the merkle package... - -// import ( -// "bytes" -// "testing" - -// eyes "github.com/tendermint/merkleeyes/client" -// "github.com/tendermint/tmlibs/log" - -// "github.com/stretchr/testify/assert" -// ) - -// func TestState(t *testing.T) { -// assert := assert.New(t) - -// //States and Stores for tests -// store := NewMemKVStore() -// state := NewState(store, log.TestingLogger()) -// cache := state.CacheWrap() -// eyesCli := eyes.NewLocalClient("", 0) - -// //reset the store/state/cache -// reset := func() { -// store = NewMemKVStore() -// state = NewState(store, log.TestingLogger()) -// cache = state.CacheWrap() -// } - -// //set the state to using the eyesCli instead of MemKVStore -// useEyesCli := func() { -// state = NewState(eyesCli, log.TestingLogger()) -// cache = state.CacheWrap() -// } - -// //key value pairs to be tested within the system -// keyvalue := []struct { -// key string -// value string -// }{ -// {"foo", "snake"}, -// {"bar", "mouse"}, -// } - -// //set the kvc to have all the key value pairs -// setRecords := func(kv KVStore) { -// for _, n := range keyvalue { -// kv.Set([]byte(n.key), []byte(n.value)) -// } -// } - -// //store has all the key value pairs -// storeHasAll := func(kv KVStore) bool { -// for _, n := range keyvalue { -// if !bytes.Equal(kv.Get([]byte(n.key)), []byte(n.value)) { -// return false -// } -// } -// return true -// } - -// //test chainID -// state.SetChainID("testchain") -// assert.Equal(state.GetChainID(), "testchain", "ChainID is improperly stored") - -// //test basic retrieve -// setRecords(state) -// assert.True(storeHasAll(state), "state doesn't retrieve after Set") - -// //Test CacheWrap with local mem store -// reset() -// setRecords(cache) -// assert.False(storeHasAll(store), "store retrieving before CacheSync") -// cache.CacheSync() -// assert.True(storeHasAll(store), "store doesn't retrieve after CacheSync") - -// //Test Commit on state with non-merkle store -// assert.True(state.Commit().IsErr(), "Commit shouldn't work with non-merkle store") - -// //Test CacheWrap with merkleeyes client store -// useEyesCli() -// setRecords(cache) -// assert.False(storeHasAll(eyesCli), "eyesCli retrieving before Commit") -// cache.CacheSync() -// assert.True(state.Commit().IsOK(), "Bad Commit") -// assert.True(storeHasAll(eyesCli), "eyesCli doesn't retrieve after Commit") -// } diff --git a/state/kvcache.go b/state/kvcache.go index bd47fc8690..1141d20e3e 100644 --- a/state/kvcache.go +++ b/state/kvcache.go @@ -23,10 +23,12 @@ func NewMemKVCache(store SimpleDB) *MemKVCache { } } +// Set sets a key, fulfills KVStore interface func (c *MemKVCache) Set(key []byte, value []byte) { c.cache.Set(key, value) } +// Get gets a key, fulfills KVStore interface func (c *MemKVCache) Get(key []byte) (value []byte) { value, ok := c.cache.m[string(key)] if !ok { @@ -36,6 +38,7 @@ func (c *MemKVCache) Get(key []byte) (value []byte) { return value } +// Has checks existence of a key, fulfills KVStore interface func (c *MemKVCache) Has(key []byte) bool { value := c.Get(key) return value != nil diff --git a/state/kvstore.go b/state/kvstore.go index 6233dddb02..08dd85f217 100644 --- a/state/kvstore.go +++ b/state/kvstore.go @@ -1,7 +1,6 @@ package state import ( - "errors" "sort" "github.com/tendermint/go-wire/data" @@ -148,7 +147,7 @@ func (m *MemKVStore) Checkpoint() SimpleDB { func (m *MemKVStore) Commit(sub SimpleDB) error { cache, ok := sub.(*MemKVCache) if !ok { - return errors.New("sub is not a cache") + return ErrNotASubTransaction() } // TODO: see if it points to us diff --git a/state/merkle.go b/state/merkle.go index 56c9349e78..f383fd0d5a 100644 --- a/state/merkle.go +++ b/state/merkle.go @@ -9,8 +9,8 @@ import ( // from the working state (for CheckTx and AppendTx) type State struct { committed *Bonsai - deliverTx *Bonsai - checkTx *Bonsai + deliverTx SimpleDB + checkTx SimpleDB persistent bool } @@ -18,8 +18,8 @@ func NewState(tree merkle.Tree, persistent bool) State { base := NewBonsai(tree) return State{ committed: base, - deliverTx: base.Checkpoint().(*Bonsai), - checkTx: base.Checkpoint().(*Bonsai), + deliverTx: base.Checkpoint(), + checkTx: base.Checkpoint(), persistent: persistent, } } @@ -28,24 +28,34 @@ func (s State) Committed() *Bonsai { return s.committed } -func (s State) Append() *Bonsai { +func (s State) Append() SimpleDB { return s.deliverTx } -func (s State) Check() *Bonsai { +func (s State) Check() SimpleDB { return s.checkTx } -// Hash updates the tree -func (s *State) Hash() []byte { - return s.deliverTx.Hash() +// Hash applies deliverTx to committed and calculates hash +// +// Note the usage: +// Hash -> gets the calculated hash but doesn't save +// BatchSet -> adds some out-of-bounds data +// Commit -> Save everything to disk and the same hash +func (s *State) Hash() ([]byte, error) { + err := s.committed.Commit(s.deliverTx) + if err != nil { + return nil, err + } + s.deliverTx = s.committed.Checkpoint() + return s.committed.Tree.Hash(), nil } // BatchSet is used for some weird magic in storing the new height func (s *State) BatchSet(key, value []byte) { if s.persistent { // This is in the batch with the Save, but not in the tree - tree, ok := s.deliverTx.Tree.(*iavl.IAVLTree) + tree, ok := s.committed.Tree.(*iavl.IAVLTree) if ok { tree.BatchSet(key, value) } @@ -54,6 +64,7 @@ func (s *State) BatchSet(key, value []byte) { // Commit save persistent nodes to the database and re-copies the trees func (s *State) Commit() ([]byte, error) { + // commit (if we didn't do hash earlier) err := s.committed.Commit(s.deliverTx) if err != nil { return nil, err @@ -66,7 +77,7 @@ func (s *State) Commit() ([]byte, error) { hash = s.committed.Tree.Hash() } - s.deliverTx = s.committed.Checkpoint().(*Bonsai) - s.checkTx = s.committed.Checkpoint().(*Bonsai) + s.deliverTx = s.committed.Checkpoint() + s.checkTx = s.committed.Checkpoint() return hash, nil }