Remove localClient.mtx

[jinsankim]

Summary

I found that my node's overall performance is slow down when I query something concurrently.

Problem Definition

I've set up my nodes and lcd, light client daemon. I found that my node's overall performance is slow down when I query something concurrently. W/ some investigating, I suspect now the bottle neck of this issue is (ABCI) localClient. I found that all functions of localClient is locked by localClient.mtx and also almost RPC handlers call localClient's functions.

Proposal

Could we remove localClient.mtx or revise it?

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[melekes]

It's something we discussed a while back. There are certain optimizations we can do. For example, we probably don't need to hold the mutex when calling app.callback. You're more than welcome to investigate further and send us a PR 😉

[jinsankim]

@melekes The issue, you mentioned, might be #2721 and the PR might be #2748, isn't it?

For example, we probably don't need to hold the mutex when calling app.callback

In my point of view, it looks like kind of micro optimization. I'm thinking more destructive way. I'm wondering why client , even though it's localClient, should protect server in terms of concurrency. With client side protection, I think we couldn't control and protect the critical section more efficient way. My idea is to remove the mutex from the client side and ABCI (or ABCI server) should protect itself from concurrency. Please could I get your feedback?

[melekes]

yeah, specifically this comment: #2748 (comment)

I'm wondering why client , even though it's localClient, should protect server in terms of concurrency

because there's no server?) localClient acts as both client & server

[jinsankim]

because there's no server?) localClient acts as both client & server

I understand what you mean but, in my point of view, I consider ABCI implementation as a server. So I expressed ABCI (or ABCI server). I still think it's needed to remove mutex from localClient for optimization and also app, cosmos-sdk, or whatever implements ABCI interface should protect itself from concurrency for better optimization.

[melekes]

ABCI interface should protect itself from concurrency for better optimization.

I'm going to open a discussion on Github. This will change the current paradigm where Tendermint serialises calls to ABCI app.

Let's continue discussion in #6048.

[melekes]

#6171