remove blocking rpc and use a separate tokio runtime for jsonrpc server #10913

patrickkuo · 2023-04-14T19:20:38Z

reverted marking RPC methods as "blocking"
using separate tokio runtime for the RPC server, worker size is defaulted to 1/2 of the CPU core, can be changed using RPC_WORKER_THREAD env variable.
use tokio::spawn for some of the long running rocksdb calls to unblock the request handling task.

vercel · 2023-04-14T19:20:40Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments

Name	Status	Updated (UTC)
explorer	⬜️ Ignored (Inspect)	Apr 22, 2023 1:44am
explorer-storybook	⬜️ Ignored (Inspect)	Apr 22, 2023 1:44am
sui-wallet-kit	⬜️ Ignored (Inspect)	Apr 22, 2023 1:44am
wallet-adapter	⬜️ Ignored (Inspect)	Apr 22, 2023 1:44am

andll · 2023-04-18T17:39:43Z

crates/sui-json-rpc/src/coin_api.rs

-            .collect::<Vec<_>>();
+        let state = self.state.clone();
+
+        let mut data = spawn_monitored_task!(async move {


What is the reason to do spawn_monitored_task(...).await here and in similar places? The only effect of this is that the read request will continue to execute if RPC is cancelled, but I can't think why would we want this?

Without this, the rocksdb call will block the jsonrpsee handle_request, this causes the RPC server to not respond to any new request until it complete.

…or jsonrpc server #10913

longbowlu

@patrickkuo have we tried to move all the spawn to the jsonrpc layer? I'm a bit concerned the changes in authority.rs could impact validator performance. It could be in a bad or even good way, but hard to tell without experimenting first.

longbowlu · 2023-04-21T21:54:38Z

crates/sui-core/src/authority.rs

+                    if obj_ref.2.is_alive() {
+                        match database.get_object_by_key(&object_id, obj_ref.1)? {
+                            None => {
+                                error!("Object with in parent_entry is missing from object store, datastore is inconsistent");


should we log the ObjectID here

patrickkuo · 2023-04-21T23:27:52Z

@patrickkuo have we tried to move all the spawn to the jsonrpc layer? I'm a bit concerned the changes in authority.rs could impact validator performance. It could be in a bad or even good way, but hard to tell without experimenting first.

sure, I will limit the change to RPC layer

…er (#10913) * reverted marking RPC methods as "blocking" * using separate tokio runtime for the RPC server, worker size is defaulted to 1/2 of the CPU core, can be changed using `RPC_WORKER_THREAD` env variable. * use tokio::spawn for some of the long running rocksdb calls to unblock the request handling task.

## Description Load testing shows that there's some perf regression in `multi_get_transaction_blocks` after #10913 because some of the blocking queries such as computing object/balance changes are not wrapped in spawn_async. This also impacts the perf for `query_transaction_blocks` which also uses `multi_get_transaction_blocks` under the hood. ## Next steps Audit rest of the codebase and make sure all blocking calls are wrapped in spawn_async ## Test Plan Tested against loadgen client and observed that performance regression is gone --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes

patrickkuo force-pushed the pat/remove_rpc_blocking branch 6 times, most recently from 71c44dc to 8beb078 Compare April 18, 2023 00:28

patrickkuo changed the base branch from main to releases/sui-v0.32.0-release April 18, 2023 00:47

patrickkuo force-pushed the pat/remove_rpc_blocking branch from 7d74669 to b159836 Compare April 18, 2023 17:20

andll reviewed Apr 18, 2023

View reviewed changes

patrickkuo force-pushed the pat/remove_rpc_blocking branch 6 times, most recently from 52823c2 to a4e7654 Compare April 20, 2023 02:58

github-actions bot added the Type: Documentation Improvements or additions to documentation label Apr 20, 2023

patrickkuo changed the base branch from releases/sui-v0.32.0-release to main April 20, 2023 03:14

patrickkuo force-pushed the pat/remove_rpc_blocking branch from a4e7654 to 15b5eef Compare April 20, 2023 11:28

github-actions bot removed the Type: Documentation Improvements or additions to documentation label Apr 20, 2023

patrickkuo force-pushed the pat/remove_rpc_blocking branch 3 times, most recently from d67b8c3 to 4fe597f Compare April 21, 2023 12:12

patrickkuo marked this pull request as ready for review April 21, 2023 15:16

patrickkuo changed the title ~~remove blocking rpc~~ remove blocking rpc and use a separate Tokyo runtime for jsonrpc server Apr 21, 2023

patrickkuo requested review from 666lcz, oxade and andll April 21, 2023 15:17

patrickkuo changed the title ~~remove blocking rpc and use a separate Tokyo runtime for jsonrpc server~~ remove blocking rpc and use a separate tokio runtime for jsonrpc server Apr 21, 2023

remove blocking rpc

b404449

patrickkuo added 8 commits April 21, 2023 21:09

use new tokio runtime for jsonrpsee

1ac6e6b

spawn task for rocksdb calls

824d3c9

add tokio worker limit

e7fd399

fix test

d06c8b5

remove block_on

1214846

make worker thread configurable

fd24e90

fix test

347f2e0

fix test

1e97d38

patrickkuo force-pushed the pat/remove_rpc_blocking branch from b5885ab to 1e97d38 Compare April 21, 2023 20:10

patrickkuo added a commit that referenced this pull request Apr 21, 2023

[cherrypick] - remove blocking rpc and use a separate tokio runtime f…

6269f04

…or jsonrpc server #10913

longbowlu reviewed Apr 21, 2023

View reviewed changes

patrickkuo marked this pull request as draft April 21, 2023 23:53

keep tokio::spawn changes in RPC layer

2bb455f

patrickkuo marked this pull request as ready for review April 22, 2023 02:40

patrickkuo requested a review from longbowlu April 22, 2023 17:45

longbowlu approved these changes Apr 22, 2023

View reviewed changes

patrickkuo merged commit 4b2b5c7 into main Apr 22, 2023

patrickkuo deleted the pat/remove_rpc_blocking branch April 22, 2023 19:05

666lcz mentioned this pull request Apr 27, 2023

[rpc] wrap multi_get_transaction_blocks with spawn async #11414

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove blocking rpc and use a separate tokio runtime for jsonrpc server #10913

remove blocking rpc and use a separate tokio runtime for jsonrpc server #10913

patrickkuo commented Apr 14, 2023 •

edited

Loading

vercel bot commented Apr 14, 2023 •

edited

Loading

andll Apr 18, 2023

patrickkuo Apr 19, 2023

longbowlu left a comment •

edited

Loading

longbowlu Apr 21, 2023

patrickkuo commented Apr 21, 2023

remove blocking rpc and use a separate tokio runtime for jsonrpc server #10913

remove blocking rpc and use a separate tokio runtime for jsonrpc server #10913

Conversation

patrickkuo commented Apr 14, 2023 • edited Loading

vercel bot commented Apr 14, 2023 • edited Loading

andll Apr 18, 2023

Choose a reason for hiding this comment

patrickkuo Apr 19, 2023

Choose a reason for hiding this comment

longbowlu left a comment • edited Loading

Choose a reason for hiding this comment

longbowlu Apr 21, 2023

Choose a reason for hiding this comment

patrickkuo commented Apr 21, 2023

patrickkuo commented Apr 14, 2023 •

edited

Loading

vercel bot commented Apr 14, 2023 •

edited

Loading

longbowlu left a comment •

edited

Loading