-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Avoid WHERE IN
for comment migration query
#28500
Conversation
- Rewrite `UpdateCommentsMigrationsByType` to not use `WHERE IN` as that's a performance diaster for MariaDB, it now use batching to query the the relevant comment IDs via JOINs (which is not possible in a UPDATE query for SQLite) and then update them in a seperate query. - Resolves https://codeberg.org/forgejo/forgejo/issues/1856
"original_author_id": 0, | ||
}) | ||
return err | ||
const batchSize = 50 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can first interate all the special repositories since they will not be expected too much.
Instead od in would a join be more efective? |
} | ||
|
||
for _, commentID := range commentIDs { | ||
if _, err := db.GetEngine(ctx).Table(&Comment{}).ID(commentID).Update(map[string]any{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woohoo.
This sounds like such an improvement.
Instead of one long query we now open one million db connections 😒.
SQLite, please fix your weird non-SQL conform shenanigans.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per #28500 (comment), I think we can reduce the requests times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how that reduces the requests.
Replace #28500 --------- Co-authored-by: Giteabot <[email protected]>
Replace go-gitea#28500 --------- Co-authored-by: Giteabot <[email protected]>
Backport #28547 by @lunny Replace #28500 Co-authored-by: Lunny Xiao <[email protected]>
Replace go-gitea#28500 --------- Co-authored-by: Giteabot <[email protected]>
Replace go-gitea#28500 --------- Co-authored-by: Giteabot <[email protected]>
Replace go-gitea#28500 --------- Co-authored-by: Giteabot <[email protected]>
UpdateCommentsMigrationsByType
to not useWHERE IN
as that's a performance diaster for MariaDB, it now use batching to query the the relevant comment IDs via JOINs (which is not possible in a UPDATE query for SQLite) and then update them in a seperate query.