Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Naive chunk creation, snapshotting #1263

Merged
merged 25 commits into from
Jun 14, 2016
Merged

Naive chunk creation, snapshotting #1263

merged 25 commits into from
Jun 14, 2016

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jun 13, 2016

This cannot be merged into master yet. It requires a DB upgrade/migration. Work will continue in the pv64 branch here, as well as my local pv64 branch. Opening the code to review so we can discuss design improvements, issues, and the way forward.

@rphmeier rphmeier added the A0-pleasereview 🤓 Pull request needs code review. label Jun 13, 2016
@arkpar arkpar added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Jun 13, 2016
@rphmeier
Copy link
Contributor Author

this specific PR is not in-progress. to avoid change size getting out of hand it is better to split it up into smaller, more easily reviewable chunks IMO.

@rphmeier rphmeier removed the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Jun 13, 2016

// lock the state db while we create the state chunks.
let state_hashes = {
let state_db = self.state_db.lock().unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

StateDB should not be locked for the whole duration, use a clone like this:

    let db = self.state_db.lock().unwrap().boxed_clone();

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 had been under the impression that it was necessary for a proper snapshot. our conversation has mitigated these worries.

@arkpar
Copy link
Collaborator

arkpar commented Jun 13, 2016

I'd avoid "pv64" name in the code, this is just a protocol version that can change.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 14, 2016
@rphmeier rphmeier merged commit 06c6844 into openethereum:pv64 Jun 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants