Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-5419

Incorrect usage of thread pool in TThreadPoolAsyncServer may lead to poor performance

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.14.1
    • Fix Version/s: 0.15.0
    • Component/s: netstd - Library
    • Labels:
      None
    • Environment:

      Windows 10 with Visual Studio 2019.

      Description

      Abstract:

      TThreadPoolAsyncServer uses ThreadPool.QueueUserWorkItem for each incoming connection to post a long-lived, synchronous task to the thread pool. This causes deadlock in the thread pool, which is only designed for short lived tasks, and can cause dramatic poor performance throughout your application, even in unrelated areas.

      Details:

      The .NET Thread Pool is not designed to handle long lived tasks. It has a complex set of logic that starts the pool at a size based on available CPUs and memory, then automatically grows it if it detects deadlock (no tasks completed for a second). Let's say that number is 4, as soon as you have 4 concurrent connections, everything in your process will come grinding to a halt until a second has passed and it grows the pool. This will affect everything in your process, even seemingly unrelated code and can have dramatic bad effects on performance.

      Fix:

      There are a number of fixes possible, but the most efficient and best is to use async/await to rewrite Execute() as asynchronous.

      Other side effects:

      Execute has a catch(TTransportException) block which I suspect will never be hit. Calling .Wait() and/or .Result always wraps exceptions in an AggregateException. A properly written async/await version would unwrap that exception for you.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                nsharp Nathan P Sharp
                Reporter:
                nsharp Nathan P Sharp
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 1h 50m
                  1h 50m