SlapperAutoMapper / Slapper.AutoMapper

Slapper.AutoMapper maps dynamic data to static types. Slap your data into submission!
MIT License
287 stars 76 forks source link

Cleaning cache causes incorrect mapping #81

Open lukashovancik opened 3 years ago

lukashovancik commented 3 years ago

Hello,

I have 20 workers executing the same code. In the code I have a query where I search for an order and list of the order products . After that, I map the result of the query to the order object class AutoMapper.MapDynamic<Order>(results).First(); , but from time to time I am missing some order products. Only when I remove line related with the cache Slapper.AutoMapper.Cache.ClearInstanceCache(); everything is working properly but of course cache is rising, there is no way to clean it.

Is possible that Slapper.AutoMapper.Cache.ClearInstanceCache() affect other order queries running in the different workers ?

randyburden commented 3 years ago

Did you ever get this sorted out? Clearing the instance cache works differently depending on your app type. If your app is running ASP.NET on .NET Framework it will try to use HttpContext if available as the cache store. If HttpContext is not available, then it uses AsyncLocal as the backing cache which has its own behavior described in the Microsoft Docs link.

One unique characteristic of AyncLocal is that it will automatically flow its data to child tasks which may be what is happening in your implementation depending on how it was architected. If this behavior is not desirable you can suppress the execution context from flowing into the child task like in the example below. See ExecutionContext.SuppressFlow for more details.

using (ExecutionContext.SuppressFlow())
{
    Task.Run(...);
}
wegnerb commented 2 years ago

Hi @randyburden,

I'm running in to a similar issue as @lukashovancik. I appreciate your response about the AsyncLocal topic because it gave me something to research since I wasn't familiar with that concept. To be 100% honest I'm not completely clear how this cache concept works but I'm not sure that totally matters.

Here is what I'm observing:

  1. When I'm running within an async flow it appears to not matter whether I say to keep the cache or not. It appears to always create a new instanceCache each time I call the MapDynamic method from a separate async flow. My guess is because the AsyncLocal is only visible to each independent asynchronous flow but I could certainly be wrong here. I figured this out by downloading the master branch and putting a couple simple log statements in the Slapper.AutoMapper.Cache>>GetInstanceCache and Slapper.AutoMapper.Cache>>ClearInstanceCache methods.

  2. When the Slapper.AutoMapper.Cache>>ClearInstanceCache method is called it ultimately calls the CallContext.FreeNamedDataSlot(...) method in my world. When it removes the key from the ConcurrentDictionary it appears to "remove" all caches for each running asynchronous flow because my other running asynchronous flows will create a new cache. You can see in the example below after one call to the clearing of the cache the other asynchronous flow creates a new instance cache and then doesn't get my expected result.

Example:

            var program = new Program();

            var tasks = new List<Task<IEnumerable<Person>>>();
            while (true)
            {
                var asyncTask = program.DoTheThingsAsync(keepCache: false);

                tasks.Add(asyncTask);

                if (tasks.Count() == 2)
                {
                    Console.WriteLine("Waiting for tasks...");

                    await Task.WhenAll(tasks);

                    foreach (var task in tasks)
                    {
                        var people = task.Result;

                        var peopleCount = people.Count();
                        var addressCount = people.Sum(x => x.Addresses.Count());
                        if (peopleCount != 1000 || addressCount != 3000)
                        {
                            Console.WriteLine($"Sad Life::Expected People Count to be 1000 and got {peopleCount} and Address Count to be 3000 and got {addressCount}");
                        }
                        else
                        {
                            Console.WriteLine($"Happy Life");
                        }
                    }
                    tasks.Clear();
                    System.Threading.Thread.Sleep(5000);
                }
            }

image

Possible Resolutions (I'm no expert but these worked):

  1. In the Slapper.AutoMapper.Cache>>ClearInstanceCache() method you could do one of the following things

    • You could remove the line of code InternalHelpers.ContextStorage.Remove(InstanceCacheContextStroageKey)
    • You could change that line of code to InternalHelpers.ContextStorage.Store(InstanceCacheContextStorageKey, null). This would make it so the instance cache is basically gone and a subsequent GetInstanceCache() call would create a new instanceCache.
  2. I could obviously just set the keepCache to true and this would make everything work like I expected but in my situation my memory would end up growing too big over time.

  3. I tried researching ExecutionContext.SuppressFlow a bit but I wasn't super familiar with this concept or how well that would fit in to our current code set.

Here is a sample C# program that I used to re-produce the problem Slapper.TestConsole.zip