Skip to content

Conversation

@PVince81
Copy link
Member

@PVince81 PVince81 commented Jan 5, 2022

Remove scanAll which relies on the "shareinfo" endpoint that returns the
full cache tree.
The latter can become big for big shares and result in timeouts.
Furthermode, the full tree would be retrieved again for each and every
detected change which can become expensive quickly.

As discussed with @icewind1991

Draft for now, needs more extensive testing to confirm that the default scanner will work correctly for federated shares.

Fixes #29941

@PVince81 PVince81 added bug 2. developing Work in progress labels Jan 5, 2022
@PVince81 PVince81 self-assigned this Jan 5, 2022
@PVince81 PVince81 force-pushed the bugfix/29941/remove-shareinfo-query-from-scanner branch from 1115141 to a8dea0f Compare January 7, 2022 08:17
@PVince81
Copy link
Member Author

PVince81 commented Jan 7, 2022

cool, no failure related to federated sharing

  • still need a bit more manual testing though, especially with deeper folder structures and also see how the desktop client reacts

@PVince81
Copy link
Member Author

PVince81 commented Jan 7, 2022

I've done a manual test as follows:

  1. Setup two instances A and B
  2. Setup user alice@A
  3. Setup desktop client for alice
  4. Login as admin@B
  5. Create a folder "a/b/c/d"
  6. Share "a" with alice@A
  7. As alice, accept the share
  8. Wait for sync
  9. As admin, create a new folder "a/b/c/d/e"
  10. Wait for sync

After the sync, the new folder "e" got synced.
I've also observed that the etag changes properly in the receiving end's storage.

So it all works fine with this PR 😀

Please review!

@PVince81 PVince81 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 7, 2022
@PVince81 PVince81 force-pushed the bugfix/29941/remove-shareinfo-query-from-scanner branch from a8dea0f to 6b081a7 Compare January 7, 2022 12:52
@PVince81 PVince81 marked this pull request as ready for review January 7, 2022 12:52
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a rebase, but all good 🚀

@PVince81 PVince81 force-pushed the bugfix/29941/remove-shareinfo-query-from-scanner branch from 6b081a7 to 8844b97 Compare January 10, 2022 09:00
Remove scanAll which relies on the "shareinfo" endpoint that returns the
full cache tree.
The latter can become big for big shares and result in timeouts.
Furthermode, the full tree would be retrieved again for each and every
detected change which can become expensive quickly.

Signed-off-by: Vincent Petry <[email protected]>
@PVince81 PVince81 force-pushed the bugfix/29941/remove-shareinfo-query-from-scanner branch from 8844b97 to ce31914 Compare January 10, 2022 10:33
@PVince81
Copy link
Member Author

JS failure unrelated, merging

@PVince81 PVince81 merged commit db0414a into master Jan 10, 2022
@PVince81 PVince81 deleted the bugfix/29941/remove-shareinfo-query-from-scanner branch January 10, 2022 15:42
@solracsf
Copy link
Member

Does this qualify for backport?

@PVince81
Copy link
Member Author

/backport to stable23

@PVince81
Copy link
Member Author

/backport to stable22

@PVince81
Copy link
Member Author

/backport to stable21

@PVince81
Copy link
Member Author

Does this qualify for backport?

thanks for your usual reminders 😄

@juliusknorr
Copy link
Member

@PVince81 FYI it seems there is a regression where uploading files or creating folders is not possible anymore with this on the receiving side: #31554 Bisecting showed this commit and reverting makes it work again.

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

Labels

3. to review Waiting for reviews bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Federation timeout to low to getShareInfo() at big Federation link

7 participants