test: add store seed for stress test #621

TomasArrachea · 2025-01-15T14:59:49Z

This PR adds a binary program that seeds the store. The program creates new accounts, creates notes to consume assets on those accounts, builds the resulting blocks (using the BlockBuilder) and sends them to the store. Each block is built with 16 batches, where each batch contains 16 transactions.

The program also measures and prints the average insertion time of blocks into the store. For inserting 100k accounts, the resulting avg insertion time was ~1000 ms and finished in 7 minutes.

bin/stress-test/src/main.rs

TomasArrachea · 2025-01-15T17:40:33Z

This current implementation is mimicking the default values of BlockBudget (with SERVER_MAX_BATCHES_PER_BLOCK = 4 and SERVER_MAX_TXS_PER_BATCH = 2) to build the transactions into the blocks. This means that to generate and store 1M accounts, 250k blocks are needed, which seems like a lot. Maybe using 16 tx per batch and 16 batches per block would make more sense? cc @bobbinth

bobbinth · 2025-01-15T18:04:13Z

This means that to generate and store 1M accounts, 250k blocks are needed, which seems like a lot. Maybe using 16 tx per batch and 16 batches per block would make more sense?

Yes, this is how I was thinking about it as well: 16 batches per block and 16 transactions per batch. Assuming we can insert ~5 blocks/second (just a guess, not sure how close this is to reality), we should be able to "fill" 1M blocks in about 15 mins.

To do this though, we can't go via the block producer (but should be fine if we build Block structs directly).

TomasArrachea · 2025-01-15T18:27:54Z

This means that to generate and store 1M accounts, 250k blocks are needed, which seems like a lot. Maybe using 16 tx per batch and 16 batches per block would make more sense?

Yes, this is how I was thinking about it as well: 16 batches per block and 16 transactions per batch. Assuming we can insert ~5 blocks/second (just a guess, not sure how close this is to reality), we should be able to "fill" 1M blocks in about 15 mins.

To do this though, we can't go via the block producer (but should be fine if we build Block structs directly).

I just ran it locally with those settings (using the BlockBuilder) and got ~1.5 blocks/second, so storing the 7814 blocks would take about 85 mins. Will try to implement the Block building manually and check the difference.

Edit: actually inserting the blocks is much faster, main delay is grinding the accounts

bobbinth · 2025-01-15T21:41:26Z

Edit: actually inserting the blocks is much faster, main delay is grinding the accounts

Are you using the latest next branch (which relies on the latest next from miden-base). Account grinding should be pretty minimal there.

bobbinth · 2025-01-16T18:08:13Z

Are you using the latest next branch (which relies on the latest next from miden-base). Account grinding should be pretty minimal there.

btw, if account creation is a bottleneck with the next branch as well, we can always parallelize it (e.g., using rayon) to get a decent speed up. not sure if possible, but would be great to get to the point where insertions into the store are the bottleneck rather than data generation.

TomasArrachea · 2025-01-16T18:11:59Z

Yes, even with latest version of miden-base, most of the binary runtime is spend on account grinding:

Using rayon sound good!

Edit: implemented this and got 40s to load all 1M accounts (storing 7814 blocks).

TomasArrachea · 2025-01-17T17:44:53Z

Just pushed an PoC of a simple test: send a sync request to the loaded store and print the elapsed time. We were discussing with @igamigo about how much we would want this stress test to be configurable. We could load a config json file, specifying the db dump file (created with the binary) and the requests to send and measure. How were you thinking about it @bobbinth ?

igamigo · 2025-01-17T17:56:45Z

I think a good first approach could be to make a simple CLI for generating snapshots and then executing a benchmark against a snapshot.

bobbinth · 2025-01-17T17:59:22Z

implemented this and got 40s to load all 1M accounts (storing 7814 blocks).

Is 40 second correct here? it sounds unbelievably fast (about 200 blocks/second, right?).

Just pushed an PoC of a simple test: send a sync request to the loaded store and print the elapsed time. We were discussing with @igamigo about how much we would want this stress test to be configurable. We could load a config json file, specifying the db dump file (created with the binary) and the requests to send and measure. How were you thinking about it @bobbinth ?

I think as the first step, would be good to be able to set the following:

Number of accounts (or blocks) to be created - e.g., we may want to run this at some point with 1M, 10M, 100M etc. accounts.
Location of the DB dump file so that we could pick it up for further stress testing.

In the future, we could add more config options (e.g., number of transactions per block, mix of public vs. private accounts and notes) - but that would be in follow-up PRs.

In terms of metrics, let's also start simple - I'm fine with tracking just block insertion times for now. We can also expand this in the future PRs.

TomasArrachea · 2025-01-17T19:07:50Z

Is 40 second correct here? it sounds unbelievably fast (about 200 blocks/second, right?).

Sorry, I don't think that number was correct. I recorded measurements again and got:

7814 block insertions in 700 seconds (about 10 blocks/second)
avg insertion time (block building and storing): 26 ms

igamigo · 2025-01-23T17:36:39Z

bin/stress-test/src/main.rs

+/// consumes a note created from a faucet. The cli accepts the following parameters:
+/// - `dump_file`: Path to the store database file.
+/// - `num_accounts`: Number of accounts to create.
+/// - `genesis_file`: Path to the genesis file of the store.


nit: I think these could also be doc comments in the SeedStore command as well to make Clap show help accordingly

igamigo · 2025-01-23T17:40:02Z

bin/stress-test/src/main.rs

+    new_faucet.id()
+}
+
+/// Create a new note with a single fungible asset.


nit:

Suggested change

/// Create a new note with a single fungible asset.

/// Create a new note containing 10 tokens of the fungible asset associated with the specified `faucet_id`.

igamigo · 2025-01-23T17:41:59Z

bin/stress-test/src/main.rs

+
+/// Create a new account with a given public key and anchor block. Generates the seed from the given
+/// index.
+fn create_account(anchor_block: &BlockHeader, public_key: PublicKey, index: usize) -> AccountId {


Instead of usize, I think this could be u64 directly, unless there's something I'm missing

igamigo · 2025-01-23T17:45:47Z

bin/stress-test/src/main.rs

+}
+
+/// Seed the store with a given number of accounts.
+async fn seed_store(dump_file: &Path, num_accounts: usize, genesis_file: &Path) {


nit: I think this could be higher in the file to make the code a bit easier to follow

Thanks! done in e545d92

SantiagoPittella · 2025-01-23T19:11:31Z

Makefile

@@ -73,6 +73,10 @@ install-node: ## Installs node
 install-faucet: ## Installs faucet
 	${BUILD_PROTO} cargo install --path bin/faucet --locked

+.PHONY: install-stress-test
+install-stress-test: ## Installs faucet


Suggested change

install-stress-test: ## Installs faucet

install-stress-test: ## Installs stress test binary

SantiagoPittella · 2025-01-23T19:12:53Z

Makefile

@@ -73,6 +73,10 @@ install-node: ## Installs node
 install-faucet: ## Installs faucet
 	${BUILD_PROTO} cargo install --path bin/faucet --locked

+.PHONY: install-stress-test
+install-stress-test: ## Installs faucet
+	${BUILD_PROTO} cargo install --path bin/stress-test --locked


Since the crate doesn't have a build.rs I'm not sure that we need ${BUILD_PROTO} here.

SantiagoPittella · 2025-01-23T19:40:30Z

crates/block-producer/Cargo.toml

 miden-air = { workspace = true }
+winterfell = { version = "0.11" }
+miden-objects = { workspace = true }
+rand_chacha = { version = "0.3", default-features = false }
 miden-lib = { workspace = true, features = ["testing"] }
 miden-node-test-macro = { path = "../test-macro" }
-miden-objects = { workspace = true, features = ["testing"] }
 miden-tx = { workspace = true, features = ["testing"] }


We should alphabetize the deps.

Also, we are using "testing" for some dependencies, maybe we need to use it only in [dev-dependencies]?

yes! didn't realize this was no longer needed

bobbinth

Thank you! Looks good. I left some comments inline - but a more general comment is that I find the current structure a bit difficult to follow. Would a more "linear" approach not work? For example, something like this:

Create a genesis block with the faucet account.
Create two sets of accounts $s_0$ and $s_1$, initially empty.
Create two sets of notes $p_0$ and $p_1$, initially empty.
Start a loop to create $n$ blocks (the loop would have $n$ iterations). In this loop:
a. Create accounts to populate $s_0$ but do not record them on chain.
b. Create a transaction that outputs a set of notes $p_0$ directed from the faucet to the accounts in $s_0$.
c. Create transactions to consume notes in $p_1$ against accounts in $s_1$ (this will be a noop on the first iteration).
d. Build a block from the above transactions and insert it into the store.
e. Assing $s_1 \leftarrow s_0$ and $p_1 \leftarrow p_0$.

bobbinth · 2025-01-26T20:16:22Z

bin/stress-test/Cargo.toml

@@ -0,0 +1,26 @@
+[package]
+name = "miden-node-stress-test"
+description = "Miden stress test binary"


nit: I would write the description as "A binary for running stress tests against the Miden node".

bobbinth · 2025-01-26T21:52:10Z

crates/block-producer/README.md

+## Crate Features
+
+- `tracing-forest`: sets logging using tracing-forest. Disabled by default.
+- `testing`: includes testing util functions to mock block-producer behaviour, meant to be used during development and not on production. Disabled by default.


nit: "not on production" -> "not in production".

bobbinth · 2025-01-26T21:56:30Z

crates/block-producer/Cargo.toml

 async-trait = { version = "0.1" }
 itertools = { version = "0.13" }
+miden-air = { workspace = true }
 miden-lib = { workspace = true }
 miden-node-proto = { workspace = true }
+miden-node-test-macro = { path = "../test-macro" }
 miden-node-utils = { workspace = true }
 miden-objects = { workspace = true }
 miden-processor = { workspace = true }
 miden-stdlib = { workspace = true }
 miden-tx = { workspace = true }
 rand = { version = "0.8" }
+rand_chacha = { version = "0.3", default-features = false }
 serde = { version = "1.0", features = ["derive"] }
 thiserror = { workspace = true }
 tokio = { workspace = true, features = ["rt-multi-thread", "net", "macros", "sync", "time"] }
 tokio-stream = { workspace = true, features = ["net"] }
 tonic = { workspace = true }
 tracing = { workspace = true }
+winterfell = { version = "0.11" }


Adding miden-node-test-macro as a dependency would make it impossible to publish this crates on crates.io (since all dependencies must be published).

Also, if we do need to move some dependencies from dev-dependencies here, we should make them optional and enabled only when the testing feature is enabled.

bobbinth · 2025-01-26T22:57:04Z

bin/stress-test/src/main.rs

+/// Create a new note containing 10 tokens of the fungible asset associated with the specified
+/// `faucet_id`.
+fn create_note(faucet_id: AccountId) -> Note {


Question: are we creating a P2ID notes here (I think we should be)? If so, do we not need to specify the note recipient?

bobbinth · 2025-01-26T23:04:17Z

bin/stress-test/src/main.rs

+/// Create a given number of notes and group them into transactions and batches.
+/// The batches are sent to the block builder.
+fn generate_note_batches(
+    num_notes: usize,
+    faucet_id: AccountId,
+    batch_sender: UnboundedSender<TransactionBatch>,
+) -> Vec<Note> {


I'm not sure I fully understood how this works. Wouldn't we need to create just 1 transaction per block to create notes? That is, this transaction would be executed against the faucet account and would output 256 notes directed to 256 accounts that were created in the previous block. Or does this cause issues somehow?

bobbinth · 2025-01-26T23:24:37Z

bin/stress-test/src/main.rs

+    println!(
+        "Consumed notes: inserted {} blocks with avg insertion time {} ms",
+        num_insertions,
+        (insertion_time / num_insertions).as_millis()
+    );


Would be cool to print out average block insertion time per some number of blocks (i.e., for every 1000 blocks). This way, we'd see how the insertion time changes the the number of blocks in the store increases.

Also, would be cool to print out the store database file size, ideally for each block interval as well.

TomasArrachea force-pushed the tomasarrachea-stress-test branch 2 times, most recently from 6d62ab8 to 7a561c6 Compare January 15, 2025 15:19

test: add stress binary

a7d4130

TomasArrachea force-pushed the tomasarrachea-stress-test branch from 7a561c6 to a7d4130 Compare January 15, 2025 15:30

bobbinth reviewed Jan 15, 2025

View reviewed changes

bin/stress-test/src/main.rs Outdated Show resolved Hide resolved

test: set 16 batches per block and 16 txs per batch

a2cdcd8

TomasArrachea added 2 commits January 16, 2025 14:52

Merge branch 'next' into tomasarrachea-stress-test

afec921

test: use only block builder instead of block producer

387d95d

test: parallelize account grinding

4395203

TomasArrachea force-pushed the tomasarrachea-stress-test branch from c303955 to 4395203 Compare January 16, 2025 20:54

test: add sync request test

dc3b1a2

TomasArrachea force-pushed the tomasarrachea-stress-test branch 5 times, most recently from 16e6098 to f435e68 Compare January 17, 2025 21:55

TomasArrachea added 2 commits January 17, 2025 19:06

feat: add cli to load store

63916b8

Merge branch 'next' into tomasarrachea-stress-test

5c39a4c

TomasArrachea force-pushed the tomasarrachea-stress-test branch from f435e68 to 5c39a4c Compare January 17, 2025 22:07

TomasArrachea marked this pull request as ready for review January 17, 2025 22:09

TomasArrachea added 7 commits January 21, 2025 11:27

chore: alphabetize dependencies and drop patch version

a6d4c1a

chore: update README

018078c

fix: rename num_accounts parameter

46a976f

Merge branch 'next' into tomasarrachea-stress-test

2e2e164

feat: refactor into separate helper functions

f009a8a

Merge branch 'next' into tomasarrachea-stress-test

08478dc

feat: remove query for note inclusion proof

348b1bb

TomasArrachea requested review from SantiagoPittella, bobbinth, igamigo and Mirko-von-Leipzig January 21, 2025 20:58

TomasArrachea added 4 commits January 23, 2025 12:46

fix: avoid blocking tokio runtime

dc7e265

Merge branch 'next' into tomasarrachea-stress-test

661c2cb

fix: format

00e86df

feat: use unbounded channels

046f690

igamigo reviewed Jan 23, 2025

View reviewed changes

fix: add docs and move seed_store definition

e545d92

SantiagoPittella reviewed Jan 23, 2025

View reviewed changes

TomasArrachea added 2 commits January 23, 2025 17:16

fix: alphabetize dependencies and declare as dev

606feb1

fix: update makefile target

886554d

TomasArrachea mentioned this pull request Jan 23, 2025

test: add sync request benchmark #640

Draft

TomasArrachea requested review from igamigo and SantiagoPittella January 23, 2025 21:32

bobbinth reviewed Jan 26, 2025

View reviewed changes

SantiagoPittella added 5 commits January 28, 2025 12:46

Merge branch 'next' into tomasarrachea-stress-test

4f63c1a

review: update crate description

0820ffd

review: fix block-producer readme

81b1d04

review: make testing deps optional for block-producer

d1ca045

review: add additional metrics

28f0632

SantiagoPittella mentioned this pull request Jan 29, 2025

draft: store seed refactor #657

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add store seed for stress test #621

test: add store seed for stress test #621

TomasArrachea commented Jan 15, 2025 •

edited

Loading

TomasArrachea commented Jan 15, 2025

bobbinth commented Jan 15, 2025 •

edited

Loading

TomasArrachea commented Jan 15, 2025 •

edited

Loading

bobbinth commented Jan 15, 2025

bobbinth commented Jan 16, 2025

TomasArrachea commented Jan 16, 2025 •

edited

Loading

TomasArrachea commented Jan 17, 2025

igamigo commented Jan 17, 2025

bobbinth commented Jan 17, 2025 •

edited

Loading

TomasArrachea commented Jan 17, 2025

igamigo Jan 23, 2025

igamigo Jan 23, 2025

igamigo Jan 23, 2025

igamigo Jan 23, 2025

TomasArrachea Jan 23, 2025

SantiagoPittella Jan 23, 2025

SantiagoPittella Jan 23, 2025

SantiagoPittella Jan 23, 2025

TomasArrachea Jan 23, 2025

bobbinth left a comment

bobbinth Jan 26, 2025

bobbinth Jan 26, 2025

bobbinth Jan 26, 2025

bobbinth Jan 26, 2025

bobbinth Jan 26, 2025

bobbinth Jan 26, 2025

	/// Create a new note with a single fungible asset.
	/// Create a new note containing 10 tokens of the fungible asset associated with the specified `faucet_id`.

	install-stress-test: ## Installs faucet
	install-stress-test: ## Installs stress test binary

test: add store seed for stress test #621

Are you sure you want to change the base?

test: add store seed for stress test #621

Conversation

TomasArrachea commented Jan 15, 2025 • edited Loading

TomasArrachea commented Jan 15, 2025

bobbinth commented Jan 15, 2025 • edited Loading

TomasArrachea commented Jan 15, 2025 • edited Loading

bobbinth commented Jan 15, 2025

bobbinth commented Jan 16, 2025

TomasArrachea commented Jan 16, 2025 • edited Loading

TomasArrachea commented Jan 17, 2025

igamigo commented Jan 17, 2025

bobbinth commented Jan 17, 2025 • edited Loading

TomasArrachea commented Jan 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobbinth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomasArrachea commented Jan 15, 2025 •

edited

Loading

bobbinth commented Jan 15, 2025 •

edited

Loading

TomasArrachea commented Jan 15, 2025 •

edited

Loading

TomasArrachea commented Jan 16, 2025 •

edited

Loading

bobbinth commented Jan 17, 2025 •

edited

Loading