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

go/oasis-node/cmd/debug/consim: Initial import #2784

Merged
merged 5 commits into from
Apr 9, 2020

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented Mar 25, 2020

No description provided.

@codecov
Copy link

codecov bot commented Mar 25, 2020

Codecov Report

Merging #2784 into master will increase coverage by 0.20%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2784      +/-   ##
==========================================
+ Coverage   66.98%   67.19%   +0.20%     
==========================================
  Files         342      343       +1     
  Lines       32968    32987      +19     
==========================================
+ Hits        22084    22165      +81     
+ Misses       8186     8129      -57     
+ Partials     2698     2693       -5     
Impacted Files Coverage Δ
go/consensus/tendermint/abci/mux.go 72.62% <ø> (ø)
go/consensus/tendermint/abci/mux_mock.go 0.00% <0.00%> (ø)
go/consensus/tendermint/abci/state.go 53.12% <60.00%> (-0.05%) ⬇️
go/storage/mkvs/lookup.go 69.38% <0.00%> (-2.05%) ⬇️
go/worker/storage/committee/checkpointer.go 66.37% <0.00%> (-1.77%) ⬇️
go/worker/compute/merge/committee/node.go 75.87% <0.00%> (-0.25%) ⬇️
go/storage/metrics.go 88.04% <0.00%> (ø)
go/worker/compute/txnscheduler/committee/node.go 66.35% <0.00%> (+0.61%) ⬆️
go/runtime/committee/client.go 77.15% <0.00%> (+1.29%) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 537104d...0518095. Read the comment docs.

@Yawning Yawning force-pushed the yawning/scratch/sim-test branch 9 times, most recently from 19a591c to ad3c982 Compare April 2, 2020 13:06
@Yawning Yawning changed the title w1p: go/oasis-node/cmd/debug/consim: Initial import go/oasis-node/cmd/debug/consim: Initial import Apr 2, 2020
@Yawning Yawning marked this pull request as ready for review April 2, 2020 13:54
Copy link
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

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

this all looks okay. some overarching comments:

  • nothing exercises this in CI, so it'll rot
  • this doesn't exercise the full functionality of staking, which looks at commit info stuff. the current simulator leaves those fields blank

@kostko
Copy link
Member

kostko commented Apr 3, 2020

Yeah we should run this as part of the daily test pipeline (see .buildkite/longtests.pipeline.yml).

@Yawning Yawning force-pushed the yawning/scratch/sim-test branch 2 times, most recently from 555a472 to cfc2057 Compare April 3, 2020 11:05
@Yawning
Copy link
Contributor Author

Yawning commented Apr 3, 2020

this doesn't exercise the full functionality of staking, which looks at commit info stuff. the current simulator leaves those fields blank

At some point, this becomes an exercise in re-implementing tendermint.

Yeah we should run this as part of the daily test pipeline (see .buildkite/longtests.pipeline.yml).

Do you honestly thing this will ever do anything more than burn CI resources? I highly doubt that this codebase will catch anything.

@kostko
Copy link
Member

kostko commented Apr 3, 2020

I don't see why this would catch anything more than what the existing long-term workload does, it will more probably catch less. But if we have it, we should at least run it somewhere, otherwise why have it at all?

@pro-wh
Copy link
Contributor

pro-wh commented Apr 3, 2020

this doesn't exercise the full functionality of staking, which looks at commit info stuff. the current simulator leaves those fields blank

At some point, this becomes an exercise in re-implementing tendermint.

I'm talking about sending fake results of a commit, not actually running a decentralized fault tolerance protocol.

Yeah we should run this as part of the daily test pipeline (see .buildkite/longtests.pipeline.yml).

Do you honestly thing this will ever do anything more than burn CI resources? I highly doubt that this codebase will catch anything.

a short smoketest on CI, not a long workload like this is intended to do. it'll catch code changes that break this.

@pro-wh
Copy link
Contributor

pro-wh commented Apr 3, 2020

I don't see why this would catch anything more than what the existing long-term workload does, it will more probably catch less. But if we have it, we should at least run it somewhere, otherwise why have it at all?

ok this PR is gonna need a description

While it's possible to use the pruner to try to prevent the ABCI induced
disk usage from growing out of control, having to save/prune is rather
slow, even when backed by tmpfs.

Enabling this option will keep ABCI state in memory, dramatically
speeding up cases where only the final dump is required, especially if
a sensible configuration is used for the pruner.
Just so this won't rot, I don't expect this to actually catch anything
other than code changes that break consim (or require re-generating the
genesis doc used for testing).
@Yawning Yawning merged commit 47191ec into master Apr 9, 2020
@Yawning Yawning deleted the yawning/scratch/sim-test branch April 9, 2020 12:58
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.

None yet

3 participants