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

Handle / simplify batch fs events #140

Closed
Tracked by #501
walkah opened this issue Oct 28, 2020 · 6 comments
Closed
Tracked by #501

Handle / simplify batch fs events #140

walkah opened this issue Oct 28, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@walkah
Copy link
Contributor

walkah commented Oct 28, 2020

Summary

Problem

Currently, the API requires that developers explicitly call fs.publish after any "mutation" (mkdir, add/write, etc). This reduces the backend load / updates and makes it more performant on batch operations.

Impact

However, it's reasonable to assume that developers may miss the two step process and assume that write will create the file and it will be subsequently ("instantly") available.

It's worth some discussion on the best approach here. Ultimately, with a goal of making "the right thing the easy thing". Some early options are:

  1. Leave the API as is, but improve the documentation / examples.
  2. Tweak the functions (mkdir, write) to accept a single object or a list, automatically publish at the end, but batch the list of updates.
  3. Have distinct / separate methods for either single (write()) vs batch (batchWrite()) or async (write()) vs sync (writeSync()) - where the former will batch operations and publish them at some point, the latter performs the operation and immediately publishes.
@walkah walkah added the enhancement New feature or request label Oct 28, 2020
@expede
Copy link
Member

expede commented Oct 28, 2020

...just riffing here...

after any "mutation"

I think you've hit the nail on the head here for several reasons, including that queries are automatically handled currently.

batchWrite()

There's definitely something in here about grouping actions. I wonder if it's more flexible and easier to think of to say "these actions together" as opposed to "this batch of files writes" and "this batch of directory changes". Something I mentioned internally was perhaps leveraging the nondestructive nature of our tech to provide a transactional API. Depending on the exact background of the users, that could look something like:

const fileDataA = foo

atomic { // or "transact"
  fs.write(filePathA, fileDataA)
  fs.delete(directoryPath)

  const fileB = await read(filePathB)
  const fileAB = fileDataA ++ fileDataB

  file.write(filePathB, fileAB)
}

// or more friendly: batch{...}, together{...}, or atOnce{...}?

Where all of those actions happen together or not at all, with an implicit publish() at the end.

The advantages are that it crosses queries and mutations, mix all CRUD actions, and lets you do things like automatically roll back if you throw partway through updating, automatic retry, and even potentially auto-rebasing over data if you were behind DNSLink for some reason.

Atomicity also something that we get more-or-less "for free" from our tech, and that's much more different on other systems.

@bgins
Copy link
Member

bgins commented Dec 9, 2020

Option 2 and the grouped actions per @expede are both appealing.

Option 2 makes sense as the "easy thing". Once you learn that you can pass a function an object or a list, you can apply that knowledge to all of the other functions. (Are there exceptions where a list wouldn't work?)

The grouped actions would be a slightly more advanced technique, where you can get more power when you need it.

Somewhat related, it would be useful to be able to cancel an action. For example, say a write() is started, but a second write() to the same file is requested before the first one completes. It would be useful to cancel the first write. This would make sense in a grouped action where you want to make sure to cancel before the second write.

@expede
Copy link
Member

expede commented Dec 9, 2020

Are there exceptions where a list wouldn't work?

Depends on what we try to enable. If you want to be able to have things like automatic retry, then the closure needs to be free of side effects. Cancelation also may lead to confusing results in that scenario if it's not clear what's happening.

That said, cancelling an action inside a write block is always possible.

Are there exceptions where a list wouldn't work?

WNFS is nondestructive, so you can always revert to a previous version of a file. Transactions make this very natural, but yeah, can't hurt to build in some sort of try and/or revert mechanism in't the WNFS DSL.

I'd say in general we'd want to encourage people to apply changes locally and sync at the end, since we 1. probably don't care about all that intermediate state, and 2. zero loss of functionality locally while allowing very fast operations, and making those "durable" (syncing DNS) at the end of the semantic block.

@gyuri-lajos
Copy link

gyuri-lajos commented Dec 16, 2020

"apply changes locally" "dont'care about all that intermediate steps"
I agree. Having used WNFS for over a month and using Orbitdb for a long time, so it is all the same technology underneath, in practice when you switch between devices you have to "refresh" the browser anyhow, and you need to "wake your" peers.
In fact there should be a way to indicate or discover automatically that you started using the app on a device.
At that point the missing publish can be executed.

@gyuri-lajos
Copy link

Guarding against data loss is of paramount importance. I have added code to add files keyed by path to a localforage database so that I can recover and sync on request across devices if and when they get out of sync, Which did happen twice.
In any case I think doing something like that would be a prudent thing to do.

@icidasset
Copy link
Contributor

Implemented in #500
Part of #501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants