LemmyNet / lemmy

🐀 A link aggregator and forum for the fediverse
https://join-lemmy.org
GNU Affero General Public License v3.0
13.25k stars 880 forks source link

Run analyze after changing post.url type (ref #4983) #5148

Closed Nutomic closed 4 days ago

Nutomic commented 1 week ago

Ive changed it to run ANALYZE post (url);, vacuum shouldnt be necessary as far as I understand. Should we reindex the column as well?

cc @phiresky @dullbananas

dessalines commented 1 week ago

I've verified that @Nutomic 's suggestion of running analyze post(url); fixes the issue, nice!

The culprit for some reason wasn't actually post/list, but the GetPost / post?id= endpoint, which was timing out. This means the crosspost query was likely timing out:

// Fetch the cross_posts
let cross_posts = if let Some(url) = &post_view.post.url {
  let mut x_posts = PostQuery {
    url_search: Some(url.inner().as_str().into()),
    local_user: local_user.as_ref(),
    ..Default::default()
  }
  .list(&local_site.site, &mut context.pool())
  .await?;

I did this by:

We should probably run analyze any time we make a column change like that, especially if its used for a join.

dessalines commented 1 week ago

@Nutomic the index type change probably isn't necessary, only running the analyze.

phiresky commented 1 week ago

nothing physical changes at all due to this migration, it just clears the statistics, so yes just analyzing the column should be enough.

it is interesting how devastating the effect of this missing statistics is in this case. I've never personally seen this effect, even on larger databases. Autovacuum also runs analyze, so this problem solves itself after some time, but I'm not sure how long that is on average:

The autovacuum daemon will automatically issue ANALYZE commands whenever the content of a table has changed sufficiently

So not sure if that starts counting as "everything changed" when stats are gone or "nothing changed". If the latter, it might take ages, otherwise analyze should happen on the first autovacuum trigger.

dessalines commented 1 week ago

Yep I've never seen anything like this before either, and I'm sure we're not the only ones to run into this issue.

I sent an email to postgres' bug tracker and linked this PR.

Nutomic commented 4 days ago

Renamed the migrations back so that we can merge it.