joplin icon indicating copy to clipboard operation
joplin copied to clipboard

Disable sync lock support

Open laurent22 opened this issue 9 months ago • 4 comments

Add an option, under Feature Flags, to disable sync locks. I think they are no longer necessary since they exist for sync target upgrades, but we don't do this anymore.

Joplin sync itself has never required sync locks since all operations are atomic.

First we put the option in the feature flags and once we are confident it works well without locks we make it the default.

laurent22 avatar May 07 '24 12:05 laurent22

all operations are atomic

what about this scenario?

  1. client A reads a note from the server, sees it hasn't been updated
  2. before it can update the note, client B pushes its changes
  3. client A pushes its change overwriting B's update

but then, i don't think a sync lock would've prevented it anyway

roman-r-m avatar May 07 '24 13:05 roman-r-m

You're right, I don't think we support this. The individual write/delete operations are atomic but checking some info, then writing something based on it is not. I guess it's never been a problem since it would require changes to be made almost at the exact same time on two different clients. And indeed the sync locks would not help with this.

laurent22 avatar May 07 '24 13:05 laurent22

tbh, i think sync should just lock the whole thing like the exclusive lock does, it'd make reasoning about sync much easier yes it'd prevent multiple clients from syncing at the same time so what

roman-r-m avatar May 07 '24 19:05 roman-r-m

Reasoning about sync, maybe, but it also brings extra complexity especially when things don't go as expected. We need to ensure sync locks can expire, but we also need to make sure they don't expire too quickly so that someone with a slow connection doesn't get locked out. Also if you import a huge ENEX file for example and sync it on desktop, meanwhile you can't use your phone at all to sync your notes.

There are probably other potential issues we aren't familiar with (since we never tried to make sync lock exclusive) and it's possible we'd simplify thing on one side but make it more complex on another side.

laurent22 avatar May 07 '24 19:05 laurent22