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

Add I/O Priority Configuration for process group in Linux Containers #1191

Merged
merged 1 commit into from
May 22, 2023

Conversation

utam0k
Copy link
Member

@utam0k utam0k commented Mar 23, 2023

Description

This PR introduces a new configuration option to set the I/O priority for the entire process group within a Linux container. By allowing users to configure I/O priority, we enable better resource management and performance tuning for various workloads, particularly in Kubernetes environments where multiple containers may be running concurrently.

One specific use case is when running batch processing jobs alongside application Pods in a Kubernetes cluster. Batch jobs are often I/O-intensive and can cause resource contention, negatively affecting the performance of other containers running on the same node. By allowing I/O priority configuration for each container, cluster administrators can minimize the impact of I/O-intensive batch jobs on application Pods.

By adding I/O priority configuration to the OCI Runtime spec, we enable Kubernetes (and other container orchestration platforms) to take advantage of this feature and better manage container I/O resources. This can lead to improved overall performance and resource utilization on nodes running multiple containers with different I/O requirements.

Implement

This feature can be implemented using the ioprio_set system call on Linux, which allows setting the I/O priority for a given process or process group. In Go, the syscall package can be used to invoke the ioprio_set system call. A sample implementation in Go could look like this: https://go.dev/play/p/KsscUN8RITD

@utam0k utam0k changed the title Add I/O Priority Configuration for Process Group in Linux Containers Add I/O Priority Configuration for process group in Linux Containers Mar 23, 2023
@AkihiroSuda
Copy link
Member

@utam0k
Copy link
Member Author

utam0k commented Mar 23, 2023

@AkihiroSuda Thanks for your quick review 😻

https://github.com/opencontainers/runtime-spec/blob/main/schema/config-linux.json has to be updated too.

Thanks, I'll update it later.

I'd like to see a POC of runc, crun, or youki before merging this

I might try this against youki in my personal repository. This will take some time, so you'll have to wait.
I'd like to hear the maintainer's first impressions of the proposal. If it's not worth it from the start, there's no point in having a prototype implementation.

@utam0k
Copy link
Member Author

utam0k commented Mar 24, 2023

@AkihiroSuda I have implemented it to runc.
opencontainers/runc#3783

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

@utam0k
Copy link
Member Author

utam0k commented Mar 26, 2023

Please update https://github.com/opencontainers/runtime-spec/blob/main/schema/config-linux.json

@AkihiroSuda
I completely forgot about it 🤦I have updated it.

@AkihiroSuda AkihiroSuda added this to the v1.1.0 milestone Mar 28, 2023
@AkihiroSuda AkihiroSuda mentioned this pull request Mar 28, 2023
@AkihiroSuda
Copy link
Member

@opencontainers/runtime-spec-maintainers PTAL

Comment on lines 147 to 155
"ioPriority": {
"class": "string",
"priority": "integer"
},
Copy link
Member

Choose a reason for hiding this comment

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

should it be an enumeration of the supported values?

Copy link
Member Author

Choose a reason for hiding this comment

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

@giuseppe Thanks for your review. It would be better! I have fixed it.

@AkihiroSuda AkihiroSuda requested a review from giuseppe May 19, 2023 21:03
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda AkihiroSuda merged commit 8e0dce8 into opencontainers:main May 22, 2023
@AkihiroSuda AkihiroSuda mentioned this pull request May 22, 2023
12 tasks
Comment on lines +147 to +155
"ioPriority": {
"class": "string",
"enum": [
"IOPRIO_CLASS_RT",
"IOPRIO_CLASS_BE",
"IOPRIO_CLASS_IDLE"
],
"priority": "integer"
},
Copy link
Member

Choose a reason for hiding this comment

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

I think the enum is defined in the wrong location, because this enum would now apply to ioPriority as a whole, but the enum defines values for class only.

So class should probably be defined as "#/definitions/IOPriorityClass" (e.g.) instead of a "string". Similar to

"$ref": "#/definitions/SeccompOperators"

"SeccompOperators": {
"type": "string",
"enum": [
"SCMP_CMP_NE",
"SCMP_CMP_LT",
"SCMP_CMP_LE",
"SCMP_CMP_EQ",
"SCMP_CMP_GE",
"SCMP_CMP_GT",
"SCMP_CMP_MASKED_EQ"
]
},

Copy link
Member

Choose a reason for hiding this comment

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

Oh! I think #1206 was meant to address that, correct @giuseppe ?

@AkihiroSuda AkihiroSuda mentioned this pull request Jun 26, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants