Race condition can cause notebook entry tag updates to be lost

[akhenry]

Summary

There is a race condition that can occur when two users update the tags on the same notebook entry at the same time. The race condition will cause one of the user's tag update to be lost. eg. If one user deletes a tag, and another user adds a tag to the same entry at the same time, the first edit (to remove the tag) is retained, but the second edit (to add a tag) will result in a conflict and an error. An identical issue can occur when adding or removing notebook entries. In this case we have some code that detects the conflict and merges the entry lists. Something similar will need to be implemented for notebook entry tags.

Expected vs Current Behavior

The edits should be merged so that both user's changes are retained.

Steps to Reproduce

1. Using a test environment with Couch DB persistence
2. Open Firefox and chrome to the same notebook
3. In Chrome, throttle the network to Slow 3G (or slower - you need enough time to switch browsers and make an edit)
4. In Firefox, delete a tag from a notebook entry
5. Quickly switch to chrome and add a tag to the same entry
6. Observe in the JavaScript console that a conflict error has occurred.

Environment

• Open MCT Version: 488cd82
• Deployment Type: local dev (with Couch DB)

Impact Check List

• Data loss or misrepresented data?
• Regression? Did this used to work or has it always been broken?
• Is there a workaround available?
• Does this impact a critical component?
• Is this just a visual bug with no functional impact?
• Does this block the execution of e2e tests?
• Does this have an impact on Performance?

Additional Information

[scottbell]

Replicated:

CouchObjectProvider.js?9318:236 Uncaught (in promise) Error: Conflict persisting undefined
    at #handleResponseCode (CouchObjectProvider.js?9318:236:1)
    at CouchObjectProvider.request (CouchObjectProvider.js?9318:211:1)
PUT http://localhost:5984/openmct/d3c48fe5-bc69-4e6a-be35-1beb6a7152c8 409 (Conflict)
  | request | @ | CouchObjectProvider.js?9318:208
  | #updateQueued | @ | CouchObjectProvider.js?9318:676
  | update | @ | CouchObjectProvider.js?9318:691
  | save | @ | ObjectAPI.js?99d1:387
  | openmct.objects.save | @ | monkeyPatchObjectAPI…otebooks.js?b478:16
  | mutate | @ | ObjectAPI.js?99d1:513
  | mutateObject | @ | notebook-entries.js?31f2:212
  | updateEntries | @ | Notebook.vue?7f91:812
  | updateEntry | @ | Notebook.vue?7f91:805

Going to look into merging entries upon conflict now.

[scottbell]

I think I've got the basic conflict resolution worked out, but running into some issues where if the annotation has never been created before (i.e., no tags have ever been made on an entry), and two users try to tag entry at the same time. In this case, knowing which annotation object to use is tricky.

[scottbell]

@akhenry Some ideas to fix:

• Create an annotation ahead of time when creating an annotation with a transaction, thus the annotation object is already created when the entry is created, reducing possible conflicts. Conflicts are still possible if two users quickly create an entry.
• Use the _id in CouchDB as form of uniqueness for notebook entries. For example, if the notebook entry ID was 175c6d78-6814-444a-b5be-b3465f8ae443, the _id of the annotation object would be 175c6d78-6814-444a-b5be-b3465f8ae443-notebook-annotation. This would ensure exactly one annotation per tagging object. The downside is we now have an non-UUID _id.
• Embrace multiple annotations being created per notebook entry and handle them properly. Always show the union of all the annotations in the notebook entry. Have any additions go to the last created.

[akhenry]

I initially envisioned each tag being its own annotation (although not in anticipation of this problem - at least not consciously), so option 3 would align with that. It also completely skirts around the problem of notebook conflicts, which is nice. It's not a panacea though, it introduces the possibility of duplicate tags if two people add the same tag at exactly the same time. That said, it's a pretty benign edge case, one of the dupes can just be deleted. Of course, both users may spot the duplicate at the same time and decide to delete their annotation at the same time and get stuck in an infinite loop...In any case, the less often users are trying to modify the same object, the fewer opportunities for conflicts.

How much work do you think it would be to make each tag a separate annotation?

[akhenry]

Also, are there performance implications for making each tag an annotation? Is it going to just completely destroy our performance?

[scottbell]

@akhenry I don't think it'll kill performance, but we'll certainly have more annotations. I prefer having one annotation per tag too, though this will be a bit of a backend overhaul. Do we want this to be backward compatible? If so, I'd suggest:

• keeping multiple tags per annotation
• handle the edge case of having tags spread across several annotation

If not, we can:

• have one annotation per tag
• handle duplicates on the front-end, ensure when we delete, we delete all of them.

[scottbell]

After discussing with @akhenry, we think we don't need to be backwards compatible, and thus we'll do:

• have one annotation per tag
• handle duplicates on the front-end, ensure when we delete, we delete all of them.

[scottbell]

Saving this here for my memory:

Me: Annotations questions: with one annotation per tag, what does it mean when a user deletes a tag? Do we delete the Previously we didn’t delete annotations, but we did remove the tags therein. If we have one annotation per tag though, and the user deletes a tag, we’ll probably need to also delete the annotation, as otherwise annotations will pile up if they add the tag back (and a thus new annotation is created). Or we can add more information to the annotation (similar to the “soft delete” option with domain objects) that can be toggled.

@akhenry: "Given the number of tags there are likely to be, maybe we should do a soft delete?"

Me: Excellent, will move forward with that

[scottbell]

@akhenry I'd also like to make sure this works with plots, and in that case, the user will be drawing a marquee across the plot to select points, and then tagging the point selection. In that case, I think we'll want the plot annotation (which is a target and a polygon) to have multiple tags, which means (I think) we won't be able to reuse the tagging components developed here, right? Or at least not as easily.

[ozyx]

Closed by #5763

[jvigliotta]

Wasn't able to test, FF won't connect (cert issue) and safari can't throttle (as far as I could find)

[ozyx]

Unable to determine. Testathon 10/4/22

Since FF doesn't play well with Testathon, and Safari inexplicably cannot handle network throttling, we had many people add and delete tags to one entry in one notebook.

There were a couple 409 conflict errors that were raised on my end, but not sure if this means this is not fixed?

[akhenry]

@akhenry to test

[akhenry]

Fixed!

There is an issue now that Notebook conflicts are being reported to the user, as all object conflicts are, but that's unrelated.