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

Revert "Revert "Remove inefficient fed share scanner"" #32852

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Jun 13, 2022

This reverts commit 6667007.

See context in #32806, this is breaking things

Fixes #31554

@CarlSchwan CarlSchwan added the 1. to develop Accepted and waiting to be taken care of label Jun 13, 2022
@CarlSchwan CarlSchwan requested a review from PVince81 June 13, 2022 08:15
@PVince81 PVince81 marked this pull request as draft June 13, 2022 10:06
@CarlSchwan
Copy link
Member Author

@PVince81 could you test with the latest changes? (disabling locking see commit 4f52670)

@CarlSchwan CarlSchwan marked this pull request as ready for review June 17, 2022 10:10
@CarlSchwan CarlSchwan requested a review from icewind1991 June 17, 2022 11:25
@CarlSchwan
Copy link
Member Author

@icewind1991 what do you think of disabling the locking as the original federated shares also didn't have locking and this is causing some issues?

@CarlSchwan CarlSchwan requested a review from juliusknorr July 12, 2022 09:03
@juliusknorr
Copy link
Member

Having an integration test for the failing case from #31554 would be good I think.

There is one that looks like it could catch that but the file upload part is commented out f6a1286#diff-3d1fc871fd7bb42743321aa17f6abae00336d14755e5992ee3d5c34890c685eeR243-R245

CarlSchwan and others added 3 commits August 3, 2022 15:17
The old inneficiant code didn't do locking and adding locking is
creating issues

Signed-off-by: Carl Schwan <[email protected]>
@icewind1991 icewind1991 force-pushed the revert-revert-federation-performance-issues branch from 4f52670 to 8367b99 Compare August 3, 2022 15:12
@icewind1991
Copy link
Member

The external scanner wasn't respecting the updater's request to disable locking (as the updater is triggered while we already have an exclusive lock) fix is added to the PR.

With this I can upload to fed. shared without issue

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 makes a lot of sense

@PVince81 PVince81 requested review from artonge and come-nc August 4, 2022 07:12
@PVince81
Copy link
Member

PVince81 commented Aug 4, 2022

/backport to stable24

@PVince81
Copy link
Member

PVince81 commented Aug 4, 2022

/backport to stable23

@PVince81 PVince81 added this to the Nextcloud 25 milestone Aug 4, 2022
@PVince81 PVince81 merged commit f421c7a into master Aug 4, 2022
@PVince81 PVince81 deleted the revert-revert-federation-performance-issues branch August 4, 2022 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of performance 🚀
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Federated Share LockedException
5 participants