Take submount etag into account for folder etags

[icewind1991]

This removes the need to do cross-storage etag propagation

[icewind1991]

@nickvergessen can you smash this?

[nickvergessen]

Smashed: ✅

[icewind1991]

I think we can try to get this in as it is currently, there is still some optimizations we can do on top of this (remove the files_external propagation logic) but in current state it should improve the performance without regressions.

cc @PVince81

[PVince81]

@icewind1991 do you mind writing a more detailed explanation of the etag propagation change as discussed last week ?

I'd also like to know what @DeepDiver1975 thinks.

While I agree that this brings a perf improvement and simplifies the etag propagation code, I'm a bit worried what happens if one user has 100+ received shared folders (mount points) and for every PROPFIND the computed etag needs to compute all of them.

[PVince81]

PHPDoc ? What does this propagator propagate and how ?

[PVince81]

Raised #20521 to provide Webdav-based etag propagation tests, as discussed last week.

[icewind1991]

While I agree that this brings a perf improvement and simplifies the etag propagation code, I'm a bit worried what happens if one user has 100+ received shared folders (mount points) and for every PROPFIND the computed etag needs to compute all of them.

Modern hardware has no problem calculating tens of thousands md5 hashes a second for strings of a few hundred characters, any overhead from the hashing will be insignificant compared to the other overhead that comes with 100+ shares/mounts

[PVince81]

and if I understand well, the mounts are already resolved and the resolution is something we already did in the past, so there should be no performance regression there ?

[icewind1991]

and if I understand well, the mounts are already resolved and the resolution is something we already did in the past, so there should be no performance regression there ?

Correct

[icewind1991]

do you mind writing a more detailed explanation of the etag propagation change as discussed last week ?

Currently when propagating etags (and mtimes) we need to propagate all the way up to the root for all users who have access to the file which is complicated and expensive.

This changes it to only propagate to the root of the storage (for the owner of the file) making the etags storage in the database storage-specific etags.

When reading the etag for a folder in getFileInfo/getDirectoryContent it calculates the etag for the folder based on the "storage etag" for the folder and the etag of the root of all mounts in the folder, thus whenever a file in a mounted storage changes the etag get's propagated to the root of that storage which will change the calculated etag for the parent folders of those mounts.

[PVince81]

Smashbox tests all passed.

[PVince81]

I'm not sure whether we should merge it now or wait for @SergioBertolinSG's Webdav level etag propagation tests ? @DeepDiver1975 what do you think ?

On the other hand having it merged early gives an opportunity for more developer testing.

@icewind1991 should this be "To review" or is anything open ?

[icewind1991]

This is good to merge in my opinion

[PVince81]

I'm ok with this 👍

[nickvergessen]

Currently when propagating etags (and mtimes) we need to propagate all the way up to the root for all users who have access to the file which is complicated and expensive.

The only problem with this is, that you change it from "write" to "read"? Now we need to always resolve this when reading the etag, as opposed to only when we change something, which happens way less?

[icewind1991]

The only problem with this is, that you change it from "write" to "read"?

Yes, but the logic we need to do on read is simple compared to the very complex logic we had on write and the cost is neglectable

[PVince81]

Would be good to have integration tests too #20521

[icewind1991]

@nickvergessen @DeepDiver1975 can this be merged?

[PVince81]

• REQUIRES: smashbox tests Sharing etag propagation tests smashbox#136

[PVince81]

@DeepDiver1975 can we merge this ? The smashbox tests are almost ready but depend on the sync client version fix for owncloudcmd.

[nickvergessen]

The smashbox test has been merged.

[icewind1991]

@DeepDiver1975 @nickvergessen can the be merged?

[nickvergessen]

I will smash a fresh master with this being merged in, so we can test it with all the latest magic,
but fine by me

[DeepDiver1975]

👍

[nickvergessen]

Smashed again and passed: ✅ 👍