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

ui: Add more exposure of memory_max #10508

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Conversation

backspace
Copy link
Contributor

@backspace backspace commented May 4, 2021

This is a continuation of #10268, focusing on exposing information via memory metrics charts.

When memory_max is configured for a task and oversubscription is enabled, the allocation response’s AllocatedResources.Tasks[].MemoryMaxMB (which maps to allocation.states[].resources.memoryMax in Ember Data representations) will be non-zero, and when it exceeds ….resources.memory, a soft limit annotation shows on the task’s memory primary metric chart:

annotation-task

Similarly, it shows on the allocation memory primary metric chart for the aggregated tasks:

annotation-allocation

While working on this, I decided that it didn’t make sense to show the configured maximum in the task group details ribbon as I added in #10268 because it would always show, even when oversubscription wasn’t enabled. AllocatedResources is the way to know the discrepancy between configured and actual limits, so it’s not possible to only show the maximum on the task group details page when oversubscription is enabled, as task groups can’t know their “reified” (?) resources.

task-group-lack

@backspace backspace added this to the 1.1.0 milestone May 4, 2021
@backspace backspace self-assigned this May 4, 2021
@github-actions
Copy link

github-actions bot commented May 4, 2021

Ember Asset Size action

As of 6ba1052

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +2.71 kB +253 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.js 0 B 0 B
nomad-ui.css 0 B 0 B
vendor.css 0 B 0 B

@github-actions
Copy link

github-actions bot commented May 4, 2021

Ember Test Audit comparison

main 6ba1052 change
passes 1079 1077 -2
failures 0 2 +2
flaky 0 2 +2
duration 8m 46s 805ms 000ms -8m 46s 805ms

backspace added 2 commits May 4, 2021 15:43
This appears to have no tests…? 🤔 Or everything is broken
locally, which is sadly not unlikely
backspace added 4 commits May 18, 2021 14:25
It’s preferable to have it stored as 0 so it can be known
that oversubscription is disabled.
Is `this.tracker.reservedMemoryMax` even valid? 🤔
@github-actions
Copy link

Ember Test Audit flaky tests

Ember Test Audit detected these flaky tests on b43b87f:

  • Acceptance | client detail: each allocation should have high-level details for the allocation
  • Acceptance | client detail: each allocation should link to the allocation detail page

This is a reversion from #10459, more background here:
#10268 (comment)
This shouldn’t assume a maximum is present. Maybe it should be
conditional…? 🧐
@github-actions
Copy link

Ember Test Audit flaky tests

Ember Test Audit detected these flaky tests on 6ba1052:

  • Acceptance | plugin detail: each allocation should have high-level details for the allocation
  • Acceptance | volume detail: each allocation should have high-level details for the allocation

@tgross tgross assigned ChaiWithJai and unassigned backspace Jun 3, 2021
@tgross tgross removed this from the 1.1.1 milestone Jun 3, 2021
@tgross
Copy link
Member

tgross commented Jun 3, 2021

@JBhagat841 I'm assigning this to you just so that we don't lose track of open PRs from Buck, but there's no special rush on it other than that we should try to ship it in the 1.1.x cycle.

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@philrenaud philrenaud removed their assignment Jul 4, 2022
@ChaiWithJai ChaiWithJai removed their assignment Aug 4, 2022
@tgross tgross added the stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows theme/ui type/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants