Skip to content

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

Merged
crusaderky merged 10 commits intodask:mainfrom
graingert:prometheus-prefix-durations
Jan 23, 2023
Merged

add prometheus metric for time and memory used per task prefix#7406
crusaderky merged 10 commits intodask:mainfrom
graingert:prometheus-prefix-durations

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 force-pushed the prometheus-prefix-durations branch from 04ff2d1 to 127bdab Compare December 14, 2022 13:42
@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
mrocklin
mrocklin reviewed Dec 14, 2022
View reviewed changes
crusaderky
crusaderky requested changes Dec 14, 2022
View reviewed changes
graingert
graingert commented Dec 14, 2022
View reviewed changes
@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 force-pushed the prometheus-prefix-durations branch from 29b72e4 to 74a9765 Compare January 5, 2023 15:06
@graingert graingert marked this pull request as ready for review January 5, 2023 16:30
ntabris
ntabris reviewed Jan 20, 2023
View reviewed changes
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

@crusaderky crusaderky Jan 21, 2023

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

crusaderky commented Jan 22, 2023

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

@mrocklin mrocklin mrocklin left review comments

@crusaderky crusaderky crusaderky approved these changes

+1 more reviewer

@ntabris ntabris ntabris left review comments

Reviewers whose approvals may not affect merge requirements

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