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

Avoid sorting the queued heap #8238

Merged
merged 3 commits into from Oct 9, 2023

Conversation

fjetter
Copy link
Member

@fjetter fjetter commented Oct 6, 2023

I saw this crippling CPU on the scheduler for cases where there are many tasks queued (>250k). I haven't thoroughly tested this but I don't see the need to sort. I also believe there is no need to transition all at once since individual task chains would be processed before the next queued tasks anyhow, i.e. this should not affect ordering, I believe.

Generally, the sort does not truly help us here and we have to pop the heap for every task individually anyhow.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       20 files   -        1         20 suites   - 1   10h 31m 47s ⏱️ + 4m 26s
  3 849 tests +     13    3 739 ✔️ +     15     107 💤 ±  0    3  - 2 
35 828 runs   - 1 243  34 084 ✔️  - 1 177  1 734 💤  - 70  10 +4 

For more details on these failures, see this check.

Results for commit 1a661a4. ± Comparison against base commit de3f755.

This pull request removes 9 and adds 22 tests. Note that renamed tests count towards both.
distributed.tests.test_worker ‑ test_conda_install
distributed.tests.test_worker ‑ test_conda_install_fails_on_returncode
distributed.tests.test_worker ‑ test_conda_install_fails_when_conda_not_found
distributed.tests.test_worker ‑ test_conda_install_fails_when_conda_raises
distributed.tests.test_worker ‑ test_package_install_failing_does_not_restart_on_nanny
distributed.tests.test_worker ‑ test_package_install_installs_once_with_multiple_workers
distributed.tests.test_worker ‑ test_package_install_restarts_on_nanny
distributed.tests.test_worker ‑ test_pip_install
distributed.tests.test_worker ‑ test_pip_install_fails
distributed.diagnostics.tests.test_packageinstall ‑ test_conda_install
distributed.diagnostics.tests.test_packageinstall ‑ test_conda_install_fails_on_returncode
distributed.diagnostics.tests.test_packageinstall ‑ test_conda_install_fails_when_conda_not_found
distributed.diagnostics.tests.test_packageinstall ‑ test_conda_install_fails_when_conda_raises
distributed.diagnostics.tests.test_packageinstall ‑ test_package_install_failing_does_not_restart_on_nanny
distributed.diagnostics.tests.test_packageinstall ‑ test_package_install_installs_once_when_reregistered
distributed.diagnostics.tests.test_packageinstall ‑ test_package_install_installs_once_with_multiple_workers
distributed.diagnostics.tests.test_packageinstall ‑ test_package_install_restarts_on_nanny
distributed.diagnostics.tests.test_packageinstall ‑ test_pip_install
distributed.diagnostics.tests.test_packageinstall ‑ test_pip_install_fails
…

♻️ This comment has been updated with latest results.

@fjetter fjetter marked this pull request as ready for review October 6, 2023 15:12
@crusaderky crusaderky self-requested a review October 9, 2023 13:29
Copy link
Collaborator

@crusaderky crusaderky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!
Only a small comment.

distributed/scheduler.py Show resolved Hide resolved
Co-authored-by: crusaderky <crusaderky@gmail.com>
@crusaderky crusaderky merged commit 33689af into dask:main Oct 9, 2023
16 of 28 checks passed
@fjetter fjetter deleted the avoid_sorting_queued_heap branch October 10, 2023 09:12
@fjetter fjetter mentioned this pull request Oct 13, 2023
8 tasks
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.

None yet

2 participants