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

core/state/snapshot: avoid thrashing during block import #23728

Closed
wants to merge 1 commit into from

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Oct 13, 2021

The code in this PR is written by @karalabe (I just moved a chunk of code a bit north)
When we're generating the snapshot, we're also processing blocks. In case we're importing multiple blocks (catching up), this adds a penalty, since there are background processes which are reading a lot from disk, all the while we're trying to catch up.

This PR instead makes it so that if the previous generation-effort ended within 450ms, then it won't start anew.

@holiman
Copy link
Contributor Author

holiman commented Oct 13, 2021

TODO: I haven't tested it on an actual sync, will try that in the near future.

@karalabe
Copy link
Member

This is not enough though. That's why I didn't open it as a PR. Currently generation runs during block processing too, not just in between, that's the primary issue. Until that's fixed, generation - especially with cold caches on startup - will cause block processing to take seconds, which makes this check a noop.

@holiman
Copy link
Contributor Author

holiman commented Oct 13, 2021

A naive but straight-forward approach:

diff --git a/core/blockchain.go b/core/blockchain.go
index 00f90415c1..bf632ae32e 100644
--- a/core/blockchain.go
+++ b/core/blockchain.go
@@ -1772,6 +1772,8 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er
 			activeState.StopPrefetcher()
 		}
 	}()
+	// Pause the snapshot generator, if it's working
+	bc.snaps.PauseGenerator()
 
 	for ; block != nil && err == nil || err == ErrKnownBlock; block, err = it.next() {
 		// If the chain is terminating, stop processing blocks
diff --git a/core/state/snapshot/snapshot.go b/core/state/snapshot/snapshot.go
index 8889fa9fec..25eab2c70a 100644
--- a/core/state/snapshot/snapshot.go
+++ b/core/state/snapshot/snapshot.go
@@ -233,6 +233,17 @@ func (t *Tree) waitBuild() {
 		<-done
 	}
 }
+func (t *Tree) PauseGenerator() {
+	t.lock.Lock()
+	defer t.lock.Unlock()
+	layer := t.disklayer()
+	// If the base layer is generating, abort it
+	if layer.genAbort != nil {
+		abort := make(chan *generatorStats)
+		layer.genAbort <- abort
+		<-abort
+	}
+}
 
 // Disable interrupts any pending snapshot generator, deletes all the snapshot
 // layers in memory and marks snapshots disabled globally. In order to resume

Testing it a bit now, not sure if I'm missing something

@Cr4shOv3rrid3
Copy link
Contributor

Cr4shOv3rrid3 commented Jul 30, 2022

can someone plz fix this. i am having issues because of that with a virtual private server setup that is not allowing me to make a new snapshot. seem like it also affects the bitcoin course price already. those pull requests in both networks always synch into latest course price. at least we get a roof that finally makes sense now for bitcoin tech for price speculations (no.of latest pull request). guess this makes the price of 1 ETH (this one included and herby hardcoded into this comment). round about a net income on a monthly basis, from an average user for this tech (what would also make the most sense and would put bitcoin into net income for lowest connected people per year as i think the savings aspect bitcoin is the better choice cause ethereum wasnt build by savings people. and savings commonly are APY based.)

besides of this i would ACK the idea for the fix that holiman already proposed.
switching vom a into b chain as well sounds reasonable (bitcoin consensus coded).

change my comment or delete it according to your needs if you want. i am not a core developer but i am working with the code of both networks a lot. so i just wanna help this tech to run more smoothly and for what it originally came to life.

@holiman holiman closed this Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants