Skip to content

[R4R] BEP-18: for state sync enhancement#18

Merged
darren-liu merged 4 commits intomasterfrom
statesync
Jun 28, 2019
Merged

[R4R] BEP-18: for state sync enhancement#18
darren-liu merged 4 commits intomasterfrom
statesync

Conversation

@cryptom-dev
Copy link
Contributor

@cryptom-dev cryptom-dev commented Jun 14, 2019

Background

State sync is a way to help newly-joined user sync latest status of binance chain. It sync latest syncable peer's status so that fullnode user (who want catch up with chain as soon as possible) doesn't need sync from block height 0 (with cost that discard all historical blocks locally).

BEP-18 Proposal describes an enhancement of existing state sync implementation to improve user experience. It introduces the following details:

  • What's the procedure to take a snapshot
  • What's the procedure to sync snapshot from other peers
  • Snapshot (manifest, snapshot chunks) format

Motivation

We propose this BEP to enhance full node user experience (and ease their paining) on using state sync because of following implementation limitation.

  1. Users complain most about state syncing testnet is very slow and usually stucking on some request.

    In this enhancement, we want data responsed more evenly across peers so that syncing can continously making progress and the overall syncing time can reduce from 30 - 45 min to around 5 min.

  2. Interruption during state sync (node process get killed because of reboot computer or user inpatient) would make already synced state in vain (because current full node doesn't persist synced part on dist) more worse it mistakenly write a lockfile prevent user state sync again.

    In this enhancement, we want support break-resume downloading and keep consistent status for arbitrarily restart.

  3. User cannot control which height they can sync from. Current implementation only allows user sync latest syncable height.

    In this enhancement, we want make syncing from historical syncable height possible. User either specify a height to sync in config file or follow latest syncable height.

@cryptom-dev cryptom-dev self-assigned this Jun 14, 2019
@cryptom-dev cryptom-dev changed the title BEP for state sync enhancement [R4R] BEP for state sync enhancement Jun 14, 2019
@cryptom-dev cryptom-dev changed the title [R4R] BEP for state sync enhancement [R4R] BEP18 for state sync enhancement Jun 16, 2019
@cryptom-dev cryptom-dev changed the title [R4R] BEP18 for state sync enhancement [R4R] BEP-18: for state sync enhancement Jun 16, 2019
chainwhisper
chainwhisper previously approved these changes Jun 24, 2019
@cryptom-dev cryptom-dev force-pushed the statesync branch 2 times, most recently from aa1b1c0 to 84644b0 Compare June 25, 2019 06:58
Copy link

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

BEP18.md Outdated

If the snapshot taking procedure is interrupted, the node will be still in good status, but it cannot provide the interrupted height for other peers.

Note: automatically take snapshot would keep occupying disk space. Fullnode would not delete them automatically, so the user should periodically delete unneeded snapshots manually if they want to save disk space.
Copy link

Choose a reason for hiding this comment

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

What prevents you from deleting them automatically?

Copy link
Contributor Author

@cryptom-dev cryptom-dev Jun 28, 2019

Choose a reason for hiding this comment

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

What prevents you from deleting them automatically?

The frequency of snapshot taken is once per day for binance chain. And in our testnet, it takes 300M per snapshot and should be around just 30M at production considered the account numbers are ten times less than testnet there. Whole year less than 15GB storage cost is not a concern for now.


Stop and restart fullnode during state sync is allowed. The next time full node is started, it will resume by loading Manifest and downloaded chunks then download missing chunks.

Once state sync is successful, a `STATESYNC.LOCK` file will be created under `$HOME/data` to prevent state sync next time.
Copy link

Choose a reason for hiding this comment

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

Does this mean node can only sync once?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this mean node can only sync once?

nodes are supposed to only sync once. But as I wrote in this BEP, they can delete this file manually and trigger state sync again if they are too many days lagged. (but their blockchain.db is not continuous in this case)


Manifest serves as a summary of snapshot chunks to be synced. It also maintains the order and types of snapshot chunks. Fullnode firstly asks peer's for the manifest file at the beginning of state sync and will trust majority peers with the same manifest.

SHA256 hash sum of each chunk synced will be checked against the hash declared within the manifest file.
Copy link

Choose a reason for hiding this comment

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

Does it make sense to hash the manifest file itself and give the user ability to provide that hash when it wants to sync to a specific height?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it make sense to hash the manifest file itself and give the user the ability to provide that hash when it wants to sync to a specific height?

actually, the user can set state_sync_height in config.tomlto the snapshot height other peers have. But the height number is not fixed for every day, we are considering how to let node knowns what snapshot heights I can provide.

The height is much simpler than the hash of manifest right?

Copy link

Choose a reason for hiding this comment

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

Yeah, but height + hash might give the user additional insurance (like when the nodes the user trust publish height + height of the snapshot and the user uses them to sync the state)

BEP18.md Outdated
| StateHashes | []SHA256Sum | hashes of tendermint state chunks |
| AppStateHashes | []SHA256Sum | hashes of app state chunks |
| BlockHashes | []SHA256Sum | hashes of the block in this snapshot, currently only one block is synced |
| NumKeys | []int64 | number of keys for each sub-store, this sacrifices clear boundary between cosmos and tendermint, saying tendermint knows application DB might be organized by sub-stores.<br><br> |
Copy link

Choose a reason for hiding this comment

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

What if the order or number of sub-stores changes? Or this is controlled by Version (snapshot version)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if the order or number of sub-stores changes? Or this is controlled by Version (snapshot version)?

State sync will accept majority peer's manifest. Saying if the order or length of NumKeys of manifest in some nodes is inconsistent with the majority node (maybe they forget to upgrade their binary whereas others changed the order or num of sub stores), the new joined node won't sync from it (accept his manifest file).

BEP18.md Outdated
| Field | Type | Description |
|--------------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| StartIdx | int64 | compare (startIdx and number of complete nodes) against (Manifest.NumKeys) we can know each node's sub-store |
| Completeness | uint8 | flag of completeness of this chunk, not enum because of go-amino doesn't support enum encoding |
Copy link

Choose a reason for hiding this comment

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

can't this be a bool?

Copy link
Contributor Author

@cryptom-dev cryptom-dev Jun 28, 2019

Choose a reason for hiding this comment

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

can't this be a bool?

It has 4 kinds of value: 0 complete, 1 - incomplete start, 2 - incomplete middle, 3 - incomplete end.

I have left a more detailed explanation on StartIdx field, you can read that:)

Copy link
Contributor

Choose a reason for hiding this comment

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

please add these values into the explanation.

@darren-liu darren-liu merged commit 551abff into master Jun 28, 2019
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.

4 participants