This repository has been archived by the owner on Nov 6, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Ensure jsonrpc threading settings are sane #11267
Merged
dvdplm
merged 11 commits into
master
from
dp/fix/better-default-jsonrpc-threading-settings
Nov 18, 2019
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
9360174
Ensure jsonrpc threading settings are sane
dvdplm d94e313
Update parity/rpc.rs
dvdplm 5fd5cb0
Update parity/rpc.rs
dvdplm dc0d349
Remove (i.e. deprecate) `--jsonrpc-threads` command line option
dvdplm 5b74e46
Call numbers NUM
dvdplm a97a484
Don't show a default for --jsonrpc-threads (deprecated)
dvdplm 91911fc
Show deprecation warning when using `--jsonrpc-threads` or `processin…
dvdplm fad3bd1
Update parity/deprecated.rs
dvdplm cf63b08
Fix test
dvdplm 946763c
Merge branch 'dp/fix/better-default-jsonrpc-threading-settings' of gi…
dvdplm c49a057
Fix tests for real
dvdplm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If the default is
Some
, should it beusize
then? How a user can set it toNone
and does it make sense? Maybe we should keep anOption
and useunwrap_or(4)
instead?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.
None
here implies "use defaults", i.e. the user didn't set a value. Either way, for this particular line it's moot: thejsonrpc-threads
option does nothing and I removed it in dc0d349 – essentially it was once used to set the size of aCpuPool
but since #9657 it has not had any effect at all.The equivalent code today sets up the tokio
Executor
which is used for almost all async subsystems (sync, price fetch, rpcs and more) so it is proooobably not something users should be fiddling with. If the need to do so arises we should either use separate executors or introduce some other mechanism of configuring and reserving threads for each subsystem.