Cascading relationship deletes can select the wrong rule.

[MythicLionMan]

I've encountered an issue that has a similar root cause to issue #274 but manifests itself differently. When an object that is deleted as a result of a relationship causes another rule to fire, the second edge is not always retrieved correctly. This can cause additional objects to be deleted even though there is no relationship that should cause them to be deleted.

Unfortunately this issue is difficult to reproduce, so I haven't determined the exact conditions required to trigger it, even though the results of doing so are quite catastrophic. I do have a workflow in our application that will delete all of the objects in our database about 1 time in 20.

Since I can't give exact conditions to reproduce this I have identified what is going wrong, as well as describing an example of the failure in detail. Finally I have a workaround that we're using until a better solution is proposed.

Failure Mechanism

The root of the problem is that when YapDatabaseRelationshipTransaction enumerateExistingEdgesWithSource (or enumerateExistingEdgesWithDestination) fetches an edge from the edgeCache the cached edge is not always the correct one. I believe that this is an instance of sqlite rowid re-use as described in issue #274, but in this case it's affecting the rowid's of edges instead of the objects that they reference.

When the edge is retrieved from the cache the sourceRowid and destinationRowid are initialized from the data retrieved from the database. This is fine if the data fetched and the edge in the cache match, but in this case data for a different edge is fetched from the database. (The only things that match are the edgeRowid's). Since the edge came from the cache the source reference and destination reference are already filled out, but they don't correspond to the objects referenced by sourceRowid and destinationRowid.

The edge is then passed back from the enumerator to the flush method. The edge deletion rule is examined and determines that it must delete the destinationRowId. But this is the wrong row id and so the wrong object is deleted.

The situation is exacerbated by the fact that most of our edges reference the root of the object graph, so there's a high likely hood that the mishandled edge will delete the root of the graph and then cause everything else to be deleted.

Failure Example

1. A thread model object is deleted: <YapCollectionKey collection(threads) key(4292)> rowID = 13656. The description of the thread in the db is as follows:
   rowid collection key

   ---

   13656 threads 4292

2. YapDatabaseRelationshipTransaction enumerates the edges for the thread by fetching them from the db.

3. While checking edges where the thread is the src (src = 13656) an edge with row id 15634 is retrieved. The on disk representation of this edge is as follows (this is the edge that deletes the thread if the account is deleted):
   rowid name src dst rules manual

   ---

   15634 account 13656 10 4 0 r
   The destination of this edge is the account, which has rowid 10. Its description is as follows:
   rowid collection key

   ---

   10 accounts [email protected]

4. The edge retrieved from the edge cache by enumerateExistingEdgesWithSource: does not match this row. It has the following in memory representation:

   <YapDatabaseRelationshipEdge[0x6080012fdc80] name(mutationReferences) src(148B0453-7E17-4D62-A4AF-3CCBE5DEF222, RemoteMutations) dst(148B0453-7E17-4D62-A4AF-3CCBE5DEF222_0, RemoteMutationsReferences) rules(DeleteDestinationIfSourceDeleted) state(HasSourceRowid, HasDestinationRowid, HasEdgeRowid) action(none) manual>

This is a different edge, but it does have the correct edgeRowId, 15634. The srcRowid is the correct value that was queried, 13656, and the destRowId = 10, which is the account.

5. This edge is now updated with the retrieved sourceRowid and destinationRowid, which reference the thread and the account via the rowid's, but the RemoteMutation and RemoteMutationReferences via the collection/keys. The edge is in an inconsistent state since it references different objects via row ids and via references.

6. The new hybrid rule is passed to the callback supplied by the flush method. The flush tries to delete the destination node, which it assumes is a RemoteMutationReference, but instead is an account.

7. The account is the root of our object graph and the remaining relationships proceed to remove all the things.

Proposed Solutions / Workarounds

1. Delete edges from the edge cache immediately when the edge is deleted to avoid re-use when its rowid is reassigned. I looked at the clean-up code and it looks like this is happening anyway, so it just isn't happening at the right time, or there's a bug that's letting the edge stay in the cache when it shouldn't. There is a synchronization issue here to ensure that the memory cache and database transaction are reflecting the same data.

2. Validate edges when retrieving them from the edge cache and discard them if they don't make sense. If an edge comes out of the cache with a different rowid than was fetched from the database it's easy to detect. But there is a possibility that the incorrect edge has the correct sourceRowid and destinationRowid but a different nodeDeleteRule, or a destination URL to be retrieved, so all values retrieved from the database need to be compared to the cached value before it can be trusted.

3. Don't initialize the rowid's of an edge retrieved from the cache if it indicates that they are already set. This would probably eliminate our problem, because it would maintain the integrity of the edge, but the edge that is being examined is still not the edge that was fetched from the database.

4. Disable the edge cache. When the edge cache is queried it's because the database has returned an edgeid value. The query that does so includes most of the data relevant to the edge anyway, so the only savings in using the cache is avoiding the overhead of instantiating a new wrapper. The cached edge also stores the resolution of the sourceRowid and destinationRowid to their respective key/collection pairs, but that information is cached in the databaseTransaction collectionKeyForRowid: so performing the lookup again isn't expensive (though considering issue Relationship regression: Removing and inserting an object breaks the relationship #274 the results are somewhat suspect).

5. Don't use the relationship extension.

For now we're trying option 5, disabling the edge cache, since it was the easiest to implement. If we continue to have problems we'll move on to option 5.

[shsteven]

Ran into the same issue.
Actively flushing the cache before deletion will work around it.

Maybe for cascaded object deletion, the Relationship extension should bypass the cache altogether?