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

fix(dot/state): add StorageState Lock/Unlock API for usage by babe and sync #1700

Merged
merged 32 commits into from
Jul 26, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
569d955
update gssmr genesis to have pallet_babe::SameAuthoritiesForever
noot Jul 14, 2021
f9236b7
update gssmr genesis.json
noot Jul 14, 2021
234ae15
restore auths
noot Jul 14, 2021
8cc3dd6
fix test
noot Jul 15, 2021
41429b3
attempt to fix trie state
noot Jul 15, 2021
d93d511
lock entire handleBlock func
noot Jul 15, 2021
f4df4c5
snapshot trie in StoreTrie
noot Jul 15, 2021
4563627
attempt to fix by copying branch.children
noot Jul 15, 2021
b813973
add trie snapshot concurrency test
noot Jul 20, 2021
f332399
fix tests, lint
noot Jul 20, 2021
4f68e4f
use StorageState-wide lock, seems to fix
noot Jul 20, 2021
25cfd53
update logs
noot Jul 20, 2021
226c88a
update build block to return if queue empty
noot Jul 20, 2021
b4a9489
fix ordering of SetContextStorage and lock
noot Jul 20, 2021
9148108
re-add StorageState-wide lock
noot Jul 20, 2021
c3e8a2c
lock before calling TrieState
noot Jul 21, 2021
35a5b3a
use storageState lock, cleanup
noot Jul 21, 2021
7bad042
cleanup
noot Jul 21, 2021
edf6c1b
re-add trie deletion if syncing
noot Jul 21, 2021
071cb67
Merge branch 'development' of github.com:ChainSafe/gossamer into noot…
noot Jul 21, 2021
c2a6bee
fix test
noot Jul 21, 2021
9c6026c
fmt
noot Jul 21, 2021
c3b38be
Merge branch 'development' into noot/fix-trie-state
noot Jul 21, 2021
0440355
Merge branch 'development' of github.com:ChainSafe/gossamer into noot…
noot Jul 22, 2021
75d2896
Merge branch 'noot/fix-trie-state' of github.com:ChainSafe/gossamer i…
noot Jul 22, 2021
f6db69a
fix
noot Jul 22, 2021
cc97361
use storageState Lock/Unlock
noot Jul 22, 2021
67a5849
fix
noot Jul 23, 2021
7201436
merge w development
noot Jul 23, 2021
3606069
fix invalid runtime stop (#1705)
noot Jul 23, 2021
5eff1ff
Merge branch 'development' into noot/fix-trie-state
noot Jul 24, 2021
e874b01
skip failing test
noot Jul 26, 2021
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
14 changes: 7 additions & 7 deletions dot/core/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,16 +307,17 @@ func (s *Service) handleCurrentSlot(header *types.Header) error {
// does not need to be completed before the next block can be imported.
func (s *Service) handleBlocksAsync() {
for {
prev := s.blockState.BestBlockHash()

select {
case block := <-s.blockAddCh:
if block == nil {
continue
}

// TODO: add inherent check
// if err := s.handleChainReorg(prev, block.Header.Hash()); err != nil {
// logger.Warn("failed to re-add transactions to chain upon re-org", "error", err)
// }
if err := s.handleChainReorg(prev, block.Header.Hash()); err != nil {
logger.Warn("failed to re-add transactions to chain upon re-org", "error", err)
}

if err := s.maintainTransactionPool(block); err != nil {
logger.Warn("failed to maintain transaction pool", "error", err)
Expand Down Expand Up @@ -422,12 +423,11 @@ func (s *Service) maintainTransactionPool(block *types.Block) error {
// re-validate transactions in the pool and move them to the queue
txs := s.transactionState.PendingInPool()
for _, tx := range txs {
// TODO: re-add this on update to v0.8

// TODO: re-add this
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to be re-added before merging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll open an issue for this, didn't want to do it here

// val, err := s.rt.ValidateTransaction(tx.Extrinsic)
// if err != nil {
// // failed to validate tx, remove it from the pool or queue
// s.transactionState.RemoveExtrinsic(ext)
// s.transactionState.RemoveExtrinsic(tx.Extrinsic)
// continue
// }

Expand Down
8 changes: 7 additions & 1 deletion dot/network/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,13 @@ func (q *syncQueue) benchmark() {
continue
}

logger.Info("💤 node waiting", "peer count", len(q.s.host.peers()), "head", before.Number, "finalised", finalised.Number)
logger.Info("💤 node waiting",
"peer count", len(q.s.host.peers()),
"head", before.Number,
"hash", before.Hash(),
"finalised", finalised.Number,
"hash", finalised.Hash(),
)
time.Sleep(time.Second * 5)
continue
}
Expand Down
9 changes: 4 additions & 5 deletions dot/state/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,13 @@ func (s *Service) Stop() error {
return err
}

s.Storage.lock.RLock()
t := s.Storage.tries[head]
s.Storage.lock.RUnlock()

if t == nil {
st, has := s.Storage.tries.Load(head)
if !has {
return errTrieDoesNotExist(head)
}

t := st.(*trie.Trie)

if err = s.Base.StoreLatestStorageHash(head); err != nil {
return err
}
Expand Down
6 changes: 2 additions & 4 deletions dot/state/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,8 @@ func TestService_PruneStorage(t *testing.T) {
time.Sleep(1 * time.Second)

for _, v := range prunedArr {
serv.Storage.lock.Lock()
_, ok := serv.Storage.tries[v.hash]
serv.Storage.lock.Unlock()
require.Equal(t, false, ok)
_, has := serv.Storage.tries.Load(v.hash)
require.Equal(t, false, has)
}
}

Expand Down
200 changes: 78 additions & 122 deletions dot/state/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ func errTrieDoesNotExist(hash common.Hash) error {
// StorageState is the struct that holds the trie, db and lock
type StorageState struct {
blockState *BlockState
tries map[common.Hash]*trie.Trie // map of root -> trie
tries *sync.Map // map[common.Hash]*trie.Trie // map of root -> trie

db chaindb.Database
lock sync.RWMutex
db chaindb.Database
sync.RWMutex

// change notifiers
changedLock sync.RWMutex
Expand All @@ -66,8 +66,8 @@ func NewStorageState(db chaindb.Database, blockState *BlockState, t *trie.Trie,
return nil, fmt.Errorf("cannot have nil trie")
}

tries := make(map[common.Hash]*trie.Trie)
tries[t.MustHash()] = t
tries := new(sync.Map)
tries.Store(t.MustHash(), t)

storageTable := chaindb.NewTable(db, storagePrefix)

Expand Down Expand Up @@ -97,25 +97,23 @@ func (s *StorageState) SetSyncing(syncing bool) {
}

func (s *StorageState) pruneKey(keyHeader *types.Header) {
s.lock.Lock()
defer s.lock.Unlock()

delete(s.tries, keyHeader.StateRoot)
s.tries.Delete(keyHeader.StateRoot)
}

// StoreTrie stores the given trie in the StorageState and writes it to the database
func (s *StorageState) StoreTrie(ts *rtstorage.TrieState, header *types.Header) error {
s.lock.Lock()
defer s.lock.Unlock()

root := ts.MustRoot()

if s.syncing {
// keep only the trie at the head of the chain when syncing
for key := range s.tries {
delete(s.tries, key)
}
// TODO: probably remove this when memory usage improves
s.tries.Range(func(k, _ interface{}) bool {
s.tries.Delete(k)
return true
})
}
s.tries[root] = ts.Trie()

_, _ = s.tries.LoadOrStore(root, ts.Trie())

if _, ok := s.pruner.(*pruner.FullNode); header == nil && ok {
return fmt.Errorf("block cannot be empty for Full node pruner")
Expand All @@ -136,7 +134,7 @@ func (s *StorageState) StoreTrie(ts *rtstorage.TrieState, header *types.Header)

logger.Trace("cached trie in storage state", "root", root)

if err := s.tries[root].WriteDirty(s.db); err != nil {
if err := ts.Trie().WriteDirty(s.db); err != nil {
logger.Warn("failed to write trie to database", "root", root, "error", err)
return err
}
Expand All @@ -156,20 +154,21 @@ func (s *StorageState) TrieState(root *common.Hash) (*rtstorage.TrieState, error
root = &sr
}

s.lock.RLock()
t := s.tries[*root]
s.lock.RUnlock()

if t != nil && t.MustHash() != *root {
panic("trie does not have expected root")
}

if t == nil {
st, has := s.tries.Load(*root)
if !has {
var err error
t, err = s.LoadFromDB(*root)
st, err = s.LoadFromDB(*root)
if err != nil {
return nil, err
}

_, _ = s.tries.LoadOrStore(*root, st)
}

t := st.(*trie.Trie)

if has && t.MustHash() != *root {
panic("trie does not have expected root")
}

nextTrie := t.Snapshot()
Expand All @@ -178,10 +177,20 @@ func (s *StorageState) TrieState(root *common.Hash) (*rtstorage.TrieState, error
return nil, err
}

logger.Trace("returning trie to be modified", "root", root, "next", next.MustRoot())
logger.Trace("returning trie to be modified", "root", root)
return next, nil
}

// BeginModifyTrie should be called before the trie is modified
func (s *StorageState) BeginModifyTrie() {
s.Lock()
}

// FinishModifyTrie should be called after the trie is done being modified
func (s *StorageState) FinishModifyTrie() {
s.Unlock()
}

// LoadFromDB loads an encoded trie from the DB where the key is `root`
func (s *StorageState) LoadFromDB(root common.Hash) (*trie.Trie, error) {
t := trie.NewEmptyTrie()
Expand All @@ -190,13 +199,31 @@ func (s *StorageState) LoadFromDB(root common.Hash) (*trie.Trie, error) {
return nil, err
}

s.lock.Lock()
defer s.lock.Unlock()

s.tries[t.MustHash()] = t
_, _ = s.tries.LoadOrStore(t.MustHash(), t)
return t, nil
}

func (s *StorageState) loadTrie(root *common.Hash) (*trie.Trie, error) {
if root == nil {
sr, err := s.blockState.BestBlockStateRoot()
if err != nil {
return nil, err
}
root = &sr
}

if t, has := s.tries.Load(*root); !has && t != nil {
Copy link
Member

Choose a reason for hiding this comment

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

should this be has && t != nil not !has?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, updated!

return t.(*trie.Trie), nil
}

tr, err := s.LoadFromDB(*root)
if err != nil {
return nil, errTrieDoesNotExist(*root)
}

return tr, nil
}

// ExistsStorage check if the key exists in the storage trie with the given storage hash
// If no hash is provided, the current chain head is used
func (s *StorageState) ExistsStorage(root *common.Hash, key []byte) (bool, error) {
Expand All @@ -215,11 +242,8 @@ func (s *StorageState) GetStorage(root *common.Hash, key []byte) ([]byte, error)
root = &sr
}

s.lock.RLock()
defer s.lock.RUnlock()

if trie, ok := s.tries[*root]; ok {
val := trie.Get(key)
if t, has := s.tries.Load(*root); has {
val := t.(*trie.Trie).Get(key)
return val, nil
}

Expand Down Expand Up @@ -258,109 +282,41 @@ func (s *StorageState) EnumeratedTrieRoot(values [][]byte) {

// Entries returns Entries from the trie with the given state root
func (s *StorageState) Entries(root *common.Hash) (map[string][]byte, error) {
if root == nil {
head, err := s.blockState.BestBlockStateRoot()
if err != nil {
return nil, err
}
root = &head
}

s.lock.RLock()
tr, ok := s.tries[*root]
s.lock.RUnlock()

if !ok {
var err error
tr, err = s.LoadFromDB(*root)
if err != nil {
return nil, errTrieDoesNotExist(*root)
}
tr, err := s.loadTrie(root)
if err != nil {
return nil, err
}

s.lock.RLock()
defer s.lock.RUnlock()
return tr.Entries(), nil
}

// GetKeysWithPrefix returns all that match the given prefix for the given hash (or best block state root if hash is nil) in lexicographic order
func (s *StorageState) GetKeysWithPrefix(hash *common.Hash, prefix []byte) ([][]byte, error) {
if hash == nil {
sr, err := s.blockState.BestBlockStateRoot()
if err != nil {
return nil, err
}
hash = &sr
}

s.lock.RLock()
tr, ok := s.tries[*hash]
s.lock.RUnlock()

if !ok {
var err error
tr, err = s.LoadFromDB(*hash)
if err != nil {
return nil, errTrieDoesNotExist(*hash)
}
func (s *StorageState) GetKeysWithPrefix(root *common.Hash, prefix []byte) ([][]byte, error) {
tr, err := s.loadTrie(root)
if err != nil {
return nil, err
}

s.lock.RLock()
defer s.lock.RUnlock()
return tr.GetKeysWithPrefix(prefix), nil
}

// GetStorageChild return GetChild from the trie
func (s *StorageState) GetStorageChild(hash *common.Hash, keyToChild []byte) (*trie.Trie, error) {
if hash == nil {
sr, err := s.blockState.BestBlockStateRoot()
if err != nil {
return nil, err
}
hash = &sr
}

s.lock.RLock()
tr, ok := s.tries[*hash]
s.lock.RUnlock()

if !ok {
var err error
tr, err = s.LoadFromDB(*hash)
if err != nil {
return nil, errTrieDoesNotExist(*hash)
}
// GetStorageChild returns a child trie, if it exists
func (s *StorageState) GetStorageChild(root *common.Hash, keyToChild []byte) (*trie.Trie, error) {
tr, err := s.loadTrie(root)
if err != nil {
return nil, err
}

s.lock.RLock()
defer s.lock.RUnlock()
return tr.GetChild(keyToChild)
}

// GetStorageFromChild return GetFromChild from the trie
func (s *StorageState) GetStorageFromChild(hash *common.Hash, keyToChild, key []byte) ([]byte, error) {
if hash == nil {
sr, err := s.blockState.BestBlockStateRoot()
if err != nil {
return nil, err
}
hash = &sr
}

s.lock.RLock()
tr, ok := s.tries[*hash]
s.lock.RUnlock()

if !ok {
var err error
tr, err = s.LoadFromDB(*hash)
if err != nil {
return nil, errTrieDoesNotExist(*hash)
}
// GetStorageFromChild get a value from a child trie
func (s *StorageState) GetStorageFromChild(root *common.Hash, keyToChild, key []byte) ([]byte, error) {
tr, err := s.loadTrie(root)
if err != nil {
return nil, err
}

s.lock.RLock()
defer s.lock.RUnlock()
return tr.GetFromChild(keyToChild, key)
}

Expand Down
Loading