Skip to content

Conversation

@codablock
Copy link

Currently, when Dash crashes, the chainstate DB and CEvoDB are left in an inconsistent way. Recent PRs/commits have added VerifyBestBlock and WriteBestBlock to CEvoDB so that we can at least detect inconsistencies at startup. This however leads to Dash not starting anymore and forcing a reindex upon the user.

This PR introduces 2-stage commits for CEvoDB. First stage commits happen after block connect/disconnect (the same way as before), but instead of directly writing into LevelDB, it will move the transaction contents into a second stage transaction. The second stage transaction is committed at the same time when the chainstate is flushed. This should fix many of the situations where reindex was required before.

The boolean return value will loose its meaning in the next commit
CDBTransaction is changed to allow CDBBatch, CDBWrapper and other
CDBTransactions as parent instead of just CDBWrapper. This in turn allows
to implement multi-staged commits in CEvoDB.

We now have the "current transaction" which is started and ended (commit
or rollback) for each call to Connect-/DisconnectBlock. When the current
transaction is committed, it moves its contents into the "root transaction"
instead of directly writing to CDBWrapper.

CommitRootTransaction() then handles the final commitment to CDBWrapper. It
is called at the same time when the chainstate is flushed to disk, which
guarantees consistency between chainstate and CEvoDB.
…pies

When CDBTransaction<CDBTransaction<...>>::Commit() is called, we can avoid
copying values from this transaction to the parent transaction and instead
pass values by rvalue and let the contents be moved.
@UdjinM6 UdjinM6 added this to the 14.0 milestone Mar 6, 2019
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

LGTM, will test this a bit. I wonder if we can revert #2560 now?

@codablock
Copy link
Author

Yeah I assume #2560 can be safely reverted now. I can do that as part of this PR if you want?

@UdjinM6
Copy link

UdjinM6 commented Mar 6, 2019

I reverted it locally already to see if kill -9 dash-qt while generating 100s blocks in regtest would cause evodb inconsistency and all seems to be working just fine. Let's do it :)

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

👍

ACK

@UdjinM6 UdjinM6 merged commit 521d4ae into dashpay:develop Mar 6, 2019
@codablock codablock deleted the pr_2stagecommit branch March 7, 2019 05:52
codablock added a commit to codablock/dash that referenced this pull request Mar 7, 2019
…ashes (dashpay#2744)

* Let Commit() return void

The boolean return value will loose its meaning in the next commit

* Implement 2-stage commits for CDBTransaction and CScopedDBTransaction

CDBTransaction is changed to allow CDBBatch, CDBWrapper and other
CDBTransactions as parent instead of just CDBWrapper. This in turn allows
to implement multi-staged commits in CEvoDB.

We now have the "current transaction" which is started and ended (commit
or rollback) for each call to Connect-/DisconnectBlock. When the current
transaction is committed, it moves its contents into the "root transaction"
instead of directly writing to CDBWrapper.

CommitRootTransaction() then handles the final commitment to CDBWrapper. It
is called at the same time when the chainstate is flushed to disk, which
guarantees consistency between chainstate and CEvoDB.

* Allow to efficiently move values into parent transactions to avoid copies

When CDBTransaction<CDBTransaction<...>>::Commit() is called, we can avoid
copying values from this transaction to the parent transaction and instead
pass values by rvalue and let the contents be moved.

* Revert "Force FlushStateToDisk on ConnectTip/DisconnectTip while not in IBD (dashpay#2560)"

This reverts commit 6dfceab.
MIPPL pushed a commit to biblepay/biblepay that referenced this pull request May 26, 2019
* commit '4570079e53ad859c571aeac3f132cf2969888e14':
  Add missing entry to changelog (dashpay#2769)
  Move IS block filtering into ConnectBlock (dashpay#2766)
  [0.13.x] Bump version to 0.13.2 and add release notes (dashpay#2749)
  Bump minChainWork and AssumeValid to block #1033120 (dashpay#2750)
  Fix error message for invalid voting addresses (dashpay#2747)
  Make -masternodeblsprivkey mandatory when -masternode is given (dashpay#2745)
  Implement 2-stage commit for CEvoDB to avoid inconsistencies after crashes (dashpay#2744)
  Add collateraladdress into masternode/protx list rpc output (dashpay#2740)
  Only include selected TX types into CMerkleBlock (dashpay#2737)
  Stop g_connman first before deleting it (dashpay#2734)
  Fix incorrect usage of begin() when genesis block is requested in "protx diff" (dashpay#2699)
  Do not process blocks in CDeterministicMNManager before dip3 activation (dashpay#2698)
  Backport bitcoin#14701: build: Add CLIENT_VERSION_BUILD to CFBundleGetInfoString (dashpay#2687)
  Fix some typos in doc/guide-startmany.md (dashpay#2711)
  Minimal fix for litemode vs bad-protx-key-not-same issue (dashpay#2694)
  Release notes v0.13.1.0 (dashpay#2689)

# Conflicts:
#	configure.ac
#	doc/guide-startmany.md
#	doc/release-notes.md
#	src/chainparams.cpp
#	src/clientversion.h
#	src/validation.cpp
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.

2 participants