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

Display block and chunk producers of an epoch #7222

Merged
merged 16 commits into from
Jul 21, 2022
Merged

Conversation

nikurt
Copy link
Contributor

@nikurt nikurt commented Jul 20, 2022

No description provided.

@nikurt nikurt requested a review from a team as a code owner July 20, 2022 11:59
@nikurt nikurt requested a review from mina86 July 20, 2022 11:59
matklad and others added 12 commits July 20, 2022 14:00
There are many lovable aspects to `setup_mock_all_validators`! What this PR deals with is "tying the knot" infra. `setup_mock_validator` is based on creating `n` validators and setting up the mock network between them. To create a mock network, you need all `n` validators, and to create a validator, you need a mock network. The cycle is broken at runtime, in three different ways: 

* inside `setup_mock_all_validators`, we have `locked_connectors` 
* at some call-sites, we overwrite the whole `peer_manager_mock`
* yet at other call-sites, we essentially create a copy of `locked_connectors`

This PR unifies that to a single infra: 

* there's a single `Arc<OnceCell<Vec<Addr<Client>>>>` inside `setup_mock_all_validators`
* `OnceCell::wait` is used as a barrier 
* The `&[Addr<CLient>]` are passed in as an argument to `peer_manager_mock`, so that it doesn't have to re-invent a way to get a hold of those

This changes the signature of the function, so the diff is pretty large. It is designed to be reviewed per-commit.
The condition to check for high variance of measurements so far has been
that no single sample is further than 15% from the mean away. We often
missed that when running with 5 iterations and one warm-up iteration.
The counter-intuitive side of this check is that is becomes less likely
that variance is considered low enough, if more iterations are added.

The new condition computes actual variance and standard deviation.
Now we expect that standard deviation is lower than 10%.

Also add an outer loop to block overhead computation. Usually there
is a loop + a number of actions inside a block. But this estimation
has empty blocks and therefore each measurement so far was
just a single apply block call. Grouping 10 empty blocks together
for each measurement and adding an outer loop for `--iters` reduces 
the overall variance.
Got this one spuriously(?) firing in some test with nightly features.
Let's make the failure a bit more helpful here!
trie_key module stores separators and keys as slices even though they
are all just one-byte long.  There are parts of the code which assume
the length to be one and won’t work if the length ever changes.

Simplify the code by keeping the values as plain bytes rather than
slices so that it’s explicit in their type that they are always just
one byte.
* eagarly compute assignemt of validators to blocks & chunks, which I find a bit easier to wrap my head around (you can print `EpochValidatorSet`s and just see the result)
* compress the code by centralizing cfg handling
Closes #7203 

There are a few structs that do not need to derive `Serialize` and `Deserialize`.  Remove the derives highlights that there are some structs that we do not need at all and that we do not need to define custom deserializer: `bytes_as_string`.

Tests
-------
- `cargo check --tests --benches` does not complain.
- Other existing tests
Explicitly tracking the metric inside `GasCost` is awkward for zero
values and it requires the estimator configuration to be passed around
more than strictly necessary.

This change makes the metric implicit, by virtue of having or not having
the measurement values stored inside.

For conversion to gas, and when printing, `ICount` measurements are
given priority over `Time` measurements when for both values exist.
This could be changed, if both need to be tracked simultaneously, for
now only one of them is of interest at the time.
Creating a function to get int rocks db properties for every column and adding a way to export it as a prometheus metric.
Reorganising parse_statistics to write stats into provided result param.
We have been copying the same code multiple times, this refactor
pulls out the functionality the `TestEnv`.
@nikurt nikurt requested a review from mm-near July 20, 2022 12:04
@near-bulldozer near-bulldozer bot merged commit 0f81dca into master Jul 21, 2022
@near-bulldozer near-bulldozer bot deleted the nikurt-bp-cp branch July 21, 2022 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants