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

Update file cache before doing file operation instead of after, rollback possibility #24665

Closed
PVince81 opened this issue May 17, 2016 · 8 comments

Comments

@PVince81
Copy link
Contributor

PVince81 commented May 17, 2016

So far for any file operation the operation is first done on the storage and only if it succeeds, the file cache will get updated.

How about doing it the other way around ?
If we first update the file cache to reflect what is intended, then do the operation.
If the operation fail, roll back the cache update.

Rolling back a database update is easier than rolling back a filesystem operation, especially knowing that it could affect other systems when using external storages (when accessing outside of ownCloud).

@icewind1991 @DeepDiver1975 what do you think ?

Could also help handle the tricky rollback cases when moving folders. @MorrisJobke

Suggested by @sagamusix here #24625 (comment)

@MorrisJobke
Copy link
Contributor

Rolling back a database update is easier than rolling back a filesystem operation, especially knowing that it could affect other systems when using external storages (when accessing outside of ownCloud).

That makes sense. But then we also need to think about what happens if only a part fails (e.g. moving a folder).

@PVince81
Copy link
Contributor Author

Had a chat with @schiesbn and he thinks the database should only be a representation of what is on filesystem, not the other way around. In this case if a DB operation on the filecache fails for whatever reason, we'd rather need a way to schedule a quick rescan of the affected folder. It would be more a "cache is not update to date" situation than a rollback.

@faulix
Copy link

faulix commented May 17, 2016

Is there actually a possibility to do a rescan not for a whole user, just for a certain subfolder? In that case we could trigger a rescan of a certain subfolder (maybe even not recursive) in case of "file doesn't exist" errors. That would also solve the ghost-file problem.

Close to this topic there is also an other option: Right now external storages are considered to be the master and OC is the slave. What about adding an option to choose whether OC should be master or slave and in case of master all files are stored locally and all operations get done on the local file system and afterwards synchronized. But this would most probably require asynchrones operations again, as OC would need to retry the synchronization in case it fails.

@PVince81
Copy link
Contributor Author

Having multiple modes would definitely be too complex.

Rescanning a folder is a good way, yes.

@rullzer
Copy link
Contributor

rullzer commented May 18, 2016

Really... rescanning is the fallback?
Why not use proper transactions... as you know... that is what they are for...

@PVince81
Copy link
Contributor Author

@rullzer long-lived transactions could cause database lock timeouts, in the following scenario:

  1. begin transaction
  2. update file cache
  3. do file operation
  4. commit or rollback depending on the result

If the file operation takes too long, the table rows (or even the full database in the case of sqlite) would be locked and unavailable to other processes.

With rollback above I was mostly referring to explicitly re-inserting the deleted rows in case of errors.

@rullzer
Copy link
Contributor

rullzer commented May 30, 2016

mmm ok fair enough... but I think this will lead to a world of pain.
Shares will break etc.

@PVince81
Copy link
Contributor Author

PVince81 commented Oct 7, 2016

Or the better alternative, async file operations: #24509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants