Incomplete graph persistence in some cases

Posts   
 
    
Posts: 7
Joined: 27-Mar-2014
# Posted on: 27-Mar-2014 07:32:18   

We have observed that in some cases LLBLGen does not save all changes made in our graphs. In our case several nodes from a graph are passed to AddForSave (not all nodes). Depending on the set and order of nodes passed to the method, subsequent Commit call may or may not save all changes.

I have managed to create a simple repro with only 6 tables. Please find it attached. There is a readme file with steps to run the project. I could not use real names of entities because of NDA, acronyms are used instead.

LLBLGen version: v4.1 Final (November 26th, 2013) Runtime version: 4.1.0.0 Template group: Adapter Target .NET version: 4.0 Database: MS SQL Server 2012

Attachments
Filename File size Added on Approval
Llblgen.zip 107,667 27-Mar-2014 07:32.34 Approved
Posts: 7
Joined: 27-Mar-2014
# Posted on: 27-Mar-2014 07:38:41   

Judging by my debugging, the problem is that ProduceTopologyOrderedList returns different results for the same graph depending on the starting node. Sometimes (as in the repro case starting from object urr, line 98 ) the EEntity e that depends on 2 newly created EEntities gets into that list before the 2 entities it depends on. CheckIfEntityHasPendingFkSyncs returns false for it because the dictionary passed into the method does not yet contains the PK side entities. But then there is no second chance to analyze the entity because it gets into the list of already processed entities and is skipped during processing queues for entity newP (line 99)

Posts: 7
Joined: 27-Mar-2014
# Posted on: 27-Mar-2014 07:39:48   

Any suggestions on how to resolve this issue are highly appreciated.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39880
Joined: 17-Aug-2003
# Posted on: 27-Mar-2014 15:30:36   

We'll look into it simple_smile Too bad the names are obfuscated with short lookalike fragments, hard to keep track what is what.

I drew a diagram to see what is dependent on what from the init code. I did this wrong the first couple of times. I'm writing this out below as it's a very complicated repro case, and if I make a mistake along the way it's impossible to find what's wrong.

I came up with: e1 depends on pt1 e2 depends on pt1 e3 depends on pt1 ept1 depends on e1, pt1 ept2 depends on e2, pt1 ept3 depends on e3, pt1 ep1 depends on e1, p1 ep2 depends on e2, p1 ep3 depends on e3, p1 p1 depends on pt1, e3 pt1 depends on nothing.

there are several orders possible: as long as pt1 is done first, e3 before p1 and p1 before ep1 etc.

The inserts all succeed and are in the right order. The order chosen follows from the depth-first-search over the edges, meaning another order might have been chosen if another node is picked. That's fine, what counts in these cases is the dependencies: A has to be persisted before B if B depends on A; if there's a lot of work done between the two, that's not important.

Ok, you're now retrieving the graph again. This is done without sorting. This might be the problem cause (as you assume x to be at location y which might not be the case). The problem is that e3 is inserted before e2. So p1.Eps[2].E, which you pass to MakeChanges, is not e3, it's e2.

Is this expected?

If I change the index to 1 instead of 2 (so e3 is picked) it doesn't fix it though, but I thought I'd mention it. Also, as you reference elements more than once, to get uniquing, a context has to be used with the fetch (so an object is fetched multiple times but you get the same instance back). You can do so with linqMetaData.ContextToUse = new Context();

This too doesn't fix it, though I just wanted to point things out. I'll now check the MakeChanges code.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 7
Joined: 27-Mar-2014
# Posted on: 27-Mar-2014 17:06:57   

The initial dependencies are correct.

Thank you for pointing out the issues in the repro. I was not careful enough while writing it. However, you are right saying that Context usage and selecting correct e3 by the Value property does not fix it.

I guess the issue in Make changes is that it creates a cycle between 3 entities: e, te and re: e depends on te and re, te depends on e, re depends on te. Therefore they are kind of equal. I am afraid I cannot remove this cycle. And the fact that Commit works fine if the first processed entity is pt, but breaks if starts from urr (depending on re via a EptEntity) makes me think it should actually work in all cases.

I am sorry for weird entity names. Though I am not sure if real names would help, unless you are very familiar with insurance business wink

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39880
Joined: 17-Aug-2003
# Posted on: 27-Mar-2014 17:52:03   

PavelGatilov wrote:

The initial dependencies are correct.

Thank you for pointing out the issues in the repro. I was not careful enough while writing it. However, you are right saying that Context usage and selecting correct e3 by the Value property does not fix it.

Indeed, though it's necessary, otherwise you end up with multiple objects with the same data (as e.g. e1 is referenced multiple times)

I added lines for every edge fetched again to the same graph and found that you don't fetch Ept from E. So you have to add the subpath to the EEntity prefetch .Prefetch<EEntity>(pt => pt.Es).SubPath(f=>f.Prefetch<EptEntity>(p=>p.Ept))

if you want the reference to be present in memory (otherwise the reference isn't there. The FK field values of course are as they're part of the entity).

I guess the issue in Make changes is that it creates a cycle between 3 entities: e, te and re: e depends on te and re, te depends on e, re depends on te. Therefore they are kind of equal. I am afraid I cannot remove this cycle. And the fact that Commit works fine if the first processed entity is pt, but breaks if starts from urr (depending on re via a EptEntity) makes me think it should actually work in all cases.

yes, and there's another cycle at line 90:

            te.Obe = te;

here the instance te references itself. This can never work, as it can't persist itself.

the lines:

            e.Obe = te;
            e.Rbe = re;

these two create a cycle indeed, as e points to te, but te already pointed to e (line 45). The Rbe reference too creates a cycle (e-> re -> te -> e)

As it encounters a cycle, it skips the branch. This is expected, topological sort doesn't work on graphs with a cycle (it's a DAG algorithm). This is ok though, as graphs with a cycle can't be persisted in 1 go anyway. That the code does persist something when you start with another node is 'luck', as the cycle edges are ignored in such a way that it doesn't affect whole entities being cut off. The FK's still aren't persisted.

I am sorry for weird entity names. Though I am not sure if real names would help, unless you are very familiar with insurance business wink

The Russian equivalents likely wouldn't have helped wink .

Ok, now that that's cleared up, how to fix this. The main issue is that the graph traverser does its work in 1 go: it sorts the graph and along the way it collects work. This fails when there's a cycle as you described in your second post, because work for the update queue is skipped as it's seen as work to be done before elements which are for the insert queue (te and re) which isn't true of course: e depends on te and re but not for an insert, for an update, which is always executed after the inserts (that is, by default, one can execute updates before inserts in a UoW, though that's always based on the queues calculated, it fails already before that).

in theory I think if the traverser walks the graph twice, once with a clean slate for the inserts, then again for the updates, starting with the entities in the insert queue as 'processed' and not any other entity, it could work: e then would be processed again (as it should, it's not in the insert queue) and ne and te are inserted before it, so fk syncs take place.

Of course except the te.Obe = te; line, that will never work.

This likely isn't doable in this version (v4.1) as it will break the DetermineActionQueues code, but I'll see what I can do. v4.2 is currently in development, so if I can postpone the real work to v4.2 and add a workaround in the UoW ConstructSaveProcessQueues it might help you a bit.

Perhaps there's an easier fix, I have to examine the code more closely, which I'll do tomorrow (friday).

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39880
Joined: 17-Aug-2003
# Posted on: 27-Mar-2014 18:38:11   

FIXED. Turned out to be simpler than I thought. I refactored the code a bit so the only method which calculates the queues is now delegating the work to another method and calls it twice. Bookkeeping had to be a bit different but that's all. Graph saves properly now. The tests still have to run and as said the te referencing itself is not going to work regardless...

Will attach a working ORMSupportClasses when the tests pass simple_smile

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39880
Joined: 17-Aug-2003
# Posted on: 27-Mar-2014 18:53:08   

Attached.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 7
Joined: 27-Mar-2014
# Posted on: 27-Mar-2014 18:57:18   

Wow, thank you very much! Will check tomorrow on our code.

Posts: 7
Joined: 27-Mar-2014
# Posted on: 28-Mar-2014 10:12:27   

Works fine. Thank you for such a quick fix!

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39880
Joined: 17-Aug-2003
# Posted on: 28-Mar-2014 10:39:40   

You're welcome, glad it's working now. Keep in mind the things I suggested though: the te.Orb=te; code which will never work, and the context usage simple_smile

Frans Bouma | Lead developer LLBLGen Pro
Posts: 7
Joined: 27-Mar-2014
# Posted on: 28-Mar-2014 10:48:39   

Well, te.Orb=te could work if 2-pass commit were allowed. First insert te, then update it with its id. Just shared an idea simple_smile

Thankfully, this reference seems to be not important for our app, more of a historical artifact. So we don't really care about it. Maybe I remove it later.

dzubrilin
User
Posts: 1
Joined: 01-May-2014
# Posted on: 01-May-2014 22:31:04   

Hello Otis,

Is there a way you may provide this fix for .NET 4.5 version (v4.1. SD.LLBLGen.Pro.ORMSupportClasses.dll)?

Walaa avatar
Walaa
Support Team
Posts: 14994
Joined: 21-Aug-2005
# Posted on: 02-May-2014 08:19:20   

Please check the attached zip file above, it has the .NET 4.5 version.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39880
Joined: 17-Aug-2003
# Posted on: 02-May-2014 09:51:18   

The latest build on the website also has the fixes included.

Frans Bouma | Lead developer LLBLGen Pro