Re: [RFC PATCH] fsnotify: raise default fs.inotify.max_user_instances now that memcg accounts for it

[Jan Kara]

Hello!

On Wed 12-08-26 20:51:36, Harsh Upadhayay wrote:
> I hit this running a 3-node k3s homelab cluster: every container on a
> node shares one 128-slot fs.inotify.max_user_instances pool (no user
> namespace remapping), kubelet alone was holding 25-40+ instances just
> from ConfigMap/Secret volume watching, and the pool emptied under
> normal load -- surfaced as an unhelpful, inotify-silent "too many open
> files" in whichever container's turn it was to ask next. Filing as
> RFC since I couldn't find prior discussion of raising this default.

Not sure what Amir thinks about this but I think default is just a default
and it is natural it is not good for everybody. I can see how 128 inotify
instances may not be enough for Kubernetes setups but then the answer is
for Kubernetes installation to configure maximum inotify instances to a
higher value. And as you show below, different container runtimes tune the
value differently which just proves my point that whatever value we choose
people will need to tune it - which is the point of this limit being tunable
after all.

So I don't see a strong enough reason to bump up the default limit because
there can be other users for which increased limit could allow a potential
DoS in their setups. The concern isn't as much the memory consumed by
inotify instances themselves as you write below but more all the events
that can get queued to each instance. OTOH those are accounted to the
instance's memcg so your overall argument that in memcg enabled setups the
limits aren't that much needed is still somewhat valid.

> fs.inotify.max_user_instances has been a flat 128 since inotify's
> introduction in 2005. That number is not per-container or per-workload
> -- it is a single pool shared by every process running under a given
> real UID on the host. On a multi-tenant Linux box, that means every
> container's root process shares one 128-slot pool with every other
> unremapped container's root, plus the host's own kubelet/containerd
> (or equivalent) bookkeeping, since user-namespace UID remapping is
> off by default in essentially every mainstream container runtime.
> 
> In practice this pool empties well before 128 "real" watchers are
> running, because kubelet alone routinely holds 25-40+ instances on a
> modestly loaded node just from tracking projected ConfigMap/Secret
> volumes, before any application container asks for one. Once the pool
> is empty, the next process on the node to call inotify_init() anywhere
> gets EMFILE, surfaced by most userspace libraries as a generic "too
> many open files" -- with no mention of inotify at all, making the
> underlying cause hard to diagnose from the error text alone.

Well, if you read manpage of inotify_init() the meaning of EMFILE is
explained there so I'm not sure how we can make that more obvious.

> This is not a new observation. It has been independently hit and
> worked around downstream, at the deployment-tooling layer rather than
> the kernel default, at least by:
> - Red Hat/OpenStack (dnsmasq-per-subnet exhausting the pool at scale;
> fixed only in tripleo-heat-templates, defaulting to 1024) [1]
> - NixOS containers (~20 containers sufficient to exhaust it) [2]
> - Docker, which raises its own daemon default to 8192 [3]

This is not a workaround, this is how things are expected to work - if you
find the limit is too low for your usecase, increase the limit. This is an
administrative decision of the sysadmin. And yes, I'm well aware of the
pain with tuning thousand and one various parameters to make your setup
work so these days we try to design things so that there are less tunables
and things work out of the box. But that was not the case of inotify and so
far I don't see that the risk of increasing the default would outweight the
benefit.
								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR