Caching remote discovery results

[guruz]

What about caching discovery results in the journal?

Normally this should not be needed as we're always syncing all files/dirs and storing the proper ETag in the journal.
However in practice I think there's often issues like fatal sync errors, network timeouts etc which then on the next sync mean a complete re-discovery..

Something to chat about next week.

@ogoffart @dragotin @ckamm

[MorrisJobke]

Currently we noticed some problems with randomly broken connections to an LDAP server. So the user backend can't properly hand back the home directory. To avoid random guessing what the correct home directory could be we hand back an exception (owncloud/core#15332). In this case the sync client also could see such responses (that will cause a HTTP 500). If this happens during discovery the whole discovery process is aborted and starts from scratch.(and maybe never finished because of this) Is it possible to just repeat the last (failed) request (because it is likely that the error was just temporarily caused) and then resume the discovery from that point? We maybe should improve the error message that is handed back to say "this was just a temporary error and could be fixed with the next request". (503 with Retry-After header)

That would make the client a bit more robust for these cases.

[guruz]

@ogoffart @ckamm @jturcotte @PVince81 @DeepDiver1975 Now that we're thinking of doing the recursive/infinity PROPFIND I'm wondering if it makes sense to think about this again.

The infinity PROPFIND could be implemented by having it fill a local cache.
map (remote_dirName+ETag)->contents

The issue was originaly about any discovery issue. It would also help with flaky network connections, basically everytime when the discovery would abort.

This should be implemented in a way to not fill so much memory again. (e.g. sqlite3?)

[ogoffart]

that cache already exists. It's called the database (sync journal)

[guruz]

Please read the bug above. This is about failed syncs and people complaining about the long repetive re-discovery

[guruz]

...which means it could be flushed after a successful sync.

[guruz]

Similar how it was implemented before: 980c176

[dragotin]

@ogoffart 💟
@guruz isn't it true that we only do real propfinds into a tree if the ETag of the directory has changed, and read from the client journal instead if not?

IIRC what we discussed long time ago was to keep the entire tree in memory to not have to rebuild the whole tree again, maybe that should be discussed again.

[ogoffart]

It's true we could have another table in the database if we want to persist the value between failled sync.

[dragotin]

@ogoffart how would that look like? And what would make you confident that it is still valid? What would it make different from the now existing table?

[guruz]

If we want to persist it between failed syncs but for simplicity not between client restarts, we could use a sqlite3 temporary DB which gets automatically cleaned up and if small enough stays in memory: http://stackoverflow.com/a/32004907/2941

I was so far only thinking about a simple key value store: (remote_dirName+ETag)->dir_contents
The dir_contents could even be plain XML returned from PROPFIND. We dice the incoming infinity PROPFIND into single PROPFINds or if we request a single PROPFIND we put it in directly.

[guruz]

And what would make you confident that it is still valid?

Having ETag in the cache key.

[ogoffart]

This is the current algorithm:

remote_discovery(folder) {
   folder_from_network = run_propfind(folder);
   folder_from_db = load_from_db(folder);
   if (folder_from_network.etag == folder_from_db) {
      readFromDb(folder);
   } else {
      file_tree_walk(folder_from_network);  // recursively go to every folder
  }
}

The new alorithm would looke like:

remote_discovery(folder) {
   folder_from_network = run_propfind(folder);
   folder_from_db = load_from_db_recursively(folder);
   if (folder_from_network.etag == folder_from_db) {
      readFromDb(folder);
   } else {
          folder_from_cache = load_from_cache(folder);
         if (folder_from_network.etag == folder_from_cache) {
              readFromCache(folder);
        } else {
            file_tree_walk(folder_from_network);  // recursively go to every folder
            add_to_cache(folder_from_network);
       }
  }
}

sounds pretty simple

If we want to persist it between failed syncs but for simplicity not between client restarts,

I think if it's in the db, it's as simple to make it persist accross client restart. We'd clear the cache after every successful sync anyway.

[ckamm]

I agree that it makes sense to cache PROPFIND responses - even if the rest of the sync failed and the resulting changes aren't propagated yet because of it.

[ckamm]

I agree with @ogoffart - it should be relatively straightforward since our metadata table already functions as a propfind cache. Just having a second table with the same kind of staleness and retrieval logic should work. Data could be cleared when the data arrives in the journal.

[ckamm]

I've started looking at this.

[ogoffart]

I think it would be better to do the discovery and propagation in parallel

Now that the discovery has been refactored, it should be easier.

Then the cache wouldn't make sense anymore

[ckamm]

@ogoffart Yeah, anything that reduces time between server query and writing to the db would do, and parallel propagation would have other advantages. Need to deal with deletes though. Let's discuss!