From ea161996859eca15f41de8d8cd91375bcbb01b07 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 12 Apr 2022 09:42:22 +0200 Subject: [PATCH 1/6] core/state/snapshot: fix race condition --- core/state/snapshot/difflayer.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/state/snapshot/difflayer.go b/core/state/snapshot/difflayer.go index ee88938b774d..4a8c24f999d9 100644 --- a/core/state/snapshot/difflayer.go +++ b/core/state/snapshot/difflayer.go @@ -258,6 +258,8 @@ func (dl *diffLayer) Root() common.Hash { // Parent returns the subsequent layer of a diff layer. func (dl *diffLayer) Parent() snapshot { + dl.lock.RLock() + defer dl.lock.RUnlock() return dl.parent } From 8aaca0cf4fc489920e4df75e3718320a3c7caaa1 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 12 Apr 2022 09:52:12 +0200 Subject: [PATCH 2/6] eth/protocols/snap: fix race condition --- eth/protocols/snap/sync.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index 665d7601cfe2..1c88543c6f98 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -705,7 +705,9 @@ func (s *Syncer) loadSyncStatus() { } } } + s.lock.Lock() s.snapped = len(s.tasks) == 0 + s.lock.Unlock() s.accountSynced = progress.AccountSynced s.accountBytes = progress.AccountBytes From 16d8d70f2dea96d3a776f329e348fb1d7e9b6dd4 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 12 Apr 2022 10:01:56 +0200 Subject: [PATCH 3/6] core/state/snapshot: f whitespace --- core/state/snapshot/difflayer.go | 1 + 1 file changed, 1 insertion(+) diff --git a/core/state/snapshot/difflayer.go b/core/state/snapshot/difflayer.go index 4a8c24f999d9..bc57ab0af347 100644 --- a/core/state/snapshot/difflayer.go +++ b/core/state/snapshot/difflayer.go @@ -260,6 +260,7 @@ func (dl *diffLayer) Root() common.Hash { func (dl *diffLayer) Parent() snapshot { dl.lock.RLock() defer dl.lock.RUnlock() + return dl.parent } From 6d4f686856740cb35d56646dcbe11c3c383d87ae Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 13 Apr 2022 17:52:14 +0200 Subject: [PATCH 4/6] eth/protocols/snap: fix race condition (properly) --- eth/protocols/snap/sync.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index 1c88543c6f98..ee2f797d584e 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -706,8 +706,9 @@ func (s *Syncer) loadSyncStatus() { } } s.lock.Lock() + defer s.lock.Unlock() + s.snapped = len(s.tasks) == 0 - s.lock.Unlock() s.accountSynced = progress.AccountSynced s.accountBytes = progress.AccountBytes From da5dc9c74020c4ddecfe989fbc15f201561bb5ec Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 27 Apr 2022 21:19:06 +0200 Subject: [PATCH 5/6] eth/protocols/snap: properly fix race on s.Progress --- eth/protocols/snap/sync.go | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index ee2f797d584e..2f69f5feabbe 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -422,6 +422,8 @@ type Syncer struct { storageSynced uint64 // Number of storage slots downloaded storageBytes common.StorageSize // Number of storage trie bytes persisted to disk + extProgress *SyncProgress // progress that can be exposed to external caller. + // Request tracking during healing phase trienodeHealIdlers map[string]struct{} // Peers that aren't serving trie node requests bytecodeHealIdlers map[string]struct{} // Peers that aren't serving bytecode requests @@ -477,6 +479,8 @@ func NewSyncer(db ethdb.KeyValueStore) *Syncer { trienodeHealReqs: make(map[uint64]*trienodeHealRequest), bytecodeHealReqs: make(map[uint64]*bytecodeHealRequest), stateWriter: db.NewBatch(), + + extProgress: new(SyncProgress), } } @@ -633,6 +637,21 @@ func (s *Syncer) Sync(root common.Hash, cancel chan struct{}) error { s.assignTrienodeHealTasks(trienodeHealResps, trienodeHealReqFails, cancel) s.assignBytecodeHealTasks(bytecodeHealResps, bytecodeHealReqFails, cancel) } + p := &SyncProgress{ + AccountSynced: s.accountSynced, + AccountBytes: s.accountBytes, + BytecodeSynced: s.bytecodeSynced, + BytecodeBytes: s.bytecodeBytes, + StorageSynced: s.storageSynced, + StorageBytes: s.storageBytes, + TrienodeHealSynced: s.trienodeHealSynced, + TrienodeHealBytes: s.trienodeHealBytes, + BytecodeHealSynced: s.bytecodeHealSynced, + BytecodeHealBytes: s.bytecodeHealBytes, + } + s.lock.Lock() + s.extProgress = p + s.lock.Unlock() // Wait for something to happen select { case <-s.update: @@ -805,19 +824,7 @@ func (s *Syncer) saveSyncStatus() { func (s *Syncer) Progress() (*SyncProgress, *SyncPending) { s.lock.Lock() defer s.lock.Unlock() - - progress := &SyncProgress{ - AccountSynced: s.accountSynced, - AccountBytes: s.accountBytes, - BytecodeSynced: s.bytecodeSynced, - BytecodeBytes: s.bytecodeBytes, - StorageSynced: s.storageSynced, - StorageBytes: s.storageBytes, - TrienodeHealSynced: s.trienodeHealSynced, - TrienodeHealBytes: s.trienodeHealBytes, - BytecodeHealSynced: s.bytecodeHealSynced, - BytecodeHealBytes: s.bytecodeHealBytes, - } + progress := s.extProgress pending := new(SyncPending) if s.healer != nil { pending.TrienodeHeal = uint64(len(s.healer.trieTasks)) From 08f4188cf7f61a48fd96b441b393258f89fcf206 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 5 May 2022 15:19:03 +0200 Subject: [PATCH 6/6] eth/protocols/snap: nits --- eth/protocols/snap/sync.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index 2f69f5feabbe..5bbf5ee48214 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -637,7 +637,9 @@ func (s *Syncer) Sync(root common.Hash, cancel chan struct{}) error { s.assignTrienodeHealTasks(trienodeHealResps, trienodeHealReqFails, cancel) s.assignBytecodeHealTasks(bytecodeHealResps, bytecodeHealReqFails, cancel) } - p := &SyncProgress{ + // Update sync progress + s.lock.Lock() + s.extProgress = &SyncProgress{ AccountSynced: s.accountSynced, AccountBytes: s.accountBytes, BytecodeSynced: s.bytecodeSynced, @@ -649,8 +651,6 @@ func (s *Syncer) Sync(root common.Hash, cancel chan struct{}) error { BytecodeHealSynced: s.bytecodeHealSynced, BytecodeHealBytes: s.bytecodeHealBytes, } - s.lock.Lock() - s.extProgress = p s.lock.Unlock() // Wait for something to happen select { @@ -824,13 +824,12 @@ func (s *Syncer) saveSyncStatus() { func (s *Syncer) Progress() (*SyncProgress, *SyncPending) { s.lock.Lock() defer s.lock.Unlock() - progress := s.extProgress pending := new(SyncPending) if s.healer != nil { pending.TrienodeHeal = uint64(len(s.healer.trieTasks)) pending.BytecodeHeal = uint64(len(s.healer.codeTasks)) } - return progress, pending + return s.extProgress, pending } // cleanAccountTasks removes account range retrieval tasks that have already been