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

enable pressure stall kernel config by default #3517

Closed
dschaaff opened this issue Oct 10, 2023 · 4 comments
Closed

enable pressure stall kernel config by default #3517

dschaaff opened this issue Oct 10, 2023 · 4 comments
Assignees
Labels
area/settings Issues related to our settings handling status/research This issue is being researched type/enhancement New feature or request

Comments

@dschaaff
Copy link

What I'd like:

I'd like pressure stall information to be enabled by default so that I can use psi metrics for our workloads.

CONFIG_PSI_DEFAULT_DISABLED=y currently disables psi by default.

Any alternatives you've considered:

I know we can use the below config and I have verified it enables psi, however I don't want to accept the small increase boot time. A big goal of ours is to keep boot time as low as possible. Based on a very unscientific review of the logs it appears this reboot adds an additional 10s or more to the time for the node to boot and start the kubelet.

    [settings.boot]
    reboot-to-reconcile = true
    [settings.boot.kernel-parameters]
    "psi" = ["1"]
@dschaaff dschaaff added status/needs-triage Pending triage or re-evaluation type/enhancement New feature or request labels Oct 10, 2023
@gthao313
Copy link
Member

@dschaaff Thanks for opening this issue! We will research on it and reach back to you. Thanks.

@gthao313 gthao313 added type/support User support related issues. area/settings Issues related to our settings handling status/icebox Things we think would be nice but are not prioritized and removed type/support User support related issues. status/needs-triage Pending triage or re-evaluation labels Oct 17, 2023
@foersleo
Copy link
Contributor

Hi @dschaaff,

I have had a look at the PSI stuff and here is some of my thoughts towards it.

The PSI accounting is not without overhead. That is the main reason why the kernel provides the ability to disable PSI by default through the mentioned config option. Extra code is run at every sleep/wakeup code paths of the scheduler, which seems scary and hints at default disable is the right way to go. However, As far as I understand the impact of that overhead is of an academic nature and can be seen on micro benchmarks but has yet to be proven to impact real-world workloads.

With all of these trade-off it comes down to some preferring one option over the other. With default disabling we are on the safe side of not introducing overhead to those that may be paranoid and do not use PSI, but are making the lives of those that want to use it a bit harder. The other way around we may make your life easier, but need to have others to disable it explicitly. So which side do we pick?

That being said more recent developments have also included some optimizations for the PSI code that recouped some of the overhead and made these code paths more efficient. With our variants that ship with kernel 6.1 we are in fact not disabling PSI by default. You can use it today without the overhead of the extra reboot by using variants aws-ecs-2*, *-k8s-1.28* as well as the *-dev variants. Please let us know if that is an option for you to unblock your use-case.

In the meantime I will have a look at the overhead on the older kernel versions and if the improvements may have been made available to the 5.10 and 5.15 kernel series. That may inform a decision on changing the default for variants with these kernels.

@foersleo foersleo self-assigned this Oct 20, 2023
@foersleo foersleo added status/research This issue is being researched and removed status/icebox Things we think would be nice but are not prioritized labels Oct 20, 2023
@dschaaff
Copy link
Author

Thanks for the thorough response! We've found the pressure stall metrics valuable for a number of database workloads we run on ec2 directly. I'm glad to hear it is enabled for variants with the 6.1 kernel. We are currently on Kubernetes 1.27 with plans to upgrade to 1.28 beginning of next year. I'm content waiting until then to get the pressure stall info. If it happens to get enabled on the 5.15 kernel versions I'll consider that a treat for me :)

@foersleo
Copy link
Contributor

So I have had a look at the improvements that were introduced with the 6.1 kernel through this pull request. Given that the improvements are non-trivial in the 4-9% range and these patches have not been back-ported to any earlier series, I am quite opposed to just enable it by default on the older kernels.

Even though, I have not found reports of the impact of older PSI implementations on real-world workloads, there may be users that are sensitive to these kind of things and that would be in for an unpleasant surprise when just doing a minor update. So we will not change the default for 5.10 and 5.15 based variants to be on the safe side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/settings Issues related to our settings handling status/research This issue is being researched type/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants