Skip to content
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

Allow user to specify number of workers #2829

Merged
merged 3 commits into from
Mar 18, 2022
Merged

Allow user to specify number of workers #2829

merged 3 commits into from
Mar 18, 2022

Conversation

garrettjstevens
Copy link
Collaborator

As discovered with @cmdcolin yesterday, there was a workerCount property in BaseRpcDriver that was set to 0 and had no way to change it, meaning the worker count was always auto-calculated. We also realized that the config for the worker was not getting passed to the worker driver. This fixes that by passing the worker configs to the drivers and then reading workerCount out of the config. You can specify a certain number of workers like this:

{
  "configuration": {
   "rpc": {
     "drivers": {
       "WebWorkerRpcDriver": {
         "type": "WebWorkerRpcDriver",
         "workerCount": 2
        }
      }
    }
  }
}

This also contains a bug fix related to #2798. As a consequence of that change, if you had a lot of tracks open when the page loaded, it would sometimes create some unused workers, but this PR fixes that.

@garrettjstevens garrettjstevens added bug Something isn't working enhancement New feature or request labels Mar 18, 2022
@garrettjstevens garrettjstevens self-assigned this Mar 18, 2022
@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #2829 (4bfa41e) into main (26a8881) will increase coverage by 0.00%.
The diff coverage is 75.00%.

@@           Coverage Diff           @@
##             main    #2829   +/-   ##
=======================================
  Coverage   59.98%   59.99%           
=======================================
  Files         584      584           
  Lines       26688    26695    +7     
  Branches     6462     6467    +5     
=======================================
+ Hits        16010    16016    +6     
- Misses      10350    10351    +1     
  Partials      328      328           
Impacted Files Coverage Δ
packages/core/rpc/RpcManager.ts 70.58% <41.66%> (-15.13%) ⬇️
packages/core/rpc/BaseRpcDriver.ts 84.76% <88.00%> (ø)
packages/core/rpc/configSchema.ts 100.00% <100.00%> (ø)
products/jbrowse-web/src/Loader.tsx 63.84% <100.00%> (+0.28%) ⬆️
.../linear-genome-view/src/LinearGenomeView/index.tsx 85.27% <0.00%> (+0.17%) ⬆️
...inearGenomeView/components/RefNameAutocomplete.tsx 80.80% <0.00%> (+3.03%) ⬆️
packages/core/TextSearch/TextSearchManager.ts 100.00% <0.00%> (+3.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26a8881...4bfa41e. Read the comment docs.

@cmdcolin cmdcolin merged commit a2a20af into main Mar 18, 2022
@cmdcolin cmdcolin deleted the fix_workerCount branch March 18, 2022 21:03
cmdcolin pushed a commit that referenced this pull request Mar 18, 2022
* Fix bug where too many workers were created

* Add ability to specify workerCount in config

* Fix name of base rpc driver config schema
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants