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

Manage concurrent edits of notes #56

Closed
korelstar opened this issue Jan 25, 2017 · 5 comments · Fixed by #692
Closed

Manage concurrent edits of notes #56

korelstar opened this issue Jan 25, 2017 · 5 comments · Fixed by #692
Labels
feature: API Related to the API for 3rd-party apps, i.e. Android or iOS feature request Requests for complete new features
Milestone

Comments

@korelstar
Copy link
Member

korelstar commented Jan 25, 2017

Currently, we have no proper handling of concurrent edits. Those can happen in many scenarios and it results in lost updates:

Scenario 1: Shared notes

  • User A shares a note to User B
  • both users open their notes app and begin to edit
  • when User B finishes after User A, all changes made by User A will be overwritten by User B without notice by User B (or User A)

Scenario 2: Mobile clients

  • the user makes changes using a client app on the mobile phone without network connection
  • later, the user edits the same note somewhere else, e.g. on a desktop computer using the web app
  • when the mobile device reconnects and synchronizes, one note overwrites the other without notice by the user

I would like to improve this situation and I thought about a strategy how to cope with this. We are experiencing a classical lost update problem, due to a write-write-conflict. Traditional databases use locking to prevent this. However, in our situation, we have a distributed system, where locking is not appropriate. Instead, I would propose to use optimistic concurrency control. This means, we allow always editing and try to resolve conflicts later on.

To do this, two steps are needed: 1.) Identify conflicts, and 2.) Resolve conflicts.

1. Identify conflicts
In order to check, if a note has been changed in the meanwhile (from getting the current state from the server to putting the changed data back to the server) we could use the ETags I proposed in #49 (in summary: it represents the state of the note by using a checksum over all attributes).
The client (mobile client or web app) has to save the current ETag for each note and update it on every synchronization (this is required for #49, too). When the note is edited, the client sends the HTTP header If-Match: "etag". The server (i.e. NotesController resp. NotesAPIController) checks if the ETag from the HTTP request header matches the current ETag from filesystem/database. If true, the note is changed and sent back to the client. If not, the server sends the HTTP response code 412 Precondition Failed and the current server state of the note in the body of the response. The client now sees that the note has changed in the meanwhile and resolves the write-write-conflict (see 2.)).
These changes are totally backwards-compatible and conform to the HTTP protocol.

2. Resolve conflicts:
The client now has data from local changes and remote changes and has to do something. This is harder than the first step and multiple reactions are possible.

a) Let the user decide: ask the user whether i) overwrite local changes, ii) overwrite remote changes, or iii) save local (or remote) changes as new note.
b) Let the user merge: provide an interface which allows for merging the files (you know it from your version control).
c) Try to merge automatically: merge all changes automatically, e.g. using the google-diff-match-patch (Demo, Code) library.

I think, user interaction has to be avoided, so I'm preferring c), but I don't know if this will always work.

However, I wanted to write down my thoughts on this so that everybody can participate on a discussion on this. The first step is easy to implement and must be done in this app (controller). The second step is much more comprehensive and requires changes on those clients (i.e. this app, Android app, iOS app) which should be concurrency-save.

Any comments are welcome :-)
How high do you would estimate the importance of this issue?
Does nextcloud/server (resp. its clients like nextcloud/android and desktop clients) deal with this issue?


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@korelstar korelstar added feature: API Related to the API for 3rd-party apps, i.e. Android or iOS enhancement New feature or request labels Jan 25, 2017
@jancborchardt jancborchardt added this to the 2.5.0 – 🚀 New nice features milestone Jun 5, 2018
@korelstar korelstar removed this from the M2 – 🚀 New nice features milestone Dec 21, 2018
@korelstar korelstar added feature request Requests for complete new features and removed enhancement New feature or request labels Dec 14, 2019
@stefan-niedermann
Copy link
Member

@korelstar are there any updates on this?

I think this issue has been described very well by you.

Why not go way c) and try to merge automatically on server side and if this is impossible due conflicts

1.) first, easy step; attach the lines that could not be merged under each other (imagine like a git merge conflict)
2.) second step, more todo; send a 4xx http status code containing the note on the server and let the mobile clients (yep, that's a todo for me) manually merge the notes in a dialog or so.

This way (only implementing an automatical merge and displaying conflicting lines under each other) the most problems could be solved (at least i believe that most "conflicts" should be mergable automatically).

At least there is no more data loss.

@korelstar
Copy link
Member Author

korelstar commented Jan 12, 2020

Yep, too bad that we haven't implemented this yet. But for the web app, this will be fixed by implementing #331. However, this wouldn't be a solution for the Android app. So I'm unsure, which way we should go for the Android app.

Of course, we could go the way I described in the initial posting, here. But we can't do step 2 before step 1. What I described under c) has to be done on the client, since the server has the current note, only. The server has no information about the note state before the mobile client changed it. Hence, the server can't create a diff of those changes. But this is required in order to apply these changes on the current state.

Ergo: The server part is easy to implement, I'd love to do this! But the hard part is in the Android app. So if you would like to take over this part, let's start it! 😀

When you will really start, please ping me, I have some ideas on implementing the backend in the Android app that I could share.

@stefan-niedermann
Copy link
Member

I disagree, the server has the new content.

As soon as i POST the new note.

@korelstar
Copy link
Member Author

Yes, of course. But the server doesn't know from which state the mobile user has made his changes. This is required to gather the diff from those changes. Then these changes can be applied to the current state from server. Otherwise, the server can't merge the changes, it can only overwrite the changes with those from the mobile user (like it is currently implemented).

@korelstar
Copy link
Member Author

Finally, I've implemented this: please have a look at #692 !

Third-party clients have to implement their part, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: API Related to the API for 3rd-party apps, i.e. Android or iOS feature request Requests for complete new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants