ssbc / async-append-only-log

A new append-only-log for SSB purposes
16 stars 6 forks source link

Make multiprocess-safe #27

Open KyleMaas opened 3 years ago

KyleMaas commented 3 years ago

Right now if two processes open the same log file and are both able to write, they can garble the log by writing over each others' data due to each using a write offset held in non-shared RAM.

Looking at the code, I'm not even sure how you'd fix this. I'll have to study this some more. It'd be good to at least document this behavior so people know not to do it. But it'd sure be nice if it had some method of actually fixing this.

KyleMaas commented 3 years ago

I'm thinking maybe one way to do this without impacting performance unnecessarily would be to:

  1. Break log offset calculation up from the initialization process so it can be run again later as a function.
  2. Rename write() to writeSingle or something so we have one call which writes the full queue that gets called by debounce() and a separate function to bounce back and force with writeBlock like it does now.
  3. Have write() either lock the file or use a cross-process mutex to prevent multiple concurrent writes. Before writing, stat() the file. If the length differs from what is expected, re-initialize the offsets using the function from (1) and recalculate each of the block offsets in the write queue. Theoretically, on any system with an in-memory filesystem cache, this stat() call should be able to come from RAM and should be fairly fast, RAF's implementation details being the wildcard here. Plus, it's only done once per debounced batch. The write can then proceeed and afterward the mutex can be released.

From what I understand of the code, this should mean that none of the concurrent writers' caches would need to be invalidated, and the offset calculation looks to be relatively quick and would only have to be recalculated in the case of a collision with another process.

staltz commented 3 years ago

Yeah, conceptually we didn't account for multiple writers at all (e.g. also there cannot be two Electron apps open at the same time). I think for the browser-specific use case, a service worker dedicated to asyncAOL could maybe solve this.

arj03 commented 3 years ago

Yeah it goes a bit deeper than that, like there is a protection in jitdb from running concurrent index update for example, again that is only single-process. We had a similar problem with multiple programs trying to run on the same log and the solution there was to do something like 1 daemon that all programs used. I'm going to run a few experiments and see what I can come up with for the browser.

KyleMaas commented 3 years ago

Yeah, concurrency's hard. To quote Eiríkr Åsheim:

Some people, when confronted with a problem, think "I know, I'll use multithreading". Nothhw tpe yawrve o oblems.

And for the case of ssb-browser-demo, it's probably easier to handle it on the browser end of things. But for this project, concurrency's not a conscious design choice. It's just a possible use case we have no protection against.

But in the general case, it'd be nice if there was some protection here as well. I might just do some experimenting with this to see if I can get it to work fast enough to make it worth using. It's easy enough to make a single-threaded unit test which breaks this the same way - just open the same log file in two different objects. And there may be other stuff that needs concurrency protection as well, but if each one gets worked on, eventually we may have something.

Not that concurrency's the intended use case. But it'd at least be good if it didn't corrupt data if you open two tabs, or open two Electron apps, or any other sets of two clients both using the same log. This would be a low-priority thing to guard against an edge case, so it's not something I plan on working on right away. But...eventually.

staltz commented 3 years ago

Fully agreed! By the way, flume uses ReadWriteLock for that purpose, I think. https://github.com/flumedb/aligned-block-file/blob/master/file.js