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.

Sign up for GitHub

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

Jump to bottom

Introduce long running mode to linera benchmark #3265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce long running mode to linera benchmark #3265

wants to merge 1 commit into from
+279 −133

Conversation

ndr-ds
Copy link
Contributor

@ndr-ds ndr-ds commented Feb 6, 2025 •
edited
Loading

Motivation

We need a way of proposing blocks to a network at a fixed BPS rate

Proposal

Introduce a long running mode that you can specify a BPS rate, and it'll attempt to reach that. This is the most naive version, doesn't scale very well.
We need to figure out how much we're gonna paralellize these before having another binary run this in different processes with different wallets.
My proposal is that we implement until Version 3, then instead of the "worker tasks" we split these into separate processes with separate wallets.

Test Plan

Ran it locally against a local kind network.

Release Plan

  • Nothing to do / These changes follow the usual release cycle.

This was referenced Feb 6, 2025
Merged
Merged
Merged
Merged
Merged
Closed
Merged
@ndr-ds Graphite App
Copy link
Contributor Author

ndr-ds commented Feb 6, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch from f45203e to b6212a2 Compare February 6, 2025 03:58
@ndr-ds ndr-ds requested review from afck, christos-h, jvff, ma2bd and Twey February 6, 2025 03:58
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch 3 times, most recently from 26ecdea to ea4e86f Compare February 6, 2025 04:05
@burakacar7

This comment was marked as spam.

Sign in to view
afck
afck reviewed Feb 6, 2025
View reviewed changes
linera-client/src/client_context.rs Outdated
};

#[cfg(feature = "benchmark")]
fn deserialize_response(response: RpcMessage) -> Option<ChainInfoResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

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

(You could make it a static method of ChainClient below; one less cfg attribute.)

linera-client/src/client_options.rs Outdated
@@ -602,6 +602,11 @@ pub enum ClientCommand {
/// If none is specified, the benchmark uses the native token.
#[arg(long)]
fungible_application_id: Option<linera_base::identifiers::ApplicationId>,

/// If provided, will be long running, and block proposals will be send at the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// If provided, will be long running, and block proposals will be send at the
/// If provided, will be long running, and block proposals will be sent at the

linera-client/src/client_context.rs
num_sent_proposals = 0;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be slower than running run_benchmark in a loop. What's the advantage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal here is having block proposals be sent at a given/controlled BPS/TPS. This will allow us to ramp this up while watching metrics (client's resources like CPU/Memory/IO, and validator metrics like proxy latency, etc). This way we can figure out what the max TPS is for different network configurations, and also play around with parameters to find potential bottlenecks.
But like I said, this is version 1 from the doc. It's not very scalable. So this will become more useful as we build the next versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm keeping the old version around btw because I think if we ever replace linera-benchmark with something that calls this, we'll need the old version for CI, probably, as we won't want it to run continuously

@ndr-ds ndr-ds force-pushed the 02-05-make_benchmark_block_proposals_now_returns_blockproposals branch from b0e57b7 to 89a96c3 Compare February 6, 2025 13:37
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch from ea4e86f to cf574c9 Compare February 6, 2025 13:37
@ndr-ds ndr-ds force-pushed the 02-05-make_benchmark_block_proposals_now_returns_blockproposals branch from 89a96c3 to d10477e Compare February 6, 2025 14:19
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch from cf574c9 to 7abebc9 Compare February 6, 2025 14:24
afck
afck reviewed Feb 6, 2025
View reviewed changes
Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

This is not a blocker from my side, but I don't think we'll need two separate implementations for this in the long run. A limit of blocks per chain that can be set to 1 would make run_benchmark a special case of run_benchmark_long_running.

@ndr-ds Graphite App
Copy link
Contributor Author

ndr-ds commented Feb 6, 2025

This is not a blocker from my side, but I don't think we'll need two separate implementations for this in the long run. A limit of blocks per chain that can be set to 1 would make run_benchmark a special case of run_benchmark_long_running.

True, we can think about unifying it at some point, but it just felt easier for now to keep them separate. I can look into that in a follow up PR

@ndr-ds ndr-ds force-pushed the 02-05-make_benchmark_block_proposals_now_returns_blockproposals branch from d10477e to e54f165 Compare February 6, 2025 17:32
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch from 7abebc9 to 04924c4 Compare February 6, 2025 17:32
@ndr-ds ndr-ds force-pushed the 02-05-make_benchmark_block_proposals_now_returns_blockproposals branch from e54f165 to e0c17fe Compare February 6, 2025 17:42
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch 2 times, most recently from d761756 to 6fafdeb Compare February 6, 2025 18:12
@ndr-ds ndr-ds changed the base branch from 02-05-make_benchmark_block_proposals_now_returns_blockproposals to graphite-base/3265 February 6, 2025 18:40
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch from 6fafdeb to 14821a3 Compare February 6, 2025 18:42
@ndr-ds ndr-ds force-pushed the graphite-base/3265 branch from e0c17fe to b843259 Compare February 6, 2025 18:42
@ndr-ds ndr-ds changed the base branch from graphite-base/3265 to main February 6, 2025 18:43
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch 2 times, most recently from b63a9ca to 5145ba3 Compare February 10, 2025 22:47
@ndr-ds ndr-ds mentioned this pull request Feb 10, 2025
Draft
@ndr-ds ndr-ds requested a review from afck February 11, 2025 19:23
@ndr-ds ndr-ds force-pushed the 02-05-introduce_long_running_mode_to_linera_benchmark branch from 5145ba3 to 7564fd3 Compare February 11, 2025 23:00
@ndr-ds ndr-ds mentioned this pull request Feb 11, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christos-h christos-h Awaiting requested review from christos-h

@jvff jvff Awaiting requested review from jvff

@ma2bd ma2bd Awaiting requested review from ma2bd

@Twey Twey Awaiting requested review from Twey

@afck afck Awaiting requested review from afck

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ndr-ds @burakacar7 @afck