-
-
Notifications
You must be signed in to change notification settings - Fork 884
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
Run analyze after changing post.url type (ref #4983) #5148
Conversation
@@ -0,0 +1,7 @@ | |||
ALTER TABLE post |
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.
Renamed the folder so that ci can test the migration. Have to rename it back once it passes.
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.
Just want to note, that I manually ran changing the column back to varchar(512)
in prod, and things still stayed slow.
So even if we do this manually on the DB, we need to take a DB backup before deploying this again.
10941ab
to
93bae6a
Compare
@@ -3,3 +3,7 @@ | |||
ALTER TABLE post | |||
ALTER COLUMN url TYPE varchar(2000); | |||
|
|||
DROP INDEX idx_post_url; | |||
|
|||
CREATE INDEX idx_post_url ON post USING HASH (url); |
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.
93bae6a
to
1e1ac12
Compare
Ive changed it to run |
I've verified that @Nutomic 's suggestion of running The culprit for some reason wasn't actually // 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. |
@Nutomic the index type change probably isn't necessary, only running the analyze. |
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:
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. |
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. |
Renamed the migrations back so that we can merge it. |
* Run analyze after changing post.url type (ref #4983) * rename back --------- Co-authored-by: Dessalines <[email protected]>
No description provided.