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

perf: Use generator/count query on cleanup tasks #5917

Closed
wants to merge 2 commits into from

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Jun 17, 2024

Fix #5904

  • Avoid running out of memory when iterating over documents
  • Think about a way to do the document cleanup later on as iterating over all documents and deleting the y.js state files is not feasible on larger instances
    • Implemented this now for testing, in general this approach should work fine as we do set this value also for new file creation if no steps were pushed.
    • Would still need some testing and thinking if the reset on create could again cause problems

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr force-pushed the fix/long-running-migration branch 2 times, most recently from ae07fed to e30d8bd Compare June 17, 2024 08:20
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr force-pushed the fix/long-running-migration branch from e30d8bd to 14a7d21 Compare June 17, 2024 08:26
@juliusknorr juliusknorr added bug Something isn't working 2. developing high labels Jun 17, 2024
Comment on lines +103 to +105
if ($document && $document->getLastSavedVersionTime() < $resetAfterUpdateTimestamp) {
$this->documentService->resetDocument($document->getId());
}
Copy link
Collaborator

@max-nextcloud max-nextcloud Jun 17, 2024

Choose a reason for hiding this comment

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

Will the resetDocument bump the value of getLastSavedVersionTime? Otherwise this will be run for every session created until the next (auto-)save.
In particular for folder descriptions this can take a loong time if they are never changed and people randomly create sessions when visiting the folder.
What about read only sessiions? Can they reset the document? Will that update the last saved version time?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah... i see... resetDocument will delete the entry from the documents table. So $document will be empty for the next request coming in.

One thing that might still get in the way would be locks - resetDocument tries to unlock the file. I don't know what would happen if the file is not locked by text.

@mejo-
Copy link
Member

mejo- commented Jun 17, 2024

Replaced by #5918

@mejo- mejo- closed this Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing bug Something isn't working high
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Session cleanup
3 participants