Skip to content

[main] Fixing active counts slow queries. (#5907) #5917

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

Merged
merged 5 commits into from
Aug 9, 2025

Conversation

dessalines
Copy link
Member

Cherry-picked from the other branch, with a few corrections (namely the _aggregates tables are gone now)

dessalines and others added 2 commits August 2, 2025 16:33
* Fixing active counts slow queries.

* Simplify back to str tuple

* Batch site and community updates

* Using update from temp table

* Making aggs temp table use interval name.

* Make dev setup use optimized postgres.

* Addressing PR comments.

* Use ref

* Removing system custom info from customPostgresql.conf
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't be run by an instance that already ran the post_like/comment_like variant of this migration. So the indexes would not be created for the new combined actions tables on that instance. You probably need to do all of these:

  • keep this migration unchanged
  • change the dates of the smoosh-tables-together migration and of other migrations that need to come after smoosh-tables-together
  • add these indexes to the smoosh-tables-together migration if they aren't there already

Also there should be WHERE liked_at IS NOT NULL on these indexes. Parenthesis might be needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. I'll diff the migrations folders between main and release/v0.19, and make sure nothing is missing, and that all the dates in main all come after the v0.19 migrations

@dessalines dessalines marked this pull request as draft August 4, 2025 13:41
Comment on lines +729 to +731
active_counts(&mut context.pool(), ONE_DAY).await?;
all_active_counts(&mut context.pool(), ALL_ACTIVE_INTERVALS).await?;
update_local_user_count(&mut context.pool()).await?;
Copy link
Member Author

Choose a reason for hiding this comment

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

Added tests for all these new ones here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. I'll diff the migrations folders between main and release/v0.19, and make sure nothing is missing, and that all the dates in main all come after the v0.19 migrations

@dessalines
Copy link
Member Author

I've now renamed all the 1.0 migrations to make sure they come after our 2 unfortunate release/v0.19 migrations. Also copied these ones over to main. You can verify doing:

git diff --diff-filter D --stat release/v0.19 migrations

In the future, lets backdate any migrations necessary on release/v0.19, because this was incredibly tedious.

@dessalines dessalines marked this pull request as ready for review August 6, 2025 18:27
Copy link
Member Author

Choose a reason for hiding this comment

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

This migration was in release/v0.19 only, and it has been run on production servers. Copied it over.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same with this one. Every 1.0 migration is after this now.

@Nutomic Nutomic merged commit 644a448 into main Aug 9, 2025
2 checks 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.

3 participants