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

dynamic host volumes: node selection via constraints #24518

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

tgross
Copy link
Member

@tgross tgross commented Nov 20, 2024

When making a request to create a dynamic host volume, users can pass a node pool and constraints instead of a specific node ID.

This changeset implements a node scheduling logic by instantiating a filter by node pool and constraint checker borrowed from the scheduler package. Because host volumes with the same name can't land on the same host, we don't need to support distinct_hosts/distinct_property; this would be challenging anyways without building out a much larger node iteration mechanism to keep track of usage across multiple hosts.

Ref: #24479

Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

LGTM! with one long thought about potential disk utilization

nomad/host_volume_endpoint_test.go Outdated Show resolved Hide resolved
}

for {
raw := iter.Next()
Copy link
Member

Choose a reason for hiding this comment

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

I don't recall how our binpacking algorithm works for allocs. is it like this, where it's just whatever order comes out of state? I suspect, based on no real evidence, that folks won't want to binpack volumes the same way, unless they are the kind of volume that has a disk space limit, and we placed them based on available disk space.

basically, if I'm reading this right, this feels like a recipe for a full disk alert waking someone up.

I suppose their main mechanisms to avoid this would be to

  • use careful explicit constraints, which seems a little IAC-unfriendly, if they'd need a lot of specs?
  • reuse the same vol name a lot, so each instance lands on a distinct host

any other considerations I'm missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

For allocs in the general scheduler (batch/service), we:

  • find all the nodes in the node pool and DC
  • shuffle them
  • iterate over them until we find 2 that are feasible (or a lot more than 2 for jobs with spread)
  • pick the best of 2

When using spread, we iterate over enough nodes to guarantee we're not putting allocs for the same job on the same host, which is effectively what we're doing here. Operators are going to want to spread volumes with the same "purpose" out because of failure domains. If the node is full, then the plugin will tell us that and we'll get an error back.

@tgross tgross force-pushed the dynamic-host-volumes branch from a3784ca to 83de356 Compare November 20, 2024 22:04
@tgross tgross requested review from a team as code owners November 20, 2024 22:04
When making a request to create a dynamic host volumes, users can pass a node
pool and constraints instead of a specific node ID.

This changeset implements a node scheduling logic by instantiating a filter by
node pool and constraint checker borrowed from the scheduler package. Because
host volumes with the same name can't land on the same host, we don't need to
support `distinct_hosts`/`distinct_property`; this would be challenging anyways
without building out a much larger node iteration mechanism to keep track of
usage across multiple hosts.

Ref: #24479
@tgross tgross force-pushed the dhv-rpc-volume-placement branch from 7a6cecb to b2b39b8 Compare November 20, 2024 22:07
@tgross tgross merged commit e28f99a into dynamic-host-volumes Nov 21, 2024
17 checks passed
@tgross tgross deleted the dhv-rpc-volume-placement branch November 21, 2024 14:28
tgross added a commit that referenced this pull request Dec 2, 2024
When making a request to create a dynamic host volumes, users can pass a node
pool and constraints instead of a specific node ID.

This changeset implements a node scheduling logic by instantiating a filter by
node pool and constraint checker borrowed from the scheduler package. Because
host volumes with the same name can't land on the same host, we don't need to
support `distinct_hosts`/`distinct_property`; this would be challenging anyways
without building out a much larger node iteration mechanism to keep track of
usage across multiple hosts.

Ref: #24479
tgross added a commit that referenced this pull request Dec 3, 2024
When making a request to create a dynamic host volumes, users can pass a node
pool and constraints instead of a specific node ID.

This changeset implements a node scheduling logic by instantiating a filter by
node pool and constraint checker borrowed from the scheduler package. Because
host volumes with the same name can't land on the same host, we don't need to
support `distinct_hosts`/`distinct_property`; this would be challenging anyways
without building out a much larger node iteration mechanism to keep track of
usage across multiple hosts.

Ref: #24479
tgross added a commit that referenced this pull request Dec 9, 2024
When making a request to create a dynamic host volumes, users can pass a node
pool and constraints instead of a specific node ID.

This changeset implements a node scheduling logic by instantiating a filter by
node pool and constraint checker borrowed from the scheduler package. Because
host volumes with the same name can't land on the same host, we don't need to
support `distinct_hosts`/`distinct_property`; this would be challenging anyways
without building out a much larger node iteration mechanism to keep track of
usage across multiple hosts.

Ref: #24479
tgross added a commit that referenced this pull request Dec 13, 2024
When making a request to create a dynamic host volumes, users can pass a node
pool and constraints instead of a specific node ID.

This changeset implements a node scheduling logic by instantiating a filter by
node pool and constraint checker borrowed from the scheduler package. Because
host volumes with the same name can't land on the same host, we don't need to
support `distinct_hosts`/`distinct_property`; this would be challenging anyways
without building out a much larger node iteration mechanism to keep track of
usage across multiple hosts.

Ref: #24479
tgross added a commit that referenced this pull request Dec 19, 2024
When making a request to create a dynamic host volumes, users can pass a node
pool and constraints instead of a specific node ID.

This changeset implements a node scheduling logic by instantiating a filter by
node pool and constraint checker borrowed from the scheduler package. Because
host volumes with the same name can't land on the same host, we don't need to
support `distinct_hosts`/`distinct_property`; this would be challenging anyways
without building out a much larger node iteration mechanism to keep track of
usage across multiple hosts.

Ref: #24479
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants