concurrency bug!

[dominictarr]

After noticing that patchcore wasn't handling branches correctly
ssbc/patchcore#18
ssb-junkyard/ssb-msg-schemas#17
I realized that any collaboratively editable structure should use something similiar.
so I had a look at the patch gathering code,
and I saw it actually had a concurrency bug
https://github.com/pietgeursen/patch-gatherings/blob/master/gathering/obs/gathering.js#L81-L97

it uses the update with the latest timestamp, which is one thing,
but unfortunately the timestamp on the links response is the locally received timestamp.
That means it's possible that two different people build a different looking event.

Picture this situation: alice and bob are gonna meet up, and use a gathering. in another channel they discussed maybe having to change the location. They end up doing this, but while they are offline. They sync within 24 hours, then meet the next day, but they both went to the place the other suggested. What happened?

Well, alice made her change, and her local receive time is right now, because she made it. Then, a day later, she receive bob's change, which appears more recent. The same thing happens to bob, but in reverse. So, when they see the location, they both see the location the other one suggested. hilarity ensues.

some discussions around this thread
%GcvjVk+NLsjOB5Vd+vceGXEOeYmHoRA6lgXVJzPuMxw=.sha256

Anyway, if we had branches (updates should use branches=ssbSort.heads(thread)) we'd know the causal order and would be able to show alice and bob they have both edited the event, or automatically accept one of them. it's it's important that the humans actually know the details of the event, I would lean towards to former.

Also, side note, the way that this uses subscribeToLinksByKey will produce 5 times as many database requests as necessary.

[pietgeursen]

Cool, thanks for this.

Also, side note, the way that this uses subscribeToLinksByKey will produce 5 times as many database requests as necessary.

Just looked at the code and can't see why this happens. Can you explain why it happens and what best practise is?

[dominictarr]

oh okay now looking again I realize I'm wrong about that. It looked like subscribeToLinksByKey was doing a query but now I realize it's not.

however, I would recommend separating the merge logic from the IO, because this setup will be quite difficult to test (and you'll need to setup the whole sbot stack, etc)

If you have a reduce function that is passed a "thread", i.e. all the messages in a gathering, and then reduces them into the current state of those various edits. This means it will be really easy to test, and if you find some bug in practice, you'll be able to copy those messages into the test case as JSON and run the same code on them. distributed systems are hard and you need good test cases!

[dominictarr]

oh, also, on initial sync, it's very likely to have weird received times, quite possibly receiving all edits by one author after another.

[mmckegg]

I wonder if something similar should be used in ssb-about which is what patchwork is using for doing gatherings.

In patchwork it's using the user timestamp to decide the most recent (in patchcore about.obs.latestValue), but using ssb-sort could be good there also!

[dominictarr]

now that user timestamp is purely a claimed time stamp, someone can set something with time significantly in the future, and that will mean other peers won't be able to edit that value because they'll be using the correct time. one troll can ruin it for everyone!