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-test-runner: configurable sanitychecker #3719

Merged
merged 5 commits into from
Mar 3, 2021

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented Feb 22, 2021

Daily longterm tests have been failing since validator-0 was falling behind the rest of the network due to the supplementary-sanity checker. This moves the supplementary-sanity checker to a new (client-1) node, which can fall behind without affecting rest of the network.

Also increase the timeout between governance workload iterations, as currently the amount of proposals was quickly pilling up over time (and both queries-workload and sanity-checker, go through all (even expired) proposal and votes).

TODO:

@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

Merging #3719 (fcbcc97) into master (4956fbd) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3719      +/-   ##
==========================================
+ Coverage   66.90%   67.01%   +0.10%     
==========================================
  Files         401      401              
  Lines       39855    39855              
==========================================
+ Hits        26665    26708      +43     
+ Misses       9412     9372      -40     
+ Partials     3778     3775       -3     
Impacted Files Coverage Δ
go/oasis-node/cmd/common/pprof/pprof.go 66.66% <100.00%> (+46.29%) ⬆️
go/consensus/tendermint/apps/governance/query.go 83.33% <0.00%> (-16.67%) ⬇️
go/worker/common/committee/runtime_host.go 64.28% <0.00%> (-6.25%) ⬇️
go/consensus/api/submission.go 69.01% <0.00%> (-5.64%) ⬇️
go/storage/api/root_cache.go 71.11% <0.00%> (-4.45%) ⬇️
go/storage/database/database.go 73.39% <0.00%> (-1.84%) ⬇️
go/consensus/tendermint/governance/governance.go 58.67% <0.00%> (-1.66%) ⬇️
go/consensus/tendermint/abci/mux.go 58.60% <0.00%> (-1.55%) ⬇️
go/consensus/tendermint/apps/staking/fees.go 46.37% <0.00%> (-1.45%) ⬇️
...onsensus/tendermint/apps/governance/state/state.go 71.83% <0.00%> (-1.41%) ⬇️
... and 26 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 4956fbd...fcbcc97. Read the comment docs.

@ptrus ptrus force-pushed the ptrus/feature/e2e-configurable-sanitychecker branch 14 times, most recently from b12d513 to 41bc63d Compare March 1, 2021 11:09
@ptrus ptrus force-pushed the ptrus/feature/e2e-configurable-sanitychecker branch 2 times, most recently from c51fa2b to 2eb29ed Compare March 3, 2021 06:24
@ptrus ptrus marked this pull request as ready for review March 3, 2021 15:53
@ptrus
Copy link
Member Author

ptrus commented Mar 3, 2021

@kostko This is ready for review. The last 2 failed runs were due to runtime staking messages being resubmitted (related to compute node being restarted at an unlucky time), causing the operation to re-apply (since the runtime doesn't use nonces).

I think we should tackle those in a subsequent PR as this one got big enough and it's kinda hard to keep testing the changes here while the daily tests are also being run. This PR however should fix the permanent test failures that are currently happening on the daily tests runs.

@ptrus ptrus force-pushed the ptrus/feature/e2e-configurable-sanitychecker branch from 2eb29ed to fcbcc97 Compare March 3, 2021 15:57
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

Comment on lines +151 to +154
if cfg.EnableProfiling {
val.Node.pprofPort = net.nextNodePort
net.nextNodePort++
}
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if we could unify/cleanup this as it is the same for every Node, but this is also true for the Node constructor above so probably best for a separate issue/PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 I'll open an issue

@ptrus ptrus merged commit 5c5223f into master Mar 3, 2021
@ptrus ptrus deleted the ptrus/feature/e2e-configurable-sanitychecker branch March 3, 2021 18:07
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.

2 participants