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

API Support for Topology Visualization #9017

Closed
DingoEatingFuzz opened this issue Oct 2, 2020 · 6 comments · Fixed by #9055
Closed

API Support for Topology Visualization #9017

DingoEatingFuzz opened this issue Oct 2, 2020 · 6 comments · Fixed by #9055
Assignees
Labels
theme/api HTTP API and SDK issues
Milestone

Comments

@DingoEatingFuzz
Copy link
Contributor

It might be better to have this place to talk generally about the requirements of the topology visualization.

The API goals are to make as few requests as possible and download as many unneeded bits as possible. I guess these are always the goals of an API, but I digress.

Currently the topology visualization does this:

  1. Fetch /v1/nodes to get all nodes
  2. For each node, fetch /v1/node/:id to get the resources data (see Add Resources, ReservedResources, and Reserved to the /v1/nodes endpoint conditionally #8995)
  3. For each node, fetch /v1/node/:id/allocations to get allocations including resources data

This results in 2n + 1 requests.

This is what I would like to do:

  1. Fetch /v1/nodes?resources=true
  2. Fetch /v1/allocations?resources=true

This would be just the two requests.

If I'm dreaming big, there is still a lot of information here I'm not sure I need, such as task events and node events, but this is a good starting point.

@DingoEatingFuzz DingoEatingFuzz added the theme/api HTTP API and SDK issues label Oct 2, 2020
@DingoEatingFuzz DingoEatingFuzz added this to the 0.13 milestone Oct 2, 2020
@schmichael
Copy link
Member

These fields?

(Node.Resources and Node.Reserved are deprecated as of 0.9 although that could be made more clear.)

(Allocation.{Resources,SharedResources,TaskResources} are all deprecated although that could be made more clear.)

Sample Allocation.AllocatedResources JSON:

{
  "Tasks": {
    "redis": {
      "Cpu": {
        "CpuShares": 100
      },
      "Memory": {
        "MemoryMB": 300
      },
      "Networks": null,
      "Devices": null
    }
  },
  "TaskLifecycles": {
    "redis": null
  },
  "Shared": {
    "Networks": null,
    "DiskMB": 300,
    "Ports": null
  }
}

@DingoEatingFuzz
Copy link
Contributor Author

Good to know!

The UI currently uses Node.Resources, Node.Reserved, and Allocation.AllocationResources. Looks like we should update to use Node.NodeResources and Node.ReservedResources.

It's totally understandable if you don't want to drag the deprecated fields into this new endpoint variant so I can update what we're using as part of this.

schmichael added a commit that referenced this issue Oct 9, 2020
Fixes #9017

The ?resources=true query parameter includes resources in the object
stub listings. Specifically:

- For `/v1/nodes?resources=true` both the `NodeResources` and
  `ReservedResources` field are included.
- For `/v1/allocations?resources=true` the `AllocatedResources` field is
  included.
@schmichael
Copy link
Member

Does #9055 seem to meet your needs?

I fear /v1/allocations?resources=true may be quite expensive for users with large clusters. Some future improvements that might help:

  • ?terminal=false - only return DesiredStatus=running allocations. Same logic could be applied to Nodes.
  • ?task_states=false - strip task states from the response as they're often the largest part of the response.

In addition or alternatively you may want to continue doing n + 1 requests: /v1/nodes?resources=true + 1 node/allocs request per node.

You could limit the n requests to just the first "page" of node results (and perhaps prefetch another page or two). Obviously this limits visualizations and sorting to Node data which might be a nonstarter.

@DingoEatingFuzz
Copy link
Contributor Author

This meets my needs! I just updated my data fetching and verified it all.

I agree about the massiveness of the allocations payload, and both of those additional toggles would help--especially task states which can be quite large and (at least for the topo viz) I'm not even looking at.

This new n + 1 approach is interesting. It definitely makes sense to progressively download things in some manner, but I still get squeamish thinking about doing this for any cluster where n is > 100. That's a lot of requests in rapid succession.

The other other approach, if we don't want to pollute the public API with combinatorial bells and whistles, is to make this an "internal" "ui" endpoint. There's precedence for this in Vault and Consul already.

I don't like the UI getting VIP treatment, but it might be nice as a stop-gap while we figure out some API configurations we actually want to support long term.

@schmichael
Copy link
Member

Altering projection (the fields like task_states) is pretty easy on top of what I've built, so I'll add that.

Altering selection (which objects like terminal=false) is a new thing altogether, so I'm going to skip it for now. Feel free to open up a new issue if you think it's worth going down that path.

schmichael added a commit that referenced this issue Oct 14, 2020
Fixes #9017

The ?resources=true query parameter includes resources in the object
stub listings. Specifically:

- For `/v1/nodes?resources=true` both the `NodeResources` and
  `ReservedResources` field are included.
- For `/v1/allocations?resources=true` the `AllocatedResources` field is
  included.

The ?task_states=false query parameter removes TaskStates from
/v1/allocations responses. (By default TaskStates are included.)
fredrikhgrelland pushed a commit to fredrikhgrelland/nomad that referenced this issue Oct 22, 2020
Fixes hashicorp#9017

The ?resources=true query parameter includes resources in the object
stub listings. Specifically:

- For `/v1/nodes?resources=true` both the `NodeResources` and
  `ReservedResources` field are included.
- For `/v1/allocations?resources=true` the `AllocatedResources` field is
  included.

The ?task_states=false query parameter removes TaskStates from
/v1/allocations responses. (By default TaskStates are included.)
@github-actions
Copy link

github-actions bot commented Nov 1, 2022

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
theme/api HTTP API and SDK issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants