Version 0 of gomu-gomu-no-gatling

[haroune-mohammedi]

This PR Addresses most of #2

Extendable metrics

For metrics to be extendable, I defined a Metric struct and a static variable METRICS of all the available metrics, the Metric compute field is a function that takes a map between block numbers and the number of transactions in that block and return a float (take a look at existing metrics to get a better idea)

To add a new metric, we define a function, the add a Metric instance to the METRICS list then reports will automatically use it. At a later stage, we'll allow users to choose what metrics they want to use for each benchmark and/or the whole benchmarking session from the config file

Post benchmark check

After discussing with @d-roak and @approv, what we're doing now is wait for all transactions to be incorporated into blocks to calculate metrics, otherwise we'll never be sure we're calculating the right metrics because we can calculate the metrics before the transactions are accepted or worse after that with one or empty blocks which will give us wrong results

Is that what you meant with "post benchmark check" ?

And does fail_safe mean we have to panic if one transactions is failed/not found ... ?

Still missing:

• Handle fail_fast but we'll have to define what's the expected behavior first.

For this v0, we don't have a fail_fast flag, instead we always fail during setup phase and we never fail during benchmarking phase

• Handle nonce for failed transactions, especially transactions during setup, we probably should always panic if transactions during setup fail since that'd mean the environment isn't setup correctly for the benchmarks to execute ?

Due to the previous decision about fail_fast we don't have to bother about failed transactions during setup since the program will panic, and we can't do anything about it during the benchmarking phase for now, @tdelabro suggested we sent one transaction per account which will be considered in a later stage in a separate issue/PR

• Make the number of blocks to be used to calculate the metrics for the last X blocks configurable

[haroune-mohammedi]

In another issue/PR, we'll have to:

• Skip the in between blocks from the Full Session Reports, those blocks are most probably not full as the start and end block
• Deploy accounts in parallel, especially if we introduce the one account per transaction solution for nonce problem suggested by @tdelabro
• Wait for transactions in parallel in check_transactions

[tdelabro]

I think you can remove the .to_string part, and keep them as &str

[haroune-mohammedi]

Yes, but we'll introduce lifetimes everywhere and maybe we'll have to use 'static lifetime because METRICS is a static variable and it's used in compute_all_metrics which is called by BenchmarkReport::from_block_range and BenchmarkReport::from_last_x_blocks

[tdelabro]

At this point, METRIC being a Vec should not be static anymore. It should be const and of type [Metric; 2]. And those metrics could be &'static str rather than String

[tdelabro]

Also please do the fixes, but let me resolve the conversation. You closing them makes it harder for me to keep track and review again.

[tdelabro]

At this point, METRIC being a Vec should not be static anymore. It should be const and of type [Metric; 2]. And those metrics could be &'static str rather than String