-
Notifications
You must be signed in to change notification settings - Fork 368
feat(ICRC_Rosetta): DEFI-2452: Spawn blocking for SQLite operations #7695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…setta-sqlite-cache-size-limit-and-flushing # Conflicts: # rs/rosetta-api/local/cluster/README.md # rs/rosetta-api/local/cluster/deploy.sh # rs/rosetta-api/local/cluster/templates/rosetta-icrc.yaml # rs/rosetta-api/local/cluster/values.yaml
…cache-size-limit-and-flushing' into mathias-DEFI-2452-add-configurable-icrc-rosetta-batch-size # Conflicts: # rs/rosetta-api/icrc1/src/common/storage/storage_client.rs # rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs # rs/rosetta-api/icrc1/src/config.rs # rs/rosetta-api/icrc1/src/main.rs # rs/rosetta-api/local/cluster/README.md # rs/rosetta-api/local/cluster/deploy.sh # rs/rosetta-api/local/cluster/templates/rosetta-icrc.yaml # rs/rosetta-api/local/cluster/values.yaml
…ize' into mathias-DEFI-2452-spawn-blocking-for-sqlite # Conflicts: # rs/rosetta-api/icrc1/src/common/storage/storage_client.rs
…setta-sqlite-cache-size-limit-and-flushing
…cache-size-limit-and-flushing' into mathias-DEFI-2452-add-configurable-icrc-rosetta-batch-size # Conflicts: # rs/rosetta-api/local/cluster/deploy.sh # rs/rosetta-api/local/cluster/templates/rosetta-icrc.yaml
…ize' into mathias-DEFI-2452-spawn-blocking-for-sqlite
# Conflicts: # rs/rosetta-api/icrc1/src/common/storage/storage_client.rs # rs/rosetta-api/icrc1/src/config.rs # rs/rosetta-api/local/cluster/README.md # rs/rosetta-api/local/cluster/values.yaml
|
|
||
| // As long as there are blocks to be fetched, keep on iterating over the blocks in the database with the given BATCH_SIZE interval | ||
| while !rosetta_blocks.is_empty() { | ||
| let mut account_balances_cache: HashMap<Account, BTreeMap<u64, Nat>> = HashMap::new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we move this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks! I had moved it in an earlier commit of the PR this PR was stacked on top of. I later reverted the change here since I realized it didn't make sense, but I must have missed it when merging in the latest changes from master and fixing merge conflicts. I moved it back outside the while loop now.
Change the synchronous functions in ICRC Rosetta that call SQLite to perform database I/O to be asynchronous and use
tokio::task::spawn_blockingto run the task on a thread in a thread pool, rather than using one of the few asynchronous worker threads. For ICRC Rosetta instances with many (tens of) tokens, this can help to avoid situations where the worker threads are all busy servicing long-running SQLite requests, and there are no available threads to serve e.g., simple endpoint requests such asnetwork/status, or for running the watchdog threads to see if some of the block synchronization threads have gotten stuck. Using a thread pool for spawning threads to serve the SQLite requests will use more memory, which needs to be taken into account when dimensioning the system where Rosetta is running. The features in the previously proposed PRs for limiting the SQLite cache size, and for reducing the balance sync batch size, can be used as a counter-balance.