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

Allow better query plans #4424

Merged
merged 20 commits into from Feb 12, 2024
Merged

Conversation

dullbananas
Copy link
Collaborator

@dullbananas dullbananas commented Feb 4, 2024

  • Change from_collapse_limit and join_collapse_limit from 8 to 11 (later I will make more queries not exceed the limit of 11)
  • Change geqo_threshold back to default if the server has a custom value for it, which is important if the custom value is not greater than 11
  • Limit connection age to 3 days to prevent use of query plans that use old statistics

@dullbananas
Copy link
Collaborator Author

I tested it with 0 instead of 3 days and the pool still works

@dullbananas dullbananas marked this pull request as ready for review February 6, 2024 22:43
@Nutomic
Copy link
Member

Nutomic commented Feb 7, 2024

Did you get any performance improvements with these changes?

.batch_execute("SET geqo_threshold=12;SET fro_collapse_limit=11;SET join_collapse_limit=11;")
.await
.map_err(ConnectionError::CouldntSetupConfiguration)?;
Ok(conn)
Copy link
Member

Choose a reason for hiding this comment

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

Please add some comments what these settings do and why you picked those values. I also think this could cause problems if an admin tries to optimize their db, but changes get silently overwritten by Lemmy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also think this could cause problems if an admin tries to optimize their db, but changes get silently overwritten by Lemmy.

I will eventually make queries not exceed the new collapse limits. Higher values set by admins will make no difference in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added comment

@dullbananas dullbananas marked this pull request as draft February 10, 2024 20:41
@dullbananas dullbananas marked this pull request as ready for review February 10, 2024 20:56
@dullbananas
Copy link
Collaborator Author

dullbananas commented Feb 10, 2024

Did you get any performance improvements with these changes?

There's no measurable changes yet because db_perf currently doesn't run any query that's affected by the changed collapse limits.

One potential performance improvement is in PostReportView, which joins 9 or 10 tables. Currently, it needs to join the community_moderator table last, which is very bad for performance. Now, it no longer exceeds the collapse limits, which makes it possible for the query planner to change the join order.

I don't expect the connection age limit to improve performance in most cases, but it will be very helpful in some cases where the number of rows in a table increases very quickly from a small amount (which could cause prepared statements to use sequential scans) to a much bigger amount. This reduces the chance of needing to restart the server.

@Nutomic Nutomic merged commit 677d54a into LemmyNet:main Feb 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants