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

refactor(storage): use flatbuffer instead of bincode in storage #1032

Merged
merged 7 commits into from
Jun 25, 2019

Conversation

chaoticlonghair
Copy link
Contributor

@chaoticlonghair chaoticlonghair commented Jun 15, 2019

Associated Issues

Description

  • I use flatbuffer, not cfb. We can do it later.

  • I copy some code from ckb-protocol, not just use it, because:

    • Since the data is from local storage, we can ensure the data format shouldn't be malformed.

      Do expect(..) is safety for storage, but not safety for network.

    • I try to split those proto-types into different schemas, classify them according to their usages.

    • If required, we can rewrite proto-types for storage, and don't have to modify proto-types for network.

    • I rewrite some of them.

  • I just remove bincode from ckb-store, but I keep the bincode for computing hashes. We can do it later.

  • I can't remove serde from ckb-core, because:

    I think we should use jsonrpc-types for importing and exporting jsons and use Display for log outputs.

    Then we can remove serde from ckb-core.

BREAKING CHANGE

The data format in storage is changed.

@chaoticlonghair chaoticlonghair requested a review from a team June 15, 2019 20:00
@nervos-bot
Copy link

nervos-bot bot commented Jun 15, 2019

@quake @TheWaWaR @driftluo are assigned as the chief reviewers

@nervos-bot nervos-bot bot added the breaking change The feature breaks consensus, database, message schema or RPC interface. label Jun 15, 2019
@doitian doitian changed the title refactor(storage): use flatbuffer to instead of bincode in storage ✋ refactor(storage): use flatbuffer to instead of bincode in storage Jun 18, 2019
nervos-bot[bot]
nervos-bot bot previously requested changes Jun 18, 2019
Copy link

@nervos-bot nervos-bot bot left a comment

Choose a reason for hiding this comment

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

Hold as requested by @doitian.

@doitian
Copy link
Member

doitian commented Jun 20, 2019

This will not be included in v0.15.0. Hold until 0.15.0 RC.

@doitian doitian added the b:database Break database schema label Jun 20, 2019
@doitian doitian changed the title ✋ refactor(storage): use flatbuffer to instead of bincode in storage refactor(storage): use flatbuffer to instead of bincode in storage Jun 21, 2019
@nervos-bot nervos-bot bot dismissed their stale review June 21, 2019 13:29

Unhold as requested by @doitian.

@chaoticlonghair

This comment has been minimized.

@chaoticlonghair
Copy link
Contributor Author

chaoticlonghair commented Jun 22, 2019

@keroro520:
I force push after rebase this pr from commit-3648f80.
Please help me to compare the performance between commit-3648f80 and commit-de6a5a8.

@chaoticlonghair

This comment has been minimized.

@doitian doitian changed the title refactor(storage): use flatbuffer to instead of bincode in storage refactor(storage): use flatbuffer instead of bincode in storage Jun 22, 2019
@chaoticlonghair chaoticlonghair force-pushed the pr/fb-storage branch 2 times, most recently from 5af7311 to 9b52562 Compare June 22, 2019 11:47
@chaoticlonghair

This comment has been minimized.

@chaoticlonghair chaoticlonghair merged commit 4a724e1 into nervosnetwork:develop Jun 25, 2019
@doitian doitian mentioned this pull request Jul 4, 2019
1 task
@chaoticlonghair chaoticlonghair deleted the pr/fb-storage branch August 30, 2019 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:database Break database schema breaking change The feature breaks consensus, database, message schema or RPC interface.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants