[PATCH] winbindd: control number of winbindd's client connections
urisimchoni at gmail.com
Thu Jun 4 13:54:44 MDT 2015
Here's V2 with followiing changes:
v2 1/6 - int->unsigned
v2 6/6 (new) - periodically (every 5 seconds) check for idle clients
and close them.
On Thu, Jun 4, 2015 at 9:46 PM, Jeremy Allison <jra at samba.org> wrote:
> On Thu, Jun 04, 2015 at 08:13:14AM +0300, Uri Simchoni wrote:
>> > On Wed, Jun 03, 2015 at 09:12:06AM +0300, Uri Simchoni wrote:
>> >> This patch handles a case we've encountered in which winbindd opened
>> >> client connections up to the process limit on open file descriptors.
>> > One thing that we should take into account: If we happen to
>> > have winbind requests that do multiple steps to children,
>> > like for example getpwnam or more extremely getgrgroup with
>> > group expansion we do not check between two child requests
>> > whether the client has given up in the meantime. We might
>> > want to measure if that's an issue and drop client
>> > connections earlier, saving a lot of work inside winbind.
>> > Have you thought about that, or do you have a gut feeling if
>> > that could be an issue?
>> > Volker
>> That's a good catch.
>> It's true that while winbindd is working on a client request, it never
>> looks back at the client connection to see whether the client is still
>> there - it notices this only when trying to send the response back.
>> I considered polling the client, but noticed that we have
>> remove_timed_out_clients(), so I figured I'd raise the client timeout
>> to 300 sec and let the server give up first (by default - after 60
>> I ASSUME that removing a client this way also cancels any jobs on
>> behalf of the client, because otherwise the program would crash when
>> completing the request (verifying this is indeed so required me to dig
>> deeper into the RPC infra and I didn't do it -
>> remove_timed_out_clients() is not new).
>> What I DIDN'T notice was that remove_timed_out_clients() is being
>> called only when a new connection is to be accepted (i.e. to free some
>> connections) and not as a general server timeout mechanism.
>> So I propose to also remove_timed_out_clients() periodically.
> Ok Uri, can you post a follow-up patch please ?
> I'd really like to get this changeset into master.
> The only change I'd like to see is changing the
> + int completed_requests; /* Number of requests completed on this connection */
> to unsigned int, as counts can't be negative.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 16878 bytes
Desc: not available
More information about the samba-technical