lbrycrd: Concurrent RPC requests against the ClaimTrie blocked critical section lock
Summary
In syncing the ClaimTrie in LbryCRD with Chainquery a bottleneck was identified where a mutex lock on the critical section was blocking concurrent RPC connections from accessing the claimtrie in the same RPC call. The example is the getclaimsforname
call. The mutex lock is located in several places:
Affected Code
getclaimsintrie
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L36
getclaimtrie
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L108
getvalueforname
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L182
getclaimsforname
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L308
getclaimbyid
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L373
getotalclaimednames
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L414
gettotalclaims
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L434
getotalvalueofclaims
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L456
getclaimsfortx
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L495
getnameproof
https://github.com/lbryio/lbrycrd/blob/master/src/rpc/claimtrie.cpp#L720
Proposal
These locks are locking a critical section. It is used in many areas of bitcoin. However, since we are dealing with the claimtrie, we should not use a critical section lock. We should implement a shared mutex that allows for RW/RO shared/exclusive access.
The processing of a block should require exclusive access to the claimtrie due to the ability to add, delete, update claimtrie nodes. Since the RPC calls do not modify but read the trie these should provide for shared access allowing concurrent reads of the claimtrie.
It appears this lock was simply copied from other bitcoin rpc calls.
Use Case
The use case is where you are processing claim specific information where you can get the claims in the trie with getclaimsintrie
, however subsequent calls to getclaimsforname
are required to get certain fields like effective_amount
or valid_at_height
.
Reproduction 1 - Chainquery
chainquery.zip(linux)
run: ./chainquery serve
branch: https://github.com/lbryio/chainquery/tree/lbrycrdissue
code: https://github.com/lbryio/chainquery/blob/lbrycrdissue/daemon/jobs/claimtriesync.go#L21
Outputs time per call of iterating through the claim names and getting the claims for each name from lbrycrd.
Reproduction 2 - Apache Benchmark
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Comments: 24 (14 by maintainers)
This is a great ticket. Thank you @tiger5226 !