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.

Sign up for GitHub

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

Jump to bottom

add prometheus metric for time and memory used per task prefix #7406

Merged
merged 10 commits into from
Jan 23, 2023
Merged

add prometheus metric for time and memory used per task prefix #7406

merged 10 commits into from
Jan 23, 2023

Conversation

graingert
Copy link
Member

@graingert graingert commented Dec 14, 2022 •
edited
Loading

closes #7341

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

@graingert graingert changed the title add prometheus metrics for time used per task prefix add prometheus metric for time used per task prefix Dec 14, 2022
@graingert graingert force-pushed the prometheus-prefix-durations branch 2 times, most recently from e76ec9c to bc06bc9 Compare December 14, 2022 16:19
@github-actions
Copy link
Contributor

github-actions bot commented Dec 14, 2022 •
edited
Loading

Unit Test Results

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

       24 files  ±0         24 suites  ±0   10h 2m 26s ⏱️ - 8m 31s
  3 327 tests ±0    3 221 ✔️ ±0     105 💤 ±0  1 ❌ ±0 
39 228 runs  ±0  37 338 ✔️ ±0  1 889 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit b8f554d. ± Comparison against base commit fae59c4.

♻️ This comment has been updated with latest results.

@graingert graingert force-pushed the prometheus-prefix-durations branch 2 times, most recently from 6e488b0 to dcd4fcc Compare January 4, 2023 15:43
@graingert graingert changed the title add prometheus metric for time used per task prefix add prometheus metric for time and memory used per task prefix Jan 5, 2023
@graingert graingert force-pushed the prometheus-prefix-durations branch 2 times, most recently from 7cf3097 to 29b72e4 Compare January 5, 2023 12:47
@graingert graingert marked this pull request as ready for review January 5, 2023 16:30
ntabris
ntabris reviewed Jan 20, 2023
View reviewed changes
docs/source/prometheus.rst Outdated Show resolved Hide resolved
crusaderky
crusaderky reviewed Jan 21, 2023
View reviewed changes
docs/source/prometheus.rst Outdated
Total time (per prefix) spent computing tasks
dask_scheduler_tasks_transfer_seconds_total
Total time (per prefix) spent transferring
dask_scheduler_tasks_output_bytes_total
Copy link
Collaborator

Choose a reason for hiding this comment

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

I changed the labels.
This makes me think that it could be valuable to measure the input bytes as well, so that we can get information about compression / expansion points in the graph.

@crusaderky
Copy link
Collaborator

TaskPrefix.nbytes_total is the current size in memory, not a cumulative history. I changed the metric to a Gauge.

@crusaderky crusaderky merged commit 641d472 into dask:main Jan 23, 2023
@gjoseph92 gjoseph92 mentioned this pull request Feb 28, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntabris ntabris ntabris left review comments

@crusaderky crusaderky crusaderky approved these changes

@mrocklin mrocklin mrocklin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prometheus metric for memory/time used per task prefix
4 participants
@graingert @crusaderky @mrocklin @ntabris