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

XWIKI-9046: Renaming a document holding a lot of revisions can lead to an OutOfMemory exception #2926

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

pjeanjean
Copy link
Contributor

This requires merging #2925 first.

Jira URL

https://jira.xwiki.org/browse/XWIKI-9046

Changes

Description

  • Only load the last node when updating the document archive instead of the whole history.

Clarifications

N/A

Screenshots & Video

N/A

Executed Tests

Renaming a document with around 200k revisions was tested on an instance running this patch and it worked fine. Also, the saving process only took around 150ms instead of 1s without the patch applied (with HSQLDB).

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • stable-15.10.x

@surli surli added the backport stable-15.10.x Used for automatic backport to 15.10.x branch. label Feb 26, 2024
@pjeanjean pjeanjean marked this pull request as draft February 26, 2024 13:38
…o an OutOfMemory exception

* Only load last node when updating the document archive
@tmortagne
Copy link
Member

Renaming a document with around 200k revisions

Might be interesting to have such an integration test, a lot of pieces are involved here, and it would be easy to break this use case again.

@pjeanjean
Copy link
Contributor Author

That's reasonable, I'll see what I can do.

…o an OutOfMemory exception

* Add a basic test
@pjeanjean
Copy link
Contributor Author

I added a very basic test for now.

@tmortagne tmortagne merged commit 8e509b7 into xwiki:master Mar 21, 2024
1 check passed
github-actions bot pushed a commit that referenced this pull request Mar 21, 2024
…o an OutOfMemory exception (#2926)

* Only load last node when updating the document archive
* Add a basic test

(cherry picked from commit 8e509b7)
surli pushed a commit that referenced this pull request Mar 21, 2024
…o an OutOfMemory exception (#2926)

* Only load last node when updating the document archive
* Add a basic test

(cherry picked from commit 8e509b7)
surli added a commit that referenced this pull request May 24, 2024
…nts is moved

Revert "XWIKI-9046: Renaming a document holding a lot of revisions can lead to an OutOfMemory exception (#2926)"

This reverts commit 8e509b7.

The solution provided in this commit wasn't taking into account usecases
when the doc wasn't saved in DB yet, e.g. like in case of renaming. Also
the solution is not good since we actually never set the archive in doc
in case of criteria not all inclusive, and if it had worked it would
have been a problem since we'd have kept only latest revision and lost
the remaining of the history when saving docs. All in all we need
another solution.
surli added a commit that referenced this pull request May 24, 2024
…nts is moved

Revert "XWIKI-9046: Renaming a document holding a lot of revisions can lead to an OutOfMemory exception (#2926)"

This reverts commit 8e509b7.

The solution provided in this commit wasn't taking into account usecases
when the doc wasn't saved in DB yet, e.g. like in case of renaming. Also
the solution is not good since we actually never set the archive in doc
in case of criteria not all inclusive, and if it had worked it would
have been a problem since we'd have kept only latest revision and lost
the remaining of the history when saving docs. All in all we need
another solution.

(cherry picked from commit 03d3ebe)
surli added a commit that referenced this pull request May 24, 2024
…nts is moved

Revert "XWIKI-9046: Renaming a document holding a lot of revisions can lead to an OutOfMemory exception (#2926)"

This reverts commit 8e509b7.

The solution provided in this commit wasn't taking into account usecases
when the doc wasn't saved in DB yet, e.g. like in case of renaming. Also
the solution is not good since we actually never set the archive in doc
in case of criteria not all inclusive, and if it had worked it would
have been a problem since we'd have kept only latest revision and lost
the remaining of the history when saving docs. All in all we need
another solution.

(cherry picked from commit 03d3ebe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport stable-15.10.x Used for automatic backport to 15.10.x branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants