Skip to content

Commit d718312

Browse files
authoredSep 6, 2024
core/state/snapshot: port changes from 29995 (ethereum#30040)
ethereum#29995 has been reverted due to an unexpected flaw in the state snapshot process. Specifically, it attempts to stop the state snapshot generation, which could potentially cause the system to halt if the generation is not currently running. This pull request ports the changes made in ethereum#29995 and fixes the flaw.
1 parent 88c8459 commit d718312

File tree

3 files changed

+34
-36
lines changed

3 files changed

+34
-36
lines changed
 

‎core/state/snapshot/disklayer.go

+23
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ func (dl *diskLayer) Stale() bool {
7474
return dl.stale
7575
}
7676

77+
// markStale sets the stale flag as true.
78+
func (dl *diskLayer) markStale() {
79+
dl.lock.Lock()
80+
defer dl.lock.Unlock()
81+
82+
dl.stale = true
83+
}
84+
7785
// Account directly retrieves the account associated with a particular hash in
7886
// the snapshot slim data format.
7987
func (dl *diskLayer) Account(hash common.Hash) (*types.SlimAccount, error) {
@@ -175,3 +183,18 @@ func (dl *diskLayer) Storage(accountHash, storageHash common.Hash) ([]byte, erro
175183
func (dl *diskLayer) Update(blockHash common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer {
176184
return newDiffLayer(dl, blockHash, destructs, accounts, storage)
177185
}
186+
187+
// stopGeneration aborts the state snapshot generation if it is currently running.
188+
func (dl *diskLayer) stopGeneration() {
189+
dl.lock.RLock()
190+
generating := dl.genMarker != nil
191+
dl.lock.RUnlock()
192+
if !generating {
193+
return
194+
}
195+
if dl.genAbort != nil {
196+
abort := make(chan *generatorStats)
197+
dl.genAbort <- abort
198+
<-abort
199+
}
200+
}

‎core/state/snapshot/generate.go

+1-8
Original file line numberDiff line numberDiff line change
@@ -631,16 +631,10 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er
631631
accMarker = nil
632632
return nil
633633
}
634-
// Always reset the initial account range as 1 whenever recover from the
635-
// interruption. TODO(rjl493456442) can we remove it?
636-
var accountRange = accountCheckRange
637-
if len(accMarker) > 0 {
638-
accountRange = 1
639-
}
640634
origin := common.CopyBytes(accMarker)
641635
for {
642636
id := trie.StateTrieID(dl.root)
643-
exhausted, last, err := dl.generateRange(ctx, id, rawdb.SnapshotAccountPrefix, snapAccount, origin, accountRange, onAccount, types.FullAccountRLP)
637+
exhausted, last, err := dl.generateRange(ctx, id, rawdb.SnapshotAccountPrefix, snapAccount, origin, accountCheckRange, onAccount, types.FullAccountRLP)
644638
if err != nil {
645639
return err // The procedure it aborted, either by external signal or internal error.
646640
}
@@ -652,7 +646,6 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er
652646
ctx.removeStorageLeft()
653647
break
654648
}
655-
accountRange = accountCheckRange
656649
}
657650
return nil
658651
}

‎core/state/snapshot/snapshot.go

+10-28
Original file line numberDiff line numberDiff line change
@@ -258,24 +258,11 @@ func (t *Tree) Disable() {
258258
for _, layer := range t.layers {
259259
switch layer := layer.(type) {
260260
case *diskLayer:
261-
262-
layer.lock.RLock()
263-
generating := layer.genMarker != nil
264-
layer.lock.RUnlock()
265-
if !generating {
266-
// Generator is already aborted or finished
267-
break
268-
}
269-
// If the base layer is generating, abort it
270-
if layer.genAbort != nil {
271-
abort := make(chan *generatorStats)
272-
layer.genAbort <- abort
273-
<-abort
274-
}
275-
// Layer should be inactive now, mark it as stale
276-
layer.lock.Lock()
277-
layer.stale = true
278-
layer.lock.Unlock()
261+
// TODO this function will hang if it's called twice. Will
262+
// fix it in the following PRs.
263+
layer.stopGeneration()
264+
layer.markStale()
265+
layer.Release()
279266

280267
case *diffLayer:
281268
// If the layer is a simple diff, simply mark as stale
@@ -730,16 +717,11 @@ func (t *Tree) Rebuild(root common.Hash) {
730717
for _, layer := range t.layers {
731718
switch layer := layer.(type) {
732719
case *diskLayer:
733-
// If the base layer is generating, abort it and save
734-
if layer.genAbort != nil {
735-
abort := make(chan *generatorStats)
736-
layer.genAbort <- abort
737-
<-abort
738-
}
739-
// Layer should be inactive now, mark it as stale
740-
layer.lock.Lock()
741-
layer.stale = true
742-
layer.lock.Unlock()
720+
// TODO this function will hang if it's called twice. Will
721+
// fix it in the following PRs.
722+
layer.stopGeneration()
723+
layer.markStale()
724+
layer.Release()
743725

744726
case *diffLayer:
745727
// If the layer is a simple diff, simply mark as stale

0 commit comments

Comments
 (0)
Please sign in to comment.