[PATCH] winbindd: control number of winbindd's client connections

[Uri Simchoni]

Hi,

Attached pls find V4 of this patch, handling the case where winbindd
runs out of file descriptors during heavy load. This patch takes a
different approach suggested by Volker:
1. The assumption is that we can estimate a limit on the number of
winbindd clients (estimate number of processes using winbindd, at most
1 client per process).
2. This limit should be configured into "winbind max clients"
3. Winbindd shall modify its fd limit accordingly
4. Add code that closes the winbindd side of the socket immediately if
the client closes the socket (up until now, while a request was queued
or being processed, the client socket was not sampled) - This ensures
one socket per client
5. Take from previous patch the changes in client side - avoid retries
6. Take from previous patch the change to enforce "winbind request
timeout" at all times, not just when the number of client connections
exceeds the limit.

While the approach of the previous patch set a hard limit on number of
file descriptors, at the cost of suspending new connection acceptance,
this patch set never stops accepting connections, at the cost of
staying with the soft limit (but has provisions to ensure that a
carefully planned system will not reach the limit).

The patch set:
V4 1/7 - Set winbindd process file descriptor limit according to
"winbind max clients" and "winbind max domain connections"
V4 2/7 - Monitor client socket when request is queued / processed
V4 3/7+4/7 - Refine 2/7 by emitting differnet debug messages according
to type of client activity while socket is being monitored
V4 5/7 - Same as V3 5/6 (changes to client side)
V4 6/7 - Same as V3 6/6 (request timeout)
V4 7/7 - Change to documentation, clarifying that "winbind max
clients" is not a hard limit.

Thanks,
Uri

On Mon, Jun 8, 2015 at 11:31 PM, Volker Lendecke
<Volker.Lendecke at sernet.de> wrote:
> On Mon, Jun 08, 2015 at 10:25:53PM +0300, Uri Simchoni wrote:
>> I'll try what you suggest. I tried documenting my understanding in
>> comments inside.
>
> Thanks for reading through my mail :-)
>
>> On Mon, Jun 8, 2015 at 12:13 PM, Volker Lendecke
>> <Volker.Lendecke at sernet.de> wrote:
>> >
>> > Sorry to be a bit late in the game, I was too busy to really give the
>> > issue some thought last week.
>> >
>> > How about the following approach to solve the problem:
>> >
>> > Every time we are about to ship some request to a winbind child we could
>> > to check whether the corresponding winbind client is still around. I
>> > know that this means that we need to carry state information down to
>> > some deep levels. The information about which client is being served is
>> > not readily available in wb_child_request_trigger, but I'm sure there's
>> > ways to get it there.
>> >
>> > We can even go one step further and keep listening for readability on
>> > the client socket while a request is in progress, waiting for its turn
>> > down to the child process. While a child RPC request is in progress,
>> > we need to stop listening for the client disconnect, as this would badly
>> > destroy the parent->child protocol sequence.
>> >
>> > If that is done, isn't most of the problem already solved?  If we are
>> > running into fd limits, just bump up the number of fds. Fds are relatively
>> > cheap these days, in particular if with modern winbind you're using
>> > epoll. 3.3.16 won't have that yet, but master should.  Also, there can
>> > be at most one fd per system thread assuming normal use of libwbclient.
>>
>> It has taken me a while to figure out this one. What I understand is
>> "don't bother with stopping and resuming accept, just set winbind max
>> clients to something high enough, and bump the max FDs accordingly.
>> Even if you don't know what high enough is, you can set it to
>> something really really high because winbindd's ability to accept
>> clients (with epoll) exceeds the system's ability to create clients
>> (with fork - 1 fd per process, assuming client is not malicious and
>> well behaved)" That, assuming we keep the number of accepted
>> connections equal to the number of clients, and we do that by checking
>> whether the client is still there.
>
> Right. This assumes that we have some reasonable means to
> limit the winbind connections to one per process, which is
> not the case right now given the timeout and reconnect
> behaviour.
>
>> In that case I would also change (fix) the documentation of "winbind
>> max clients" - it is not a limit but more of a hint.
>
> Also right. And we should just remove that silly DEBUG(1)
> message...
>
>> What if the client is not well-behaved? I suppose we can attack the
>> problem of malicious clients by passing the client's pid via the unix
>> domain socket and keeping a map that enforces the one connection per
>> client rule - that's outside the scope of this patch set as this
>> possibility existed before.
>
> Also correct. Before implementing this scheme, we need to
> watch out for the recent changes in libwbclient that make it
> possible to open multiple connections from a single process
> using the Ctx variants of the calls. If I read "man unix"
> right, SCM_RIGHTS only gives us the PID. But as you said,
> we're not safe against malicious clients right now, so I
> would postpone that a bit.
>
> Volker
>
> --
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wbconns4.patch
Type: application/octet-stream
Size: 18877 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150706/de9a2fc5/wbconns4.obj>