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)

Most upvoted comments

This is a great ticket. Thank you @tiger5226 !