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

eTag for metadata changes #8477

Open
tobiasKaminsky opened this issue Feb 21, 2018 · 22 comments
Open

eTag for metadata changes #8477

tobiasKaminsky opened this issue Feb 21, 2018 · 22 comments

Comments

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Feb 21, 2018

Updated version:

Provide an eTag for metadata changes (favorite, share, note, etc).
Currently clients (Android / iOS) check for changed eTag when entering a folder.
If this is the same with stored database no real propfind to list folder content is needed.
However if e.g. a new sharee was added, we still need to retrieve it. Therefore we check every time(!)
/ocs/v2.php/apps/files_sharing/api/v1/shares

This has multiple drawbacks

  • more server load
  • more client load
  • UX on client side is not ideal, as sharees are delayed until info is fetched, seeing a spinner

On next android version we wanted to rely on changed eTags when syncing a folder:

  • enter a folder
  • check remote eTag with locally stored eTag
  • refresh content only if eTag differs

The problem now is that with this approach we do not get info about favorited/unfavorited folders. (encrypted/not encrypted I have not checked yet).

In my opinion at least the parent folder should then have a changed eTag to reflect that something has changed.

@skjnldsv @icewind1991

@rullzer
Copy link
Member

rullzer commented Feb 21, 2018

Probably we should have some way to trigger a parent update on all metadata changes.

@tobiasKaminsky
Copy link
Member Author

Any news here?
This is already included in android and planned to be in next release, which is in mid march.
Of course it needs a fallback for older server (if we do not backport the solution).

Changing the parent etag as @rullzer suggested should be the best.
Then we know that something changed and sync child's metadata.
Same for client, but as no file changed, it does not redownload anything.

@rullzer
Copy link
Member

rullzer commented Feb 28, 2018

The only problem I recently thought of is that just chaning the parent etag is not enough.

Consider:

  1. userA shares a file with userB
  2. now some shared tag is added to the file by userA
  3. now we need to update 2 locations to make sure it get propogated properly everywhere

Now with two users this is fine. But now imagin sharing a file to a group of 2000 users.

@tobiasKaminsky
Copy link
Member Author

Yeah, slight overhead ;-)
But is not the same happening if you put a file into a folder which is shared with 2k users? Then also the parent eTag has to be changed and propagated to all sharees...

@rullzer
Copy link
Member

rullzer commented Feb 28, 2018

Well yes but that happens only when those users request the info. Because the etag of the file changes so we can delay it until the info is requested. In this scenario we would have to do it on all changes.

Basically we would need to change the etag of the file and then it all would work. But doing that would lead to a redownload in the desktopclient.

@tobiasKaminsky
Copy link
Member Author

Then check the modification date, it eTag changed, but mod date not, you know it is only metadata ;-)
Feels very hacky -_-

@tobiasKaminsky
Copy link
Member Author

@rullzer any news here?
We want to do feature freeze / release 3.1rc1 on 22.03. and this should be in, or we would have to revert stuff on android.

@rullzer
Copy link
Member

rullzer commented Mar 9, 2018

revert to old stuff. Nothing will be fixed before 22.03

@tobiasKaminsky
Copy link
Member Author

Sad ;-)
On tests I also saw that share/unshare also does not change the eTag.

@tobiasKaminsky
Copy link
Member Author

Reminder for myself: If we have a server fix, we need a new PR for android.

@tobiasKaminsky
Copy link
Member Author

This was scheduled for NC14, but got delayed.
As mentioned on NC15 meeting, please let us do it for 15.

@nextcloud-bot nextcloud-bot removed the stale Ticket or PR with no recent activity label Sep 10, 2018
@tobiasKaminsky tobiasKaminsky mentioned this issue Nov 7, 2018
29 tasks
@MorrisJobke MorrisJobke added this to the Nextcloud 16 milestone Nov 7, 2018
@tobiasKaminsky
Copy link
Member Author

@rullzer @icewind1991 can this new eTag also change when a share is changed?

@MorrisJobke
Copy link
Member

Most likely nothing that still can be done. Sorry - see my comment in #14228

@MorrisJobke MorrisJobke modified the milestones: Nextcloud 16, Nextcloud 17 Mar 6, 2019
@tobiasKaminsky
Copy link
Member Author

Sad to hear, especially that this was already moved from 15 to 16…
At least we have a PR now ;-)

@skjnldsv skjnldsv added the 0. Needs triage Pending check for reproducibility or if it fits our roadmap label Jun 12, 2019
@blizzz
Copy link
Member

blizzz commented Oct 19, 2022

still an issue?

@tobiasKaminsky
Copy link
Member Author

Yes. We would benefit quite a lot by this!

@tobiasKaminsky
Copy link
Member Author

With nextcloud/android#10783 we see this is one of our biggest area to improve.
Most of time is spent on refreshing shares, as we have to do this all the time, as we do not know if it changed.

@PVince81
Copy link
Member

PVince81 commented Dec 16, 2022

possible approaches:

1a) reuse the etag and make it change when a share, share permissions, acl, favorite, tag, etc changes

  • change the etag of the parent folder in which the files' metadata has change, but don't change the etag on the file itself. Here assuming that semantically we mean that a folder is like a file and when its contents (listing, metadata) changes, its etag changes

  • pros: already propagate, no new code needed except for the new locations where we trigger the etag change

  • pros: should not disturb old clients, they'd only do the discovery phase deeper without noticing changes.

  • cons: no distinction between content change

  • cons: does this clash with the Webdav spec in terms of semantics ? (we probably already aren't fully compliant, see ctag vs etag concept)

1b) like 1a but change the etag on a file also whenever its metadata changes

  • pros: might be cleared to a client and no need to PROPFIND the parent when querying single files (ex: "keep in sync" mode)
  • cons: is there a risk that files will get redownloaded ?
  1. a new PROPFIND attribute "metadata-etag"
  • Same like 1a and 1b but it would be a separate etag attribute.
  • pros: semantic separation of file content and file metadat
  • cons: needs more effort on the server side as we need to extend the propagation logic to propagate a second etag

any further approaches or tweaks to the above ? @tobiasKaminsky

@tobiasKaminsky
Copy link
Member Author

1a/b) cannot work as eTag is currently used to distinguish when to sync a file.
So if eTag changes without it will cause a redownload of a downloaded file.
Thus this must be a separate information.

I fear 2 is the only solution.
We discussed this already quite a lot and never found another way…

@PVince81
Copy link
Member

@icewind1991 can you estimate the effort of adding an additional metadata-etag and triggering its change in various code locations, and propagating it like the initial one ?

are there any cases that we forgot to cover, like for example whether the files HPB should also expose it somehow ?

@tobiasKaminsky
Copy link
Member Author

I remember that we even started with it at some point:
https://github.com/nextcloud/server/search?q=metadata_etag

So DB is "ready" according to my limited knowledge.
Which would make it easier to add, as it is then no big impact during upgrade.

@starypatyk
Copy link
Contributor

I would like to mention that my PR #34918 is complementary to this one.

It helps in the current situation - without the metadata eTag. Will still help when the metadata eTag is implemented - in cases when the share data actually changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🧭 Planning evaluation (don't pick)
Development

No branches or pull requests