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

bring snapshotting work into master #1577

Merged
merged 87 commits into from
Jul 12, 2016
Merged

bring snapshotting work into master #1577

merged 87 commits into from
Jul 12, 2016

Conversation

rphmeier
Copy link
Contributor

No description provided.

rphmeier added 30 commits June 10, 2016 12:19
@rphmeier
take_snapshot and block chunking skeleton
5e0ba1c
@rphmeier
finish implementing basic block chunking
997fd93
@rphmeier
take_snapshot now chunks blocks
f12add6
@rphmeier
fix warning
c6e83ca
@rphmeier
added tracing
f689792
@rphmeier
removed size tolerance
b047eb2
@rphmeier
add manifestdata struct, write manifest file at the end
3b1cca5
@rphmeier
output manifest to correct file
5c252f2
@rphmeier
skeleton for StateChunker
c9846b8
@rphmeier
add account reader. trie walking not functional yet
d696a66
@rphmeier
[REQUIRES DB UPGRADE] have account_db use address hash
e5ca5e0
@rphmeier
AccountReader -> to_fat_rlp implementation
d9743f1
@rphmeier
state chunk creation tracing
f400da4
@rphmeier
create an AccountDB to extract account storage
10c2302
@rphmeier
chunk remainder at end of each stage
4ded1cc
@rphmeier
remove spammy account trace
f478812
@rphmeier
io error handling and slight blockchunker refactoring
446d594
@rphmeier
expose only chunk_blocks and chunk_state APIs
98c7677
@rphmeier
Merge with latest master
6ec282f
@rphmeier
rename pv64 module to snapshot
22a1981
@rphmeier
rename pv64_snapshot trace target to snapshot
c36f47b
@rphmeier
no longer lock the state db
5b6ea41
@rphmeier
push pair onto block chunker buffer
5cc6a68
@rphmeier
store account code if it exists
835b158
@rphmeier
use append_empty_data rather than fibbing it
b88eef5
@rphmeier
simple snappy bindings, enabling alloc free code
7501300
@rphmeier
use snappy compression on buffers
16e5895
@rphmeier
compress into reusable buffers
dfb603d
@rphmeier
refactor out common parts of write_chunk
8ed2b9c
@rphmeier
add some missing header setters
874bc80
@rphmeier rphmeier added the A0-pleasereview 🤓 Pull request needs code review. label Jul 11, 2016
@gavofyork
Copy link
Contributor

some tests would be nice...

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 11, 2016
@NikVolf
Copy link
Contributor

NikVolf commented Jul 11, 2016 •
edited
Loading

@gavofyork +1
guess lack of test is an issue, not a grumble
it will prevents further comfortable modification of the code

@NikVolf NikVolf added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jul 11, 2016
arkpar
arkpar reviewed Jul 11, 2016
View reviewed changes
ethcore/src/snapshot/block.rs
}

// write block values.
stream.append(&block_view.transactions()).append(&block_view.uncles());
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be more optimal to append rlps directly, without converting to transaction/header structure. transactions_rlp and uncles_rlp can be added to BlockView

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we skip a couple of the fields from the header, i think we have to at least decode that.

block chunking is significantly faster than state chunking even so

NikVolf
NikVolf reviewed Jul 11, 2016
View reviewed changes
ethcore/src/snapshot/mod.rs
let compressed = &compression_buffer[..compressed_size];
let hash = compressed.sha3();

assert!(snappy::validate_compressed_buffer(compressed));
Copy link
Contributor

Choose a reason for hiding this comment

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

can the error be returned instead of panic happening with the assert?

Copy link
Contributor Author

@rphmeier rphmeier Jul 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

this assert should be removed outright actually. i added it while debugging but it will only fail if snappy has a bug

@rphmeier
Copy link
Contributor Author

I added account and block tests.

I will add tests for the overall snapshotting mechanism once I implement snapshot restoration. Right now there isn't anything I can see to meaningfully test in that module.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Jul 11, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 12, 2016
@NikVolf NikVolf merged commit d956b7c into master Jul 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rphmeier @gavofyork @NikVolf @arkpar