Stable Archive on lore.kernel.org
 help / color / Atom feed
From: Dave Chiluk <chiluk+linux@indeed.com>
To: Greg KH <greg@kroah.com>, Ben Segall <bsegall@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Phil Auld <pauld@redhat.com>
Cc: stable@vger.kernel.org
Subject: Re: Please backport de53fd7aedb1 : sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
Date: Fri, 27 Sep 2019 01:12:40 -0500
Message-ID: <CAC=E7cXcujmbwMnmXeH2=80Lkki+j_b=WE4KCWaM1mYafDaWSA@mail.gmail.com> (raw)
In-Reply-To: <20190925064414.GA1449297@kroah.com>

On Wed, Sep 25, 2019 at 1:44 AM Greg KH <greg@kroah.com> wrote:
>
> On Wed, Sep 25, 2019 at 12:53:48AM -0500, Dave Chiluk wrote:
> > Commit de53fd7aedb1 ("sched/fair: Fix low cpu usage with high
> > throttling by removing expiration of cpu-local slices") fixes a major
> > performance issue for containerized clouds such as Kubernetes.
> >
> > Commit de53fd7aedb1 Fixes commit : 512ac999d275 ("sched/fair: Fix
> > bandwidth timer clock drift condition").
> >
> > This should be applied to all stable kernels that applied commit
> > 512ac999d275, and should probably be applied to all others as well.
>
> As this commit isn't in a released kernel just yet, we should wait to
> see what happens when it hits people's machines, right?

I think waiting till 5.4 is released would be irresponsible on this
one.  512ac999 was recently pushed back into many of the distro
kernels via the stable streams.  Additionally every major container
running cloud provider (public and private), is likely hitting this
whether they've noticed or not.  Before 512ac999 we would hit the
issue that resolved once every 10000 application deployments or so *(I
admit some providers appear to have been hit by that more often).
However, the fix implemented by 512ac999 is guaranteed to affect 100%
of cgroup cpu bandwidth constrained applications.

Our java applications are particularly hard hit as java tends to be
very thread happy.   Doing some napkin math, given the roughly 9000
applications we have running in our clouds we've had to over-allocate
each one's CPU quota by roughly .5 cpu to account for this behavior
change (worst case scenario is actually .01 cpu * cores in machine,
but not all of our applications are affected equally).
9000 applications * .5 CPU = 4500 cores
4500 Cores / 88 cores per node = 51 additional machines required to
satisfy the inflated quota requirements.
Given each 88 core machine costs roughly $30k that equates to $1.5M
that this issue is costing us.
We have been able to alleviate this somewhat of this by turning on CPU
overcommit on in the container scheduler. (Deploying more applications
per host than the CPU would generally allow for).  This however leads
to occasional overloaded machines, and regular high response
times/wide response time distributions for applications.

If you have a java or golang application running on a containerized
cloud anywhere chances are you are either paying extra to get the CPU
you need or your application is being throttled before using the quota
you've paid for.  I'm sure other languages could be affected, but
these are the two we've seen that by default generate enough threads
to regularly hit this issue.

All of these problems are leading the kubernetes community to change
the defaulrs away from the cfs bandwidth scheduling mechanisms and
towards other mechanisms or hackish workarounds. *(adjusting periods,
turning off cfs bandwidth, moving to only soft limits).
https://github.com/kubernetes/kubernetes/issues/67577
https://github.com/kubernetes/kubernetes/issues/70585
https://github.com/kubernetes/kubernetes/issues/51135

I appreciate your reluctance to accept this patch, but I strongly
believe pulling this sooner than later is the right thing to do.

> Also, always cc: all of the people involved in the patch you are asking
> for, so as to get their opinion.
Fixed.  I'll submit a change to stable-kernel-rules.rst upon my return
from vacation.

> For some reason this patch did not
> have a cc: stable tag on it, was that because the developers did not
> think it was relevant for stable kernels?

This was my fault, I was unaware I could simply cc: stable to have it
auto-submitted.  I promise I'll be better in the future.

> > It may also be prudent to also apply the not yet accepted patch that
> > fixes some introduced compiler warnings discussed here.
> > https://lkml.org/lkml/2019/9/18/925
>
> So we should wait for this to hit Linus's tree too, right?

This patch contributes nothing materially to de53fd7aedb1, and only
eliminates compiler warnings for unused variables *(which likely get
optimized out anyway).  I'm not sure what the stable policy is with
introduced compiler warnings, so I included the patch submission in
this discussion for completeness.

Thank you,
Dave Chiluk

  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-25  5:53 Dave Chiluk
2019-09-25  6:44 ` Greg KH
2019-09-27  6:12   ` Dave Chiluk [this message]
2019-09-27  6:24     ` Greg KH
2019-09-27 13:13     ` Sasha Levin
2019-10-03  5:15       ` Dave Chiluk
2019-10-03  6:51         ` Greg KH
2019-10-18 20:23           ` Dave Chiluk
2019-10-18 20:53             ` Peter Zijlstra
2019-10-22 14:55               ` Dave Chiluk
2019-11-04 11:08                 ` Greg KH
2019-11-08 18:06                   ` [PATCH v4.14.y 1/2] " Dave Chiluk
2019-11-08 20:15                   ` [PATCH v4.14.y 0/2] Please backport de53fd7aedb1 : " Dave Chiluk
2019-11-08 20:15                     ` [PATCH v4.14.y 1/2] " Dave Chiluk
2019-11-08 20:15                     ` [PATCH v4.14.y 2/2] sched/fair: Fix -Wunused-but-set-variable warnings Dave Chiluk
2019-11-11  9:18                     ` [PATCH v4.14.y 0/2] Please backport de53fd7aedb1 : sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices Greg KH
2019-11-08 20:20                   ` [PATCH v4.19.y " Dave Chiluk
2019-11-08 20:20                     ` [PATCH v4.19.y 1/2] " Dave Chiluk
2019-11-08 20:20                     ` [PATCH v4.19.y 2/2] sched/fair: Fix -Wunused-but-set-variable warnings Dave Chiluk
2019-11-11  9:19                     ` [PATCH v4.19.y 0/2] Please backport de53fd7aedb1 : sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices Greg KH

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAC=E7cXcujmbwMnmXeH2=80Lkki+j_b=WE4KCWaM1mYafDaWSA@mail.gmail.com' \
    --to=chiluk+linux@indeed.com \
    --cc=bsegall@google.com \
    --cc=greg@kroah.com \
    --cc=pauld@redhat.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git