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

Support adjusting GIL monitoring interval #7650

Merged
merged 4 commits into from
Mar 15, 2023

Conversation

milesgranger
Copy link
Contributor

@milesgranger milesgranger commented Mar 14, 2023

Augment the configuration schema to support
adjusting the GIL monitoring interval.

Part of #7290

  • Tests added / passed
  • Passes pre-commit run --all-files

Adjust schema to support enabling and interval value
@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 2023

Unit Test Results

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

       26 files  ±    0         26 suites  ±0   12h 17m 4s ⏱️ - 29m 31s
  3 532 tests +  30    3 426 ✔️ +  27     103 💤 ±0  3 +3 
44 656 runs  +390  42 579 ✔️ +382  2 074 💤 +5  3 +3 

For more details on these failures, see this check.

Results for commit 27b7eb8. ± Comparison against base commit 6cab0e2.

This pull request removes 3 and adds 33 tests. Note that renamed tests count towards both.
distributed.http.worker.tests.test_worker_http ‑ test_prometheus_resets_max_metrics
distributed.tests.test_worker_memory ‑ test_digests
distributed.tests.test_worker_state_machine ‑ test_slots[DigestMetric]
distributed.deploy.tests.test_cluster ‑ test_exponential_backoff
distributed.tests.test_metrics ‑ test_context_meter
distributed.tests.test_metrics ‑ test_context_meter_decorator
distributed.tests.test_metrics ‑ test_context_meter_nested
distributed.tests.test_metrics ‑ test_context_meter_nested_floor
distributed.tests.test_metrics ‑ test_context_meter_pickle
distributed.tests.test_metrics ‑ test_context_meter_raise
distributed.tests.test_metrics ‑ test_delayed_metrics_ledger
distributed.tests.test_metrics ‑ test_meter
distributed.tests.test_metrics ‑ test_meter_floor[kwargs0-0]
…

♻️ This comment has been updated with latest results.

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @milesgranger

distributed/distributed.yaml Outdated Show resolved Hide resolved
distributed/distributed.yaml Outdated Show resolved Hide resolved
@milesgranger milesgranger requested a review from fjetter as a code owner March 15, 2023 14:17
Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @milesgranger! We're now getting

distributed/tests/test_system_monitor.py::test_gil_contention - TypeError: KnockKnock.__new__() got an unexpected keyword argument 'polling_interval_micros'

in CI. My guess is we need to set a pin on the version of gilknocker that gets installed

@jrbourbeau
Copy link
Member

Other than the related test failure, this PR looks good

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @milesgranger -- will merge after CI is done

@jrbourbeau jrbourbeau merged commit 0a88b76 into dask:main Mar 15, 2023
ypogorelova pushed a commit to ypogorelova/distributed that referenced this pull request Mar 16, 2023
@milesgranger milesgranger deleted the MINOR-adjust-gil-contention-config branch March 16, 2023 08:20
milesgranger added a commit to milesgranger/distributed that referenced this pull request Mar 16, 2023
milesgranger added a commit to milesgranger/distributed that referenced this pull request Mar 21, 2023
milesgranger added a commit to milesgranger/distributed that referenced this pull request Apr 3, 2023
milesgranger added a commit to milesgranger/distributed that referenced this pull request Apr 11, 2023
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.

2 participants