[sheepdog] [PATCH] sheep: introduce 'short thread' to worker threads

[Liu Yuan]

On 07/03/2012 12:47 PM, MORITA Kazutaka wrote:
> On 2012/07/03 11:33, Yunkai Zhang wrote:
>> On Tue, Jul 3, 2012 at 2:31 AM, MORITA Kazutaka
>> <morita.kazutaka at lab.ntt.co.jp> wrote:
>>> At Fri, 29 Jun 2012 19:04:18 +0800,
>>> Liu Yuan wrote:
>>>>
>>>> On 06/29/2012 04:44 PM, MORITA Kazutaka wrote:
>>>>> Does the approach really show better performance and scalability than
>>>>> the current workqueue model with your short thread patch?  I'd like to
>>>>> see the performance number before accepting the change.
>>>>
>>>> The real benefit is that we don't need pre-set any number of threads,
>>>> because we don't assume cluster nodes number and we just dynamically add
>>>> nodes in on demand. Presetting will for sure constraint cluster
>>>> scalability and statical setting will also waste resource when node is
>>>> not busy. And dynamic threads work on demand and no waste for any
>>>> conditions.
>>>>
>>>> The performance number is not that relevant:
>>>>   1 it boils down to two pthread call efficient comparison:
>>>> pthread_cond_signal() and pthread_create(), both is heavily affected by
>>>> system resource usage at the time of being calling and kernel's scheduling.
>>>>   2 For a static code review, we can even see that short thread creation
>>>> won't need to hold wi->pending_lock at all and is scheduled to run once
>>>> is created (don't need notification and another lock holding). So I
>>>> think short threads performance might be slightly better than previous
>>>> statically work queue especially for concurrent coherent requests of
>>>> gateway and IO.
>>>>
>>>> Also we can see that both gateway and io threads exhibit 'on demand'
>>>> characteristics: we need much more threads when system is busy and very
>>>> few when is not, so we need on-demand threads implementation for that too.
>>>
>>> Agreed, and applied the patch, thanks.
>>>
>>> BTW, should we limit the maximum number of worker threads?  I guess it
>>> causes some problems to run thousands of threads simultaneously.
>>
>> Yes, we need to limit it.
>>
>> In our testing, to many threads lead to OOM.
> 
> Is this problem fixed by Yuan's patch "work: fix memory leak of shor thread creation"?
> 

No, I don't think so, we already have sys->outstanding_data_size >
MAX_OUTSTANDING_DATA_SIZE check for requests throttling. We should get
the root cause of OOM before go to any conclusion.

Thanks,
Yuan