0

We are seeing some hard to reproduce issues that seem to relate to the SQL server and how we communicate with it, and I'm investigating if we might be using DBContext in conjunction with Task wrong.

We have a signalR hub used my many different clients. When we reach a certain level of clients, the system slows down and things will eventually stop working.

The signalR is similar to this example, but with many more async methods:

namespace Test.Webservice.Hubs
{
    public class ExampleHub : Hub<ExampleClientContract>, IExampleHub
    {
        private readonly ILogger _logger;
        private readonly IExampleRepository _exampleRepo;

        public ExampleHub(ILogger logger, IExampleRepository exampleRepo)
        {
            _logger = logger;
            _exampleRepo = exampleRepo;
        }
        
        public async Task<IEnumerable<ExampleTopicState>> GetExampleStates(Guid operationId)
        {
            var examples = await _exampleRepo.GetExampleStatesAsync(operationId);

            return examples.Select(ExampleTopicState.Create);
        }   
    }
} 

The ExampleRepository looks something like this, but again with many more methods:

namespace Test.DataAccess.Repository
{
    public class ExampleRepository : IExampleRepository
    {
        private readonly ILogger _logger;
        private readonly ExampleContext _context;
        private readonly IExampleRepositoryMapping _repositoryMapping;

        public ExampleRepository(ILogger logger, IExampleRepositoryMapping repositoryMapping)
        {
            _repositoryMapping = repositoryMapping;
            _logger = logger;
            _context = new ExampleContext();
        }

        public async Task<IEnumerable<ExampleDto>> GetExampleStatesAsync(Guid operationId)
        {
            IEnumerable<Example> result = null;

            await Task.Factory.StartNew(() =>
                                        {
                                            result = _context
                                                     .Examples.Where(c => c.OperationId.Equals(operationId))
                                                     .GroupBy(o => o.Type)
                                                     .SelectMany(p => p.GroupBy(q => q.Topic))
                                                     .Select(o => o.FirstOrDefault(
                                                                 n => n.ExampleTimeUtc == o.Max(d => d.ExampleTimeUtc)));
                                        });

            return _repositoryMapping.Mapper.Map<IEnumerable<ExampleDto>>(result);
        }

    }
}

Is it correct to use DbContext inside wait Task.Factory.StartNew, or could this lead to issues? This SO answere seems to say that it could be problematic, but I'm not sure if I understand it fully.

Edit:

Thought id add en example on how the hub is called from one of the clients:

    var loadingTasks = _connectorMap.Select(o => (System.Action) ( () =>
                                                                        {
                                                                            var result =  o.Proxy.Invoke<ExampleResult>(
                                                                                "GetExampless",
                                                                                _connectedOperations.Where(c => o.HubConnection.Url.StartsWith(c.ExampleWebService)).Select(s => s.UniqueId),
                                                                                filter.TimeFilter.Start,
                                                                                filter.TimeFilter.End,
                                                                                filter.TopicFilter,
                                                                                filter.SeverityLevelFilter,
                                                                                currentPage, pageSize,
                                                                                filter.IsGrouped).Result;
                                                                            _results.Add(result);
                                                                        }));

    Parallel.ForEach(loadingTasks, lt => lt());
Q-bertsuit
  • 3,223
  • 6
  • 30
  • 55
  • 1
    What is the lifetime of `IExampleRepository` (i.e. how is it registered with DI)? – devNull Dec 14 '20 at 10:03
  • `Task.Factory.StartNew` = "please run this code on some other thread for me, I've got other things to do" - `await` - "I've got no useful work to do until this other thing has finished". Do you see how those are mostly contradictory? If your current thread is not special in some way, just run the code *on the existing thread*. – Damien_The_Unbeliever Dec 14 '20 at 10:03
  • @Damien_The_Unbeliever Good point. Could it be the cause of the issues we are seeing? – Q-bertsuit Dec 14 '20 at 10:17
  • @devNull It's registered like this: container.Register(Component.For().ImplementedBy().LifestyleTransient()); – Q-bertsuit Dec 14 '20 at 10:20

1 Answers1

2

Do you just want SelectAsync?

public async Task<IEnumerable<ExampleDto>> GetExampleStatesAsync(Guid operationId)
{
    IEnumerable<Example> result = null;

    var result = await _context
            ...
            .SelectMany(...);
            .Select(...)
            .ToListAsync(); // Fetch from db.
    });

    return _repositoryMapping.Mapper.Map<IEnumerable<ExampleDto>>(result);
}

A full example is available at Async query and save.

tymtam
  • 31,798
  • 8
  • 86
  • 126
  • Thanks for this! Do you think the design using Task could lead to issues or is it just inefficient? – Q-bertsuit Dec 14 '20 at 11:09
  • 2
    I think the most relevant adjective is that `Task.Factory.StartNew` is unnecessary. Even for cases where there isn't a method ready (like ToListAsync), Task.Run is the way to go: See [docs](https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.taskfactory.startnew?view=net-5.0#remarks):*Starting with the .NET Framework 4.5, the Task.Run method is the recommended way to launch a compute-bound task.* – tymtam Dec 14 '20 at 11:29
  • 1
    Also, you were mixing `Task` with `IEnumerable`, which is not necessarily *wrong*, but can certainly be confusing. The `StartNew` (which should be `Task.Run`) is pushing work to a thread pool thread, but the work being pushed off is literally just *setting up* the query, not even *running* it, because `IEnumerable` is lazily executed. Using `ToListAsync` will properly execute the query asynchronously. And then the thread pool thread isn't necessary at all, since it's already asynchronous. – Stephen Cleary Dec 15 '20 at 01:37
  • Turns out the problem was somewhere else in the code. Thank you for pointing out the problematic parts of the code. Much appreciated! – Q-bertsuit Dec 19 '20 at 07:11
  • 1
    @Stephen Cleary That makes sense! I was confused about why IEnumeral was used my self. This is code written by developers who no longer work in the company, so there are no one to ask about it. – Q-bertsuit Dec 19 '20 at 07:13