Skip to content

Commit a3118c1

Browse files
PastaPastaPastaPastaPastaPasta
committed
Merge #6996: perf: reduce cs_main lock scope in evodb verify/repair operations
084bb62 chore: clang-format (UdjinM6) ce506bc perf: reduce cs_main lock scope in evodb verify/repair operations (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Previously, evodb_verify_or_repair_impl held cs_main for the entire operation, which could take minutes when verifying/repairing large block ranges. This caused significant lock contention and blocked other operations requiring cs_main. This commit reduces the cs_main lock scope to only the initial setup phase where we resolve block indexes from the active chain. The actual verification and repair work (applying diffs, rebuilding lists from blocks, verifying snapshots) now runs without holding cs_main. ## What was done? Changes: - Wrap block index resolution in a scoped cs_main lock - Remove AssertLockHeld(cs_main) from helper functions: * RecalculateAndRepairDiffs * CollectSnapshotBlocks * VerifySnapshotPair * RepairSnapshotPair * RebuildListFromBlock (CSpecialTxProcessor) - Update function signatures to remove EXCLUSIVE_LOCKS_REQUIRED(cs_main) ## How Has This Been Tested? Run evodb verify/repair on a mainnet node and monitor logs - it keeps processing other stuff while rpc command is still running. ## Breaking Changes This is safe because: - CBlockIndex pointers remain valid after lock release (never deleted) - Block parent relationships (pprev, GetAncestor) are immutable - ReadBlockFromDisk takes cs_main internally when accessing nFile/nDataPos - Helper functions only process already-loaded block data and snapshots - ChainLocks prevent deep reorgs in Dash anyway ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: kwvg: utACK 084bb62 knst: utACK 084bb62 Tree-SHA512: aa0db93133767cc6de897d2989c35f00d1cd0506c51463a86cb76cbbe51ea8be5372efb9dbc16791992d8128bc26812e075d13c30c396f97f22198e9611e75dc
2 parents 11a36d5 + 084bb62 commit a3118c1

File tree

5 files changed

+40
-52
lines changed

5 files changed

+40
-52
lines changed

‎src/evo/deterministicmns.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,8 +1586,6 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA
15861586
const CBlockIndex* start_index, const CBlockIndex* stop_index, ChainstateManager& chainman,
15871587
BuildListFromBlockFunc build_list_func, bool repair)
15881588
{
1589-
AssertLockHeld(::cs_main);
1590-
15911589
RecalcDiffsResult result;
15921590
result.start_height = start_index->nHeight;
15931591
result.stop_height = stop_index->nHeight;
@@ -1696,8 +1694,6 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA
16961694
std::vector<const CBlockIndex*> CDeterministicMNManager::CollectSnapshotBlocks(
16971695
const CBlockIndex* start_index, const CBlockIndex* stop_index, const Consensus::Params& consensus_params)
16981696
{
1699-
AssertLockHeld(::cs_main);
1700-
17011697
std::vector<const CBlockIndex*> snapshot_blocks;
17021698

17031699
// Add the starting snapshot (find the snapshot at or before start)
@@ -1749,8 +1745,6 @@ bool CDeterministicMNManager::VerifySnapshotPair(
17491745
const CBlockIndex* from_index, const CBlockIndex* to_index, const CDeterministicMNList& from_snapshot,
17501746
const CDeterministicMNList& to_snapshot, RecalcDiffsResult& result)
17511747
{
1752-
AssertLockHeld(::cs_main);
1753-
17541748
// Verify this snapshot pair by applying all stored diffs sequentially
17551749
CDeterministicMNList test_list = from_snapshot;
17561750

@@ -1795,8 +1789,6 @@ std::vector<std::pair<uint256, CDeterministicMNListDiff>> CDeterministicMNManage
17951789
const CBlockIndex* from_index, const CBlockIndex* to_index, const CDeterministicMNList& from_snapshot,
17961790
const CDeterministicMNList& to_snapshot, BuildListFromBlockFunc build_list_func, RecalcDiffsResult& result)
17971791
{
1798-
AssertLockHeld(::cs_main);
1799-
18001792
CDeterministicMNList current_list = from_snapshot;
18011793
// Temporary storage for recalculated diffs (one per block in this snapshot interval)
18021794
std::vector<std::pair<uint256, CDeterministicMNListDiff>> temp_diffs;

‎src/evo/deterministicmns.h‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -740,10 +740,10 @@ class CDeterministicMNManager
740740
BlockValidationState& state,
741741
CDeterministicMNList& mnListRet)>;
742742

743-
[[nodiscard]] RecalcDiffsResult RecalculateAndRepairDiffs(
744-
const CBlockIndex* start_index, const CBlockIndex* stop_index,
745-
ChainstateManager& chainman, BuildListFromBlockFunc build_list_func,
746-
bool repair) EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main);
743+
[[nodiscard]] RecalcDiffsResult RecalculateAndRepairDiffs(const CBlockIndex* start_index,
744+
const CBlockIndex* stop_index, ChainstateManager& chainman,
745+
BuildListFromBlockFunc build_list_func, bool repair)
746+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
747747

748748
// Migration support for nVersion-first CDeterministicMNStateDiff format
749749
[[nodiscard]] bool IsMigrationRequired() const EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main);
@@ -755,14 +755,13 @@ class CDeterministicMNManager
755755

756756
// Helper methods for RecalculateAndRepairDiffs
757757
std::vector<const CBlockIndex*> CollectSnapshotBlocks(const CBlockIndex* start_index, const CBlockIndex* stop_index,
758-
const Consensus::Params& consensus_params) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
759-
bool VerifySnapshotPair(const CBlockIndex* from_index, const CBlockIndex* to_index, const CDeterministicMNList& from_snapshot,
760-
const CDeterministicMNList& to_snapshot, RecalcDiffsResult& result)
761-
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
758+
const Consensus::Params& consensus_params);
759+
bool VerifySnapshotPair(const CBlockIndex* from_index, const CBlockIndex* to_index,
760+
const CDeterministicMNList& from_snapshot, const CDeterministicMNList& to_snapshot,
761+
RecalcDiffsResult& result);
762762
std::vector<std::pair<uint256, CDeterministicMNListDiff>> RepairSnapshotPair(
763763
const CBlockIndex* from_index, const CBlockIndex* to_index, const CDeterministicMNList& from_snapshot,
764-
const CDeterministicMNList& to_snapshot, BuildListFromBlockFunc build_list_func, RecalcDiffsResult& result)
765-
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
764+
const CDeterministicMNList& to_snapshot, BuildListFromBlockFunc build_list_func, RecalcDiffsResult& result);
766765
void WriteRepairedDiffs(const std::vector<std::pair<uint256, CDeterministicMNListDiff>>& recalculated_diffs,
767766
RecalcDiffsResult& result) EXCLUSIVE_LOCKS_REQUIRED(!cs);
768767
};

‎src/evo/specialtxman.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,6 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul
185185
bool debugLogs, BlockValidationState& state,
186186
CDeterministicMNList& mnListRet)
187187
{
188-
AssertLockHeld(cs_main);
189-
190188
// Verify that prevList either represents an empty/initial state (default-constructed),
191189
// or it matches the previous block's hash.
192190
assert(prevList == CDeterministicMNList() || prevList.GetBlockHash() == pindexPrev->GetBlockHash());

‎src/evo/specialtxman.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,8 @@ class CSpecialTxProcessor
8282
// Variant that takes an explicit starting list instead of loading from GetListForBlock
8383
// Used for rebuilding diffs from trusted snapshots
8484
bool RebuildListFromBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindexPrev,
85-
const CDeterministicMNList& prevList, const CCoinsViewCache& view, bool debugLogs,
86-
BlockValidationState& state, CDeterministicMNList& mnListRet)
87-
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
85+
const CDeterministicMNList& prevList, const CCoinsViewCache& view, bool debugLogs,
86+
BlockValidationState& state, CDeterministicMNList& mnListRet);
8887

8988
private:
9089
bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const CCbTx& cbTx,

‎src/rpc/evo.cpp‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,37 +1760,38 @@ static UniValue evodb_verify_or_repair_impl(const JSONRPCRequest& request, bool
17601760
CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
17611761
CChainstateHelper& chain_helper = *CHECK_NONFATAL(node.chain_helper);
17621762

1763-
LOCK(::cs_main);
1764-
17651763
const CBlockIndex* start_index;
17661764
const CBlockIndex* stop_index;
17671765

1768-
// Default to DIP0003 activation height if startBlock not specified
1769-
if (request.params[0].isNull()) {
1770-
const auto& consensus_params = Params().GetConsensus();
1771-
start_index = chainman.ActiveChain()[consensus_params.DIP0003Height];
1772-
if (!start_index) {
1773-
throw JSONRPCError(RPC_INTERNAL_ERROR, "Cannot find DIP0003 activation block");
1774-
}
1775-
} else {
1776-
uint256 start_block_hash = ParseBlock(request.params[0], chainman, "startBlock");
1777-
start_index = chainman.m_blockman.LookupBlockIndex(start_block_hash);
1778-
if (!start_index) {
1779-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Start block not found");
1766+
{
1767+
LOCK(::cs_main);
1768+
// Default to DIP0003 activation height if startBlock not specified
1769+
if (request.params[0].isNull()) {
1770+
const auto& consensus_params = Params().GetConsensus();
1771+
start_index = chainman.ActiveChain()[consensus_params.DIP0003Height];
1772+
if (!start_index) {
1773+
throw JSONRPCError(RPC_INTERNAL_ERROR, "Cannot find DIP0003 activation block");
1774+
}
1775+
} else {
1776+
uint256 start_block_hash = ParseBlock(request.params[0], chainman, "startBlock");
1777+
start_index = chainman.m_blockman.LookupBlockIndex(start_block_hash);
1778+
if (!start_index) {
1779+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Start block not found");
1780+
}
17801781
}
1781-
}
17821782

1783-
// Default to chain tip if stopBlock not specified
1784-
if (request.params[1].isNull()) {
1785-
stop_index = chainman.ActiveChain().Tip();
1786-
if (!stop_index) {
1787-
throw JSONRPCError(RPC_INTERNAL_ERROR, "Cannot find chain tip");
1788-
}
1789-
} else {
1790-
uint256 stop_block_hash = ParseBlock(request.params[1], chainman, "stopBlock");
1791-
stop_index = chainman.m_blockman.LookupBlockIndex(stop_block_hash);
1792-
if (!stop_index) {
1793-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Stop block not found");
1783+
// Default to chain tip if stopBlock not specified
1784+
if (request.params[1].isNull()) {
1785+
stop_index = chainman.ActiveChain().Tip();
1786+
if (!stop_index) {
1787+
throw JSONRPCError(RPC_INTERNAL_ERROR, "Cannot find chain tip");
1788+
}
1789+
} else {
1790+
uint256 stop_block_hash = ParseBlock(request.params[1], chainman, "stopBlock");
1791+
stop_index = chainman.m_blockman.LookupBlockIndex(stop_block_hash);
1792+
if (!stop_index) {
1793+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Stop block not found");
1794+
}
17941795
}
17951796
}
17961797

@@ -1802,12 +1803,11 @@ static UniValue evodb_verify_or_repair_impl(const JSONRPCRequest& request, bool
18021803
throw JSONRPCError(RPC_INVALID_PARAMETER, "stopBlock must be >= startBlock");
18031804
}
18041805

1805-
// Create a callback that wraps CSpecialTxProcessor::BuildNewListFromBlock
1806-
// NO_THREAD_SAFETY_ANALYSIS: cs_main is held by the calling function (evodb_verify_or_repair_impl)
1806+
// Create a callback that wraps CSpecialTxProcessor::RebuildListFromBlock
18071807
auto build_list_func = [&chain_helper](const CBlock& block, gsl::not_null<const CBlockIndex*> pindexPrev,
18081808
const CDeterministicMNList& prevList, const CCoinsViewCache& view,
18091809
bool debugLogs, BlockValidationState& state,
1810-
CDeterministicMNList& mnListRet) NO_THREAD_SAFETY_ANALYSIS -> bool {
1810+
CDeterministicMNList& mnListRet) -> bool {
18111811
return chain_helper.special_tx->RebuildListFromBlock(block, pindexPrev, prevList, view, debugLogs, state, mnListRet);
18121812
};
18131813

0 commit comments

Comments
 (0)