- 
                Notifications
    
You must be signed in to change notification settings  - Fork 32
 
✨Dask sidecar: add threads multiplier (⚠️ Devops) #8419
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
✨Dask sidecar: add threads multiplier (⚠️ Devops) #8419
Conversation
          Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@            Coverage Diff             @@
##           master    #8419      +/-   ##
==========================================
+ Coverage   87.89%   87.91%   +0.01%     
==========================================
  Files        1953     1953              
  Lines       76045    76047       +2     
  Branches     1341     1341              
==========================================
+ Hits        66842    66858      +16     
+ Misses       8802     8788      -14     
  Partials      401      401              
 
 Continue to review full report in Codecov by Sentry. 
 🚀 New features to boost your workflow:
  | 
    
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.
Thanks
          
🧪 CI InsightsHere's what we observed from your CI run for 784bfe0. 🟢 All jobs passed!But CI Insights is watching 👀  | 
    
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.
Pull Request Overview
This PR introduces a configurable threads multiplier for Dask workers to optimize thread allocation based on CPU count rather than using fixed values. This allows for better resource utilization, particularly for non-billable scenarios and non-resource-heavy jobs.
- Adds 
DASK_NTHREADS_MULTIPLIERenvironment variable with default value of 1 - Implements thread calculation logic in the dask-sidecar boot script
 - Updates configuration files and settings across multiple services
 
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description | 
|---|---|
| services/docker-compose.yml | Adds new environment variable to clusters-keeper service | 
| services/director-v2/src/simcore_service_director_v2/modules/comp_scheduler/_scheduler_dask.py | Adds enhanced error logging for task retrieval failures | 
| services/dask-sidecar/docker/boot.sh | Implements core multiplier logic with validation and calculation | 
| services/clusters-keeper/tests/unit/conftest.py | Updates test configuration with new environment variable | 
| services/clusters-keeper/src/simcore_service_clusters_keeper/utils/clusters.py | Passes multiplier to dask-sidecar environment | 
| services/clusters-keeper/src/simcore_service_clusters_keeper/data/docker-compose.yml | Configures multiplier for dask workers and increases CPU limit | 
| services/clusters-keeper/src/simcore_service_clusters_keeper/core/settings.py | Defines new setting with validation constraints | 
| .env-devel | Sets default development environment value | 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
        
          
                services/clusters-keeper/src/simcore_service_clusters_keeper/core/settings.py
          
            Show resolved
            Hide resolved
        
      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.
thx
        
          
                services/director-v2/src/simcore_service_director_v2/modules/comp_scheduler/_scheduler_dask.py
          
            Show resolved
            Hide resolved
        
      4704124    to
    784bfe0      
    Compare
  
    
          
 | 
    



What do these changes do?
The dask worker is setup to either:
In non-billable scenarii, and when running jobs that are not resource heavy, it makes sense that the number of threads is higher than the number of CPUs. A fixed amount is suboptimal as this will apply the same number on any type of machine. The multiplier is relative to the number of CPUs.
This PR introduces
DASK_NTHREADS_MULTIPLIERfor the dask-sidecar service which is defaulted to 1. The pendantCLUSTERS_KEEPER_DASK_NTHREADS_MULTIPLIERis also added.Related issue/s
How to test
Dev-ops