-
Notifications
You must be signed in to change notification settings - Fork 110
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
fix: Stress test refactor #1501
Conversation
!!!WARNING!!! Be very careful about using Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203 Pay extra attention to the way |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use functions in cmd/zetae2e/config
to simplify runner initialization.
It's also a bit unclear if it is for local testing or live networks. Some of the code implies it's for localnet, but we also provide RPC endpoints as flag option.
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
contrib/localnet/orchestrator/smoketest/cmd/smoketest/stress.go
Outdated
Show resolved
Hide resolved
The cmd should be able to do both, depending on the network flag. I mentioned in some of the comments that, it will be useful to make make sure the test is working correctly locally before attempting to run on an external network. This can save some time and effort if there are minor issues. |
@kevinssgh testing the command locally, is this normal logs? The tx number per min seems low to me
|
Yes, locally it is much slower than the real network. You also have to remember this is outbound txn rate which takes some time based on the scheduler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's merge so we can sync we the other changes on the other PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me ,
Curious though why are we using a withdraw only for the trees test , because the withdraw would be limited by the outbound TSS signing , which we already know .
Is the purpose only to add more transactions into the cosmos mempool ?
It might be a good idea to consider lowering the rate for withdraws and add some more transactions into the mix , we can create inbounds , or even bank sends back and forth between two accounts , which might be a more realistic simulation
The idea behind this stress test was to test the bottleneck in the network which is outbound withdraws. We can add more to this test in the future for sure as well. |
Description
Enabled stress test cmd in smoketest
Closes:
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.
Checklist: