Skip to content
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

Limit amount of transactions which clients verify in parallel #2970

Open
wants to merge 9 commits into
base: testnet3
Choose a base branch
from

Conversation

vicsn
Copy link
Contributor

@vicsn vicsn commented Jan 3, 2024

Motivation

Our clients can and do run out of memory, probably because they have no limit on how many transactions they verify in parallel. This PR proposes to queue them (just like the validator does in Consensus) and limit how much parallel verification we do.

We can do a lot of clever things to increase the processing speed, like check how many constraints the incoming transactions have, await on a channel to rapidly start verifying, but the focus for now is simplicity and safety.

Test Plan

Related PRs

Implicitly assumes we have https://github.com/AleoHQ/snarkVM/pull/2271 to limit size of transactions coming through.
And this would help to minimize the overhead of transaction verification when syncing blocks with already seen transactions: https://github.com/AleoHQ/snarkVM/pull/2270

@vicsn vicsn requested a review from ljedrz January 3, 2024 21:21
node/src/client/mod.rs Outdated Show resolved Hide resolved
node/src/client/mod.rs Outdated Show resolved Hide resolved
node/src/client/router.rs Outdated Show resolved Hide resolved
node/src/client/mod.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@ljedrz ljedrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@vicsn vicsn changed the title Limit amount of transactions which can be verified in parallel Limit amount of transactions which clients verify in parallel Jan 30, 2024
use tokio::task::JoinHandle;
use tokio::{task::JoinHandle, time::sleep};

const VERIFICATION_CONCURRENCY_LIMIT: usize = 6; // 8 deployments of MAX_NUM_CONSTRAINTS will run out of memory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be dynamic based on your machine?

How much memory was this estimation based on?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was based on the Clients: 16GB of memory requirement in the snarkOS README. Please confirm if you want me to make it a dynamic value from a cli flag.

Sidenote: given our syncing issues I understood the CPU/memory requirements need to go up anyway.

@howardwu howardwu added the post-launch This issue or pull request will be handled after mainnet launch label Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
post-launch This issue or pull request will be handled after mainnet launch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants