-
Notifications
You must be signed in to change notification settings - Fork 381
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
Throttle Eth_ module queue #5945
Conversation
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.
Would change to Queue
rather than Pending
; as pending suggests in process but waiting for result.
Also MaxPendingNonSharedRequests => RequestQueueLimit; as other is a confusing description
src/Nethermind/Nethermind.JsonRpc/Modules/IRpcModuleProviderExtensions.cs
Outdated
Show resolved
Hide resolved
src/Nethermind/Nethermind.JsonRpc/Modules/IRpcModuleProviderExtensions.cs
Outdated
Show resolved
Hide resolved
nameof(IEthRpcModule.eth_uninstallFilter) + "). " + | ||
" If value is set to 0 limit won't be applied.", | ||
DefaultValue = "500")] | ||
int RequestQueueLimit { get; set; } |
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.
@benaadams it is only for non-shared methods, shouldn't we mention it in the config name?
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.
Mentions the calls its for in the description?
User wouldn't understand what "non-shared" methods are; is an implementation detail?
Fixes #5923
With spam testing
Before change:
Requests per second: 100
Requests sent in total: 100000
Succeeded requests: 4818
Failed requests: 95182
After the change:
Requests per second: 100
Requests sent in total: 100000
Succeeded requests: 57566
Failed requests: 42434
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
We did load testing with eth_call and testing on gateway infra
Optional. Remove if not applicable.
Documentation
Requires documentation update
Auto-updated with a new config parameter. However, would be great to introduce articles Nethermind for RPC Providers
Requires explanation in Release Notes
ToDo...
Remarks
Optional. Remove if not applicable.