Skip to content

Delete log table data in chunks to prevent possible performance issues #20424

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Mar 10, 2023
Merged

Delete log table data in chunks to prevent possible performance issues #20424

merged 7 commits into from
Mar 10, 2023

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Mar 6, 2023

Description:

refs #20221

This won't fully solve the issue but it will fix it for the tables that cause the biggest issues.

After this is merged, we could maybe deprioritise #20221 as it potentially can't even be fixed maybe and it'll be less of a problem then.

Review

@tsteur tsteur added the Needs Review PRs that need a code review label Mar 6, 2023
@tsteur tsteur added this to the 4.13.4 milestone Mar 6, 2023
sgiehl
sgiehl requested changes Mar 7, 2023
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Actually it looks like some tests are failing that are related to the changes of this PR. Seems the SQL syntax generated by deleteAllRows is invalid for some reason.

@sgiehl sgiehl removed the Needs Review PRs that need a code review label Mar 7, 2023
@tsteur
Copy link
Member Author

tsteur commented Mar 8, 2023

@sgiehl tests are now passing. That MySQL version had an issue with DELETE FROM log_visit log_visit where logvisit... limit 1. It didn't like the table alias when there is also a limit defined.

While debugging I noticed that only the log_visit table will actually use this logic. All other tables, even though most of them have an idsite table, would still use a join and therefore won't delete the data in chunks. If we merge this PR, then I would update the original issue to clarify that we need to further optimise this.

sgiehl
sgiehl approved these changes Mar 9, 2023
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Looks good to me now. @tsteur feel free to merge if that still improves what it was intended to.

@tsteur tsteur merged commit 41fe5da into 4.x-dev Mar 10, 2023
@tsteur tsteur deleted the m20221 branch March 10, 2023 05:06
@tsteur tsteur mentioned this pull request Mar 10, 2023
Open
@bx80 bx80 mentioned this pull request Apr 30, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
4.14.0
Development

Successfully merging this pull request may close these issues.
2 participants
@tsteur @sgiehl