V8-archive: Too many revisions for relational fields

Created on 12 Apr 2019  Â·  7Comments  Â·  Source: directus/v8-archive

Bug Report

Steps to Reproduce

  1. Create collections with Many-to-Many fields pointing back to each other.
    image
    image
  2. Create new or choose existing items from either collection.
  3. Notice the time to save increases as more items are added.
  4. Check the directus_revisions table to see an extraordinary amount of rows being written for each change.

Expected Behavior

Only new changes should be saved to revisions. https://github.com/directus/api/issues/831
Although the above is an extension, I think this is more of a bug when handling relationship fields. I don't expect each one of the items related to should get stored as a full copy in directus_revisions.

Actual Behavior

Copies of the objects are saved not only for new related items but also for ALL existing related items. This causes excessive storage space and delays when updating items through the App.

Other Context & Screenshots

I think this could be reproduced a number of ways but for me, this was this easiest to observe. For the purposes of the example, I only related to 25 items in total but you can understand this becomes a much bigger problem the more items you are relating to. In my implementation, I have items with

adding_theaters
tables

Technical Details

-Device: Desktop
-OS: Windows 10
-Browser: Chrome
-Directus: 190325A
-Install Method: Cloned master branch of directus/directus

bug

All 7 comments

I have a possible extension on this one.
As far as i reproduced the problem the user needs update permission both collections and the junction collection as well.

But it should be possible to do the following (in my opinion):

User A -> has select/update/create/delete on theaters only.
User B -> has select on theaters and select/update/create/delete on movies and theaters_movies.

Hello @benhaynes
During code review for this ticket, I have found following points :
For Example : C-1 is associated with C-2 by M2M relation

On inserting record in C-1 collection by selection of existing and new records from C-2 collection,

  • Activity and revision records are creating with create action for selected new records of C-2, and with update action for selected existing records
  • Activity and revision records are creating for all the junction table records(Created during this process)

Questions :
1) Why does activity / revisions records are inserting with update action on selection of existing records of C-2 collection while creating C-1 record(C-2 existing records are just selected while inserting C-1 record, there is no actual change in its data)?
2) Are junction table activity / revisions records used in revert functionality? If no, then where do these records used?

Suggestion :

In App, I found that, on revert a record of C-1 collection, only C-1 collection field's data reverted, not selected records of C-2 collection.
If revision and activity records of junction table and for selection of existing record of relational collection are not used anywhere, we should skip saving it, because very much excessive records are storing in this process. We will store revision and activity records for create / update c-1 collection record and related new c-2 collection records.

An unusual behaviour related to this functionality in app:
In app, update existing record of c-1 collection is not working properly.

  • Update an existing record of C-1, remove some of C-2 records and add new records or select existing records of C-2.
  • Open that record in edit mode, newly added / selected records of c-2 will be there, but removed records are not actually removing.
  • I think, this issue is taking place due to wrong payload is passing in update action. And due to nested payload(related collection records) too many activity and revision records are creating in update action.

Please share your thoughts.

Hmm, this is very interesting. Thank you @itsmerhp!

I think the App is blindly updating all related (C2) items without first checking if there are differences, and that is why we're getting update records for them even if they aren't changed. Does that sound right @rijkvanzanten? The only other thing I can think of is if this is somehow related to sort values.

I know Rijk is working on a refactor for these relational data flows, so maybe this will be handled there? @bjgajjar — any thoughts on this?

@benhaynes

I think this the change related to the API. Changes in APP will not handle this issue.

@itsmerhp

I think the updation of the fields in the relational/activity table will not harm the current flow. Right @rijkvanzanten?

@benhaynes
I have checked in latest APP build and now duplicate records are not inserting in directus_activity and directus_revisions tables, so that execution time will be reduced for add/update records of the collections having M2M relational fields.

Regarding activity and revision records insertion in the junction table, as junction table is another table and it can have other fields as well, so we can't stop activity and revision records insertion in it.

@bjgajjar This is working fine in latest APP build, so can you please close this issue?

This all looks good to me — I guess it's good to store those records, and then we can choose what gets shown in the App. Of course, we don't want to slow down actions because we're saving a million records.

@rijkvanzanten — thoughts?

The problem before was the fact that the app would upload a copy of the whole object that was changed to the API, instead of just the changes. This caused the API to update fields that didn't change in the first place, which in turn caused the amount of rows in directus_activity and revisions. Saving the actual changes to revisions is not something we should remove, as that defeats the whole purpose of having these collections in the first place.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

vuhrmeister picture vuhrmeister  Â·  3Comments

24js picture 24js  Â·  3Comments

cdwmhcc picture cdwmhcc  Â·  3Comments

Nitwel picture Nitwel  Â·  3Comments

benhaynes picture benhaynes  Â·  4Comments