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

Extract confirmation height to its own database #2174

Merged

Conversation

wezrule
Copy link
Contributor

@wezrule wezrule commented Jul 26, 2019

While logically connected to an account_info the confirmation height is updated separately in a different thread. This means a lock is required when using something like RocksDB because there can be write conflicts when writing to the same account. As account_put is only called as part of the block_processor thread and confirmation height for existing accounts is done in the confirmation height processor thread (and new accounts in the block processor thread) no external synchronization should be required if they are separated. This won't help with lmdb yet because it uses a single global write lock, it would require it to be moved to a completely separate store file. This wasn't done (as moving it to a new database was painful enough), but this would help with other backends which don't use a global write lock (e.g rocksdb) and also enable it to be moved to a new lmdb store more easily if required.

It's very important to keep the accounts in accounts_v0/_v1 in sync with the confirmation_height database. There is a debug assert in account_put which has a precondition that the account exists in the confirmation_height database. No such check exists in account_del because it may just be removing from accounts_v0 to put in accounts_v1 and so makes sense to keep the confirmation_height entry and not impose a precondition there.

@wezrule wezrule added blocker Some future items cannot be completed until this is merged. database Relates to lmdb or rocksdb labels Jul 26, 2019
@wezrule wezrule added this to the V20.0 milestone Jul 26, 2019
@wezrule wezrule self-assigned this Jul 26, 2019
@zhyatt zhyatt requested a review from cryptocode July 29, 2019 21:36
@zhyatt zhyatt removed the request for review from SergiySW August 1, 2019 14:12
@wezrule wezrule merged commit 386d7cc into nanocurrency:master Aug 1, 2019
@wezrule wezrule deleted the move_conf_height_to_new_database branch August 1, 2019 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Some future items cannot be completed until this is merged. database Relates to lmdb or rocksdb
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants