Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have a way to determine if an entity is already attached / Detach entities #21554

Closed
juliusfriedman opened this issue Jul 8, 2020 · 16 comments
Closed

Comments

@juliusfriedman
Copy link

juliusfriedman commented Jul 8, 2020

Trying to avoid The entity with ID XYZ is already being tracked

 /// <summary>
        /// Useful to release memory from an existing context.
        /// </summary>
        public void DetachAllEntities()
        {
            var changedEntriesCopy = ChangeTracker.Entries()
                .Where(e => e.State == EntityState.Added ||
                            e.State == EntityState.Modified ||
                            e.State == EntityState.Deleted)
                .ToList();

            foreach (var entry in changedEntriesCopy)
                entry.State = EntityState.Detached;
        }

        /// <summary>
        /// Determines if the <paramref name="entity"/> is tracked
        /// </summary>
        /// <typeparam name="T"></typeparam>
        /// <param name="entity"></param>
        /// <returns><see langword="true"/> when tracked / attached otherwise <see langword="false"/></returns>
        public bool IsTracked<T>(T entity) where T : class
        {
            return this.Set<T>().Local.Any(e => e == entity);
        }

Problem

Doesn't work, I can't detach the entity and I can't get a reliable word on if it's already attached so I am left with:

if (false == insertionContext.IsTracked(someObject))
                        {
                            try
                            {
                                var someEntity= insertionContext.Objects.Attach(someObject);
                                someEntity.State = EntityState.Unchanged;
                            }
#pragma warning disable CA1031 // Do not catch general exception types
                            catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
                            {
                                System.Diagnostics.Trace.WriteLine($"Method: {ex.Message}\r\nStackTrace:{ex.StackTrace}");
                            }
                        }

Which can be reduced to: (because the function above IsAttached will only return true after the exception is thrown)

 try
                            {
                                var someEntity= insertionContext.Objects.Attach(someObject);
                                someEntity.State = EntityState.Unchanged;
                            }
#pragma warning disable CA1031 // Do not catch general exception types
                            catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
                            {
                                System.Diagnostics.Trace.WriteLine($"Method: {ex.Message}\r\nStackTrace:{ex.StackTrace}");
                            }

Solution

Something that works without exceptions.

See also #21555

Additionally I have change tracker disabled on the context yet I still find Snapshots are rooted in GC which never are released even after calling something like DetachAllEntities.

@juliusfriedman juliusfriedman changed the title Have a way to determine if an entity is already attached Have a way to determine if an entity is already attached / Detach entities Jul 8, 2020
@ajcvickers
Copy link
Member

@juliusfriedman I'm finding it hard to understand the exact intentions of the methods you have. For example, if you want to detach all entities (which is not recommended, btw; creating a new context instance is recommended, but see also #15577) then why leave Unchanged entities attached? I think we will likely need more complete code that we can run that demonstrates the issue you are having so that we can properly investigate.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 10, 2020

Yes I figured that out the hard way after my usage OOM'ed.

I build up a list of entities and then insert them on a different context....

I will provide an example but again but the point stands.

Context A and Context B are both inserting the same data, I should be able to tell from the exceptions which records are failing or ignore them so I can proceed so that I can try to beat B to get some of that data in.

Additionally after a SaveChange my references are back (despite not tracking changes) so I have to either null them again before updating... it's just a headache.

Im half ready to just go back to SqlClient and just make a few thin wrappers or dynamic emitters like I used to...Quite easily

@ajcvickers
Copy link
Member

@juliusfriedman If you're using SaveChanges, then you must be tracking changes, because that's how SaveChanges decides what needs to be changed in the database.

In general, I get the impression that you are fighting against the way EF works. This generally doesn't work very well--EF Core is flexible in many ways, but it does require buying into some concepts like using change tracking for saving changes. It may be better to use other lower-level technologies like Dapper and raw ADO.NET if you want to go a different direction.

@juliusfriedman
Copy link
Author

What if I have ChangeTracker.AutoDetectChangesEnabled = false, and I Simply Attach to the context what I want to save, is that fighting against entity framework?

I realize it must start tracking but the default is Unmodified for Attach so if I tell it to Update then even if I attach again I should not get an exception, I should just take the change based on the state of that new entity e.g. if Unmodified then skip (dont throw), if Modified then change trace the diff (dont throw), if deleted... if Inserted ignore or change track. (dont throw)

@ajcvickers
Copy link
Member

Attach means start tracking this entity. That's fine, although should only be needed in disconnected scenarios where the entity was queried at some other time.

default is Unmodified for Attach so if I tell it to Update then even if I attach again I should not get an exception

Correct. Attaching an entity and then calling Update on the same instance will change the state to Modified. If this isn't happening, then please attach a small, runnable project that demonstrates how it is failing so we can investigate.

I should just take the change based on the state of that new entity e.g. if Unmodified then skip (dont throw), if Modified then change trace the diff (dont throw), if deleted... if Inserted ignore or change track. (dont throw)

This is basically how EF Core works. If you're having issues with this, then please post a small, runnable repro demonstrating the issue so that we can investigate.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 10, 2020

I cannot post my current code unless I do not expose the models is it okay to post snippets of how I use the code so that you can help me identify my problematic usage or the bug? I will be much more willing to answer questions about why I am doing something rather than sharing my projects code.

You are correct on all accounts expect the last, if you attempt to attach the same entity twice you will receive an exception the entity is already tracked. (Why)

And the reason is because I am using the entities from another context which I am using in parallel and then using multiple other contexts. (Unit of work pattern)

@ajcvickers
Copy link
Member

@juliusfriedman I'm not asking you to post your product code. I'm saying if things are not working the way you think they should, then we will investigate. But to do that effectively we generally need to be able to reproduce what you are seeing. The best way for you to communicate that is to create a small sample demonstrating the behavior so that we can reproduce it.

if you attempt to attach the same entity twice you will receive an exception the entity is already tracked. (Why)

You may be attempting to attach a different entity instance with the same key value.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 11, 2020

I am because I have a one to many, but it still should not throw, it knows the entity is attached how can it be confused about what to do? just detect changes on it or if Unchanged move on.

@ajcvickers
Copy link
Member

@juliusfriedman Replacing one instance with another (in all but the trivially un-useful case of a single entity with no relationships) means resolving differences in the graphs of the two instances, as well as resolving any conflicts in scalar values. Typical patterns such as using a single context instance for a single unit of work don't typically run into this problem.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 13, 2020

@ajcvickers, Why make the problem harder that it seems? You only need to traverse those entities which are both attached and for the following code:

if(someObject.GetHashCode() != knownHashCode)

Additionally, if there was a method:

public class DbContext{
+ public bool TryAttach<TEntity>(entity, EntityState state, out EntityState result);
}

You would be be able to then say oh this entity is definitely not updated because I was passed Unchanged or if I was passed updated you would then look for changes and apply them only if necessary.

Is that not the truth of the matter? If so I would greatly appreciate your insight as to why it is not so.

I have a single context to which I probably should not be using for writing as it was created with this method:

public static MyDbContext:DbContext CreateReadOnlyContext(string connectionString = null, int? commandTimeoutSeconds = null, int? retryOnFailureCount = null, TimeSpan? maxRetryDelay = null)
        {
            var result = new MyDbContext (connectionString, commandTimeoutSeconds, retryOnFailureCount, maxRetryDelay);
            result.Database.ExecuteSqlRaw("SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED");
            result.ChangeTracker.AutoDetectChangesEnabled = false;
            return result;
        }

With that I get my readContext, I do my work of reading and then building up my list of known entities with changes. (For various reasons I do not just Update and expect things to work, I specifically null out the entities and leave the primary keys but ensure all nested entities are also null)

When I have what I want to update / insert or deleted I then open a writeContext and then Attach as necessary and change the EntityState to either Added or Modified as required.

When I am looping through as I do I need code like this...

if (entity.PrimaryKey != 0) {
                        {
try
                            {
                                //Ensure the context is aware of the existing Entity
                                var Entity = writeContext.Entity.Attach(entity);
                                Entity.State = EntityState.Modified;
                            }
#pragma warning disable CA1031 // Do not catch general exception types
                            catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
                            {
                                System.Diagnostics.Trace.WriteLine($"MyFunction: {ex.Message}\r\nStackTrace:{ex.StackTrace}");
                            }
} else {
  var Entity = writeContext.Entity.Add(entity);
  Entity.State = EntityState.Added;
}

Is there something wrong with this approach?

I do this because I am reading and writing at different times and also that I batch my updating via SaveChangesAsync and DisposeAsync the writeContext after a given batch size:

//If we have attached more then or equal to the BatchSize save off the records now.
                    if (inserted++ >= SynchronizationStrategy.BatchSize)
                    {
                        System.Diagnostics.Trace.WriteLine($"Batching Insert / Update of Data..");

                        int localEffectedRecords = await writeContext.SaveChangesAsync(SynchronizationStrategy.CancellationTokenSource.Token).ConfigureAwait(true);

                        effectedRecords += localEffectedRecords;

                        System.Diagnostics.Trace.WriteLine($"Batching Data Compete! ({localEffectedRecords}) Records Effected");

                        await writeContext.DisposeAsync().ConfigureAwait(false);

                        writeContext= new MyDbContext(SynchronizationStrategy.DestinationConnectionString, null, SynchronizationStrategy.MaxRetries, SynchronizationStrategy.MaxRetryDelay, false);

                        writeContext.ChangeTracker.AutoDetectChangesEnabled = false;

                        inserted = 0;
                    }

///omitted

If there is a flaw in my approach I would greatly appreciate your enlightenment and also your critique on my technique.

Thank you for your time.

@ajcvickers
Copy link
Member

@juliusfriedman I could continue to go through each of your posts and comment on why EF Core doesn't work like that, but I don't think this is the most productive way forward. I think it would be great if you could spend some time either going through docs/tutorials, or maybe an online course like those at Pluralsight. This would allow you to learn how EF does work now, which in turn would form a much stronger basis for a productive discussion if you then have ideas to change the way it works.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 13, 2020

I understand you but can you please review my code and tell me how you would solve for the exact same issue or what I am doing wrong? It's not like I did not provide a code example and it's not like I don't read documentation and have not used linqtosql, entity framework and efcore but I appreciate your input and will definitely try to keep updated either via pluralsight or otherwise but until then I would appreciate the issues I raise either be addressed or my code subjected to review rather then push me towards tutorials which will likely not benefit more than the information available at: msdn or https://entityframework.net/ or https://www.learnentityframeworkcore.com/ but I will definitely check it out.

Thank you.

@ajcvickers
Copy link
Member

@juliusfriedman Can you clarify what problem it is you're trying to solve? I went back and re-read the thread and there is lots of information about what methods you are calling and that they don't do what you expect, but I'm having trouble understanding what is is you're actually trying to achieve.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

I would like to avoid the error indicated at the initial post:

The entity with ID XYZ is already being tracked

I explained how I am using one DbContext to read and determine changes myself (because the reading context was created with READ UNCOMMITTED)

I then use a normal DbContext to write my changes using the Attach method and changing the Entity state to Modified or Added based on the primary key of the Entity.

During the writing process I experience the issue I mentioned above.

What seems to happen is that one of my Something have an SomethingElse which is already tracked.

The reason why is because another Something has the same SomethingElse which is already tracked (but a different relationship)

This is where the problem occurs.

If an further example is needed I will do my best to provide one.

Please let me know if I can be any clearer and thank you for your time!

@ajcvickers
Copy link
Member

I explained how I am using one DbContext to read and determine changes myself (because the reading context was created with READ UNCOMMITTED)

Are you using a tracking query for this? If so, then the query should be resolving identities to a single instance.

I then use a normal DbContext to write my changes using the Attach method and changing the Entity state to Modified or Added based on the primary key of the Entity.

Are you attaching entities from more than one of the queries above to the same context instance?

@ajcvickers
Copy link
Member

EF Team Triage: Closing this issue as the requested additional details have not been provided and we have been unable to reproduce it.

BTW this is a canned response and may have info or details that do not directly apply to this particular issue. While we'd like to spend the time to uniquely address every incoming issue, we get a lot traffic on the EF projects and that is not practical. To ensure we maximize the time we have to work on fixing bugs, implementing new features, etc. we use canned responses for common triage decisions.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants