tmsmith / Dapper-Extensions

Dapper Extensions is a small library that complements Dapper by adding basic CRUD operations (Get, Insert, Update, Delete) for your POCOs. For more advanced querying scenarios, Dapper Extensions provides a predicate system. The goal of this library is to keep your POCOs pure by not requiring any attributes or base class inheritance.
1.79k stars 586 forks source link

Multithreading does no longer work #274

Open PlumBum91 opened 3 years ago

PlumBum91 commented 3 years ago

I have developed a simple little web application where multiple requests running the same time. Since updating Dapper.Extensions to the most current (official) version, I ran into multiple exceptions which looks like a bug within the Dapper.Extensions internal SQL query generation (DapperImplementor.LastExecutedCommand).

Here are the two exceptions that always occur:

System.InvalidOperationException: Sequence contains no elements at System.Linq.ThrowHelper.ThrowNoElementsException() at System.Linq.Enumerable.Single[TSource](IEnumerable1 source) at DapperExtensions.DapperImplementor.GetColumnAliasFromSimpleAlias(String simpleAlias) at DapperExtensions.DapperImplementor.MappColumns[T](IEnumerable1 values) at DapperExtensions.DapperImplementor.GetListAutoMap[T](IDbConnection connection, IList1 colsToSelect, IClassMapper classMap, IPredicate predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 includedProperties) at DapperExtensions.DapperImplementor.InternalGetListAutoMap[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 colsToSelect, IList1 includedProperties) at DapperExtensions.DapperImplementor.GetListAutoMap[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 includedProperties) at DapperExtensions.DapperImplementor.GetList[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 includedProperties) at DapperExtensions.DapperExtensions.GetList[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered)

System.Collections.Generic.KeyNotFoundException: The given key '[COLUMN NAME]' was not present in the dictionary. at System.Collections.Generic.Dictionary2.get_Item(TKey key) at Slapper.AutoMapper.InternalHelpers.<>c__DisplayClass11_0.<GetCacheKey>b__0(String id) at System.Linq.Enumerable.SelectListIterator2.ToArray() at System.Linq.Enumerable.DefaultIfEmptyIterator1.ToArray() at Slapper.AutoMapper.InternalHelpers.GetCacheKey(Type type, IDictionary2 properties, Object parentInstance) at Slapper.AutoMapper.Map(Type type, IEnumerable1 listOfProperties, Boolean keepCache) at Slapper.AutoMapper.Map[T](IEnumerable1 listOfProperties, Boolean keepCache) at DapperExtensions.DapperImplementor.MappColumns[T](IEnumerable1 values) at DapperExtensions.DapperImplementor.GetListAutoMap[T](IDbConnection connection, IList1 colsToSelect, IClassMapper classMap, IPredicate predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 includedProperties) at DapperExtensions.DapperImplementor.InternalGetListAutoMap[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 colsToSelect, IList1 includedProperties) at DapperExtensions.DapperImplementor.GetListAutoMap[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 includedProperties) at DapperExtensions.DapperImplementor.GetList[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable1 commandTimeout, Boolean buffered, IList1 includedProperties) at DapperExtensions.DapperExtensions.GetList[T](IDbConnection connection, Object predicate, IList1 sort, IDbTransaction transaction, Nullable`1 commandTimeout, Boolean buffered)

When I "slow" every query down with using SemaphoreSlim's to run only one DB query at a time then everything runs fine.

valfrid-ly commented 3 years ago

Could you send your mappings please and how to reproduce the error?

fpundert commented 2 years ago

It is too bad that this issue doesn't get more attention, it is basically ruining any real world usage of DapperExtensions. Quite easily reproducable as well, just create a simple api with a simple mapping to read an object (in my case sqlserver) and invoke the API multiple times in parallel, it is guaranteed to fail.

valfrid-ly commented 2 years ago

It's is not without attention but I'm wating for more information.

If you know how to reproduce, please walk me thru it I have this in use on a company where I used to work so I know that it works on real world application.

fpundert commented 2 years ago

For me the steps to reproduce were very simple but maybe it is due to my specific situation. I have noticed the issue a couple of months back an retried it recently in a completely different project, again with the same issue.

I created a simple "country" table on SQL Server 2019 with an int64 identity. Updating an country and reading back the country using a predicate on the Id single threaded works fine. However, as soon as I started testing this multithreaded, it fails. All my rest api did was update a country and read it back with "id=x" and "deleted is null" with an and-predicate, this result was returned. Multithreaded testing was done using JMeter by calling the API (although irrelevant).

I did some quick research hoping for a quick fix, however, it could require some rework. The issue appears to be related to the fact that the sqlgenerator class is used as a static thru a static implementor. This generator has a list of tables which is being recreated upon every query. But since this tables-list is ultimately static and used over multiple calls this tables-list is changed mid procedure by other threads.

Consider this:

Thread A build the tables list and start building the SQL query. Thread B rebuilds the tables list for the scenario required by thread B. Thread A wants to continue building the query but fails as the expected in memory objects are no longer there or changed.

Again, this was a very quick investigation so I could be wrong here. But this is my 2 cents.

thefat32 commented 2 years ago

I can confirm SqlGeneratorImp is not thread safe, as DapperExtensions use the same instance Implementor in each call, the sql generator is invoked by two or more threads during paralell requests resulting in different errors while generating querys or parsing results. After reviewing the code I think creating a new Implementor instance for every request can workaround this limitation. It's pretty late here and going to sleep, will try this solution tomorrow.

EDIT: After thorough testing I came to a solution to workaround this problem till it is solved in the lib. It´s necessary to create a DapperAsyncImplementor for each request you need to make in paralell.

public void ConfigureServices(IServiceCollection services)
{
    //... Other startup code
    services.AddDapper();
}

public static IServiceCollection AddDapper(this IServiceCollection services, IConfiguration configuration)
{
    // Your desired DapperConfiguration
    var dapperExtensionsConfig= new DapperExtensionsConfiguration(typeof(ClassMapper<>), mappingAssemblies, new PostgreSqlDialect());
    // Register Dependency Injection
    services.AddScoped<IDapperAsyncImplementor>((_) => new DapperAsyncImplementor(new SqlGeneratorImpl(dapperExtensionsConfig)));
}

// Usage Example

public class ListItemQueryHandler<TItem>
    {
        protected readonly IDbConnection _connection;
        protected readonly IDapperAsyncImplementor _implementor;

        public ListItemQueryHandler(IDbConnection connection, IDapperAsyncImplementor implementor)
        {
            _connection = connection;
            _implementor = implementor;
        }

        public async Task<List<TItem>> Handle()
        {
            var result = await _implementor.GetListAsync<TItem>(_connection);
            return result.ToList();
        }
    }
valfrid-ly commented 2 years ago

Thanks for the help @thefat32 , I'll be looking for the implementation of the solution

thefat32 commented 2 years ago

I think this could be solved introducing something like a SqlGeneratorFactory in DapperImplementor

patrickabernathy commented 9 months ago

Apologies for necro-ing this thread but hoping there is still some work on this project. I've used this package for years and hope this issue can be resolved. I am using channeling and any time GetList hits with multiple threads, I get the error "The multi-part identifier "dbo.Sites.SiteID" could not be bound". I attached MiniProfiler to get the sql generated and get the query below.

CommandType: Text, CommandText: SELECT [y_1].[SiteID] AS [c_0] FROM [dbo].[Sites] [y_1]
 WHERE ([dbo].[Sites].[SiteID] = @SiteID_0)
Database: ###
Parameters:
Name: SiteID_0, Value: 123456

On a single thread, the query shows correctly so it seems the predicate is not mapping correctly in a thread.

CommandType: Text, CommandText: SELECT [y_1].[SiteID] AS [c_0] FROM [dbo].[Sites] [y_1]
 WHERE ([y_1].[SiteID] = @SiteID_0)
Database: ###
Parameters:
Name: SiteID_0, Value: 123456

By using a new implementor each request I've alleviated the issue but still have to wrap in a retry mechanic to handle the ones that still occur.