-
Notifications
You must be signed in to change notification settings - Fork 32
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
Adding RPC Load Test Mode #121
Conversation
loadTestModePrecompiledContract | ||
loadTestModeRecall | ||
|
||
// All the modes AFTER random mode will not be used when mode random is selected |
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.
should we communicate this behavior to user via help cmd?
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.
I think this is more of an implementation detail that ideally shouldn't concern a user. E.g. if you add a new mode like loadTestUniswap
before loadTestModeRandom
it will be eligible to be called during random mode testing. If you add it after loadTestModeRandom
it will not be called ruing random mode. This is a quirk of using iota
but in this case I'm using it to exclude loadTestModeRPC
from random mode because it depends on only doing calls.
Either way, i would like to improve the docs of the modes because there are a lot of features and capabilities that are hard to understand.
@@ -163,6 +217,42 @@ func initializeLoadTestParams(ctx context.Context, c *ethclient.Client) error { | |||
} | |||
inputLoadTestParams.CurrentBaseFee = header.BaseFee | |||
|
|||
modes := *inputLoadTestParams.Modes | |||
if len(modes) == 0 { | |||
return fmt.Errorf("expected at least one mode") |
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.
no default mode?
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.
There is a default mode. In order to trigger this case, you would need to explicitly pass a blank string as a mode.
} | ||
|
||
if hasMode(loadTestModeRandom, inputLoadTestParams.ParsedModes) && inputLoadTestParams.MultiMode { | ||
return fmt.Errorf("random mode can't be used in combinations with any other modes") |
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.
seems to conflict with previous comment that its only modes after random that don't take effect
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.
These are two different ideas. One is related to the implementation (the previous comment) and the other is related to the usage (this particular error). Since random is calling randomly calling other modes, it doesn't make sense to use it in combination with another mode.
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.
looks good
@@ -0,0 +1,35 @@ | |||
// Code generated by "stringer -type=loadTestMode"; DO NOT EDIT. |
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.
I think we should document this in the README
in case someone ever needs to add a new mode
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.
yeah good point.
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.
lgtm
Description
This PR is doing a few things:
Testing
./out/polycli loadtest --rate-limit 100 --concurrency 25 --requests 100 --verbosity 701 --mode rpc --recall-blocks 500 http://127.0.0.1:8545
polycli loadtest --rate-limit 4096 --concurrency 2048 --requests 1000 --mode rpc --recall-blocks 5000 http://127.0.0.1:8545