config: make worker pool configurable and increase default queue size#2542
config: make worker pool configurable and increase default queue size#2542blindchaser merged 8 commits intomainfrom
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (48.75%) is below the target coverage (50.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2542 +/- ##
==========================================
- Coverage 43.33% 43.31% -0.02%
==========================================
Files 1576 1576
Lines 137831 137842 +11
==========================================
- Hits 59724 59710 -14
- Misses 72685 72709 +24
- Partials 5422 5423 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
blindchaser
force-pushed
the
yiren/config-worker-pool
branch
from
580b8d0 to
5461e6a
Compare
masih
left a comment
masih
left a comment
There was a problem hiding this comment.
Thanks for taking the time to write all the tests @blindchaser 🚀
I left a few questions and suggestions. There is one potential blocker in the logic of one of the tests regarding queue size, otherwise LGTM.
blindchaser
force-pushed
the
yiren/config-worker-pool
branch
from
16968ef to
edc038e
Compare
blindchaser
force-pushed
the
yiren/config-worker-pool
branch
from
edc038e to
7e9c0a0
Compare
blindchaser
force-pushed
the
yiren/config-worker-pool
branch
from
7e9c0a0 to
ba02e4a
Compare
evmrpc/config.go
Outdated
| if workers > 64 { | ||
| return 64 | ||
| } | ||
| return workers |
There was a problem hiding this comment.
You can write this entire body as:
min(64, runtime.NumCPU() * 2)..at which point, i would recommend against refactoring it into its own function and moving the godoc to be an inline comment that is also reflected in config documentation.
evmrpc/config_integration_test.go
Outdated
| } | ||
|
|
||
| // Try to submit many tasks quickly | ||
| for i := 0; i < taskCount; i++ { |
There was a problem hiding this comment.
Use for range syntax. Ditto for the loop above and the reminder of the code changes.
evmrpc/config.go
Outdated
| RPCStatsInterval time.Duration `mapstructure:"rpc_stats_interval"` | ||
|
|
||
| // WorkerPoolSize defines the number of workers in the worker pool. | ||
| // Set to 0 to use default (runtime.NumCPU() * 2) |
There was a problem hiding this comment.
At the very least we want to update the documentation to reflect the cap of 64. Ditto in other places.
| // MaxWorkerPoolSize caps the number of workers to prevent excessive | ||
| // goroutine creation on high-core machines. Tasks are primarily I/O bound | ||
| // (fetching and processing block logs), so 2x CPU cores can be excessive. | ||
| MaxWorkerPoolSize = 64 |
There was a problem hiding this comment.
Sad that we are repeating ourselves here regarding what the max is. Can we avoid this repetition?
…#2542) - Adds worker_pool_size and worker_queue_size config options in the app.toml [evm] section - Increases default queue from 200 to 1000 Add configurable worker_pool_size and worker_queue_size to app.toml
3 participants
Describe your changes and provide context
Testing performed to validate your change
Add configurable worker_pool_size and worker_queue_size to app.toml