Skip to content

fix: issue 3815#4026

Merged
johanneskoester merged 1 commit intosnakemake:mainfrom
fgvieira:increasing_resources
Mar 10, 2026
Merged

fix: issue 3815#4026
johanneskoester merged 1 commit intosnakemake:mainfrom
fgvieira:increasing_resources

Conversation

@fgvieira
Copy link
Copy Markdown
Contributor

@fgvieira fgvieira commented Mar 10, 2026

Fixes #3815

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

Summary by CodeRabbit

Release Notes

  • Refactor
    • Modified job scheduling logic to use a unified selection approach with fallback handling, affecting how jobs are processed during scheduling operations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 77092005-e11a-4a44-92bb-6c73d9c7dbd7

📥 Commits

Reviewing files that changed from the base of the PR and between b88171c and c5c8c05.

📒 Files selected for processing (1)
  • src/snakemake/scheduling/job_scheduler.py
💤 Files with no reviewable changes (1)
  • src/snakemake/scheduling/job_scheduler.py

📝 Walkthrough

Walkthrough

Removed an optimization in the run_selector function that previously invoked the greedy job selector directly when exactly one job was in the candidate set. The code now always calls the general job_selector function first, falling back to greedy selection only if needed.

Changes

Cohort / File(s) Summary
Job Scheduler Optimization Removal
src/snakemake/scheduling/job_scheduler.py
Removed conditional branch that directly called greedy selector for single-job candidate sets; now always invokes general job_selector first with fallback logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'fix: issue 3815' is vague and references only an issue number without describing the actual change made to the codebase. Revise the title to clearly describe the specific fix, such as 'fix: remove optimization shortcut in job scheduler for single-job scenarios' to reflect the actual code changes.
Description check ❓ Inconclusive The description contains only a placeholder comment and unchecked QC items without any substantive explanation of the changes, rationale, or implementation details. Add a detailed description explaining what issue #3815 addresses, why the change was necessary, and how it resolves the problem. Complete the QC checklist by checking relevant items or providing justification.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@johanneskoester johanneskoester merged commit b0eec96 into snakemake:main Mar 10, 2026
61 checks passed
@fgvieira fgvieira deleted the increasing_resources branch March 10, 2026 15:51
johanneskoester pushed a commit that referenced this pull request Mar 11, 2026
Before this change, in some cases jobs were started without calling
`update_available_resources` to subtract the resources used from the
resource pool. Regardless, when the job completed, the resources were
re-added to the resource pool.
This change ensures that the resource pool is always updated, and
slightly rearranges the logic to avoid duplicating the call to
`job_scheduler_greedy`.

The PR also includes a test that failed before the change, and now
passes; it tests the behaviour described in the original issue
(resources over allocated), rather than the specific bug fixed above.

fixes #3913


### QC
<!-- Make sure that you can tick the boxes below. -->

* [X] The PR contains a test case for the changes or the changes are
already covered by an existing test case.
* [X] The documentation (`docs/`) is updated to reflect the changes or
this is not necessary (e.g. if the change does neither modify the
language nor the behavior or functionalities of Snakemake).


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Added test suite validating thread allocation and resource constraints
during parallel workflow execution
* Test verifies thread usage remains within configured limits across
concurrent tasks
* Validates job scheduling behavior and completion tracking with
multi-threaded rule execution
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
johanneskoester pushed a commit that referenced this pull request Mar 13, 2026
🤖 I have created a release *beep* *boop*
---


##
[9.17.0](v9.16.3...v9.17.0)
(2026-03-13)


### Features

* Allow storing snakemake metadata in files or databases
([#4012](#4012))
([dd75f31](dd75f31))
* Allow to specify comparison command per-unit test
([#3956](#3956))
([b88171c](b88171c))
* job table orderd topological when run is started
([#4018](#4018))
([75cf506](75cf506))
* lambda functions for priority in rules
([#3253](#3253))
([d2aa226](d2aa226))
* Make on... directive of modules accessible
([#4050](#4050))
([e9f2e1c](e9f2e1c))


### Bug Fixes

* adjust conda tests to not fail on apple silicon; fix
[#4040](#4040)
([#4049](#4049))
([f5b0142](f5b0142))
* allow "--containerize apptainer" to output apptainer format instead of
dockerfile ([#4030](#4030))
([f5cac30](f5cac30))
* apptainer command not recognized when singularity is absent
([#4010](#4010))
([b8162e2](b8162e2))
* capture stderr when tests fail
([#3995](#3995))
([97d74ba](97d74ba))
* **docs:** make Data-dependent conditional execution a complete example
([#4043](#4043))
([3a1d7f2](3a1d7f2))
* don't build the DAG when running unlock. Fixes
[#4000](#4000) and
[#198](#198)
([#4007](#4007))
([acf79fd](acf79fd))
* Ensure pixi tasks may be run as advertised
([#4046](#4046))
([88253c2](88253c2))
* fix checkpoint handling corner cases
([#3870](#3870) and
[#3559](#3559))
([#4015](#4015))
([63f4257](63f4257))
* issue 3642
([#4054](#4054))
([76e6fc2](76e6fc2))
* issue 3815
([#4026](#4026))
([b0eec96](b0eec96))
* logging None in shellcmd context causes error
([#4064](#4064))
([d0652cd](d0652cd))
* lookup function returns default value for empty DataFrame queries
([#4056](#4056))
([f71de97](f71de97))
* make `cache: omit-software` a rule specific property
([#4085](#4085))
([034a9e7](034a9e7))
* reduce number of tests leaving temporary files behind
([#4033](#4033))
([a3a1c97](a3a1c97))
* regression in dynamic resource handling
([#4038](#4038))
([f2c554a](f2c554a))
* somewhat shorter announce message
([#4080](#4080))
([57efc71](57efc71))


### Performance Improvements

* switch reretry with tenacity; decouple container classes (with Python
3.7 compat for old scripts) from rest of the codebase (enabling moving
to newer python versions)
([#4032](#4032))
([ffb19e7](ffb19e7))


### Documentation

* Add AI-assisted contributions policy to contributing guidelines
([#4051](#4051))
([dd70526](dd70526))
* **codebase:** Update & simplify plugin architecture section
([#4052](#4052))
([176cf63](176cf63))
* Correct workflow.source_path() description in documentation
([#4036](#4036))
([45883c5](45883c5))
* fixed wrong code example for collect() function
([#4037](#4037))
([5c85ed8](5c85ed8))
* Minor docs improvements
([#4089](#4089))
([29ea226](29ea226))
* switch to sphinx_design for tabs
([#3976](#3976))
([9674614](9674614))
* typo in the migration table breaking a pip install command
([#4024](#4024))
([66f9dda](66f9dda))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Total resources increase with time

2 participants