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

In Terms Indexablequery_db, for term count, use wp_count_terms() instead of WP_Term_Query #3791

Merged
merged 2 commits into from
Jan 4, 2024

Conversation

rebeccahum
Copy link
Contributor

@rebeccahum rebeccahum commented Dec 13, 2023

Description of the Change

We're seeing on sites with a very large amount of terms, terms indexing isn't starting because of a query that is being killed:

SELECT t.*, tt.* FROM wp_terms AS t INNER JOIN wp_term_taxonomy AS tt ON t.term_id = tt.term_id WHERE t.term_id IN (.....,......,......,......,......,......,......,......,......,......,......,......,......,......,......,......,......,......,......)

This is because in query_db() for the terms Indexable, we are calling a new instance of WP_Term_Query, which ultimately calls _prime_term_caches and triggers an expensive, unnecessary query: https://github.com/WordPress/WordPress/blob/e99a11600602fbb5282b59227004d61c543d812d/wp-includes/taxonomy.php#L4071

It isn't required for the ultimate term count in $total_objects, so we should be good to replace it with a simple wp_count_terms() call (which doesn't ultimately call _prime_term_caches() due to an early return).

Aside: I've also set hierarchical to false because I'm seeing that it performs a no limit query which, on a site with a lot of terms, is being killed: https://github.com/WordPress/WordPress/blob/e99a11600602fbb5282b59227004d61c543d812d/wp-includes/class-wp-term-query.php#L635-L644. I don't think it's the hierarchical structure is necessary for indexing (correct me if I'm wrong).

How to test the Change

  1. Attempt to index a terms indexable and validate that the terms count is the same in wp vip-search index --indexables=term

Changelog Entry

Changed - counts are now calculated with wp_count_terms() in Indexable Terms query_db

Credits

Props @rebeccahum

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@rebeccahum rebeccahum marked this pull request as ready for review December 13, 2023 06:44
@rebeccahum rebeccahum force-pushed the terms/fix-query-db branch 4 times, most recently from 97737a3 to 60a1ca3 Compare December 13, 2023 07:55
…instead of `WP_Term_Query` to avoid `_prime_term_caches`

This is because on a DB with a lot of terms, calling `prime_term_caches` triggers an expensive, unnecessary query that isn't required for the ultimate term count. Also, add `hierarchical` => `false` for performance, otherwise it performs a no limit query
@felipeelia felipeelia self-assigned this Dec 19, 2023
@felipeelia felipeelia added this to the 5.1.0 milestone Dec 19, 2023
@felipeelia felipeelia merged commit 48b7fa9 into 10up:develop Jan 4, 2024
8 of 14 checks passed
@rebeccahum rebeccahum deleted the terms/fix-query-db branch January 4, 2024 16:50
rebeccahum added a commit to Automattic/ElasticPress that referenced this pull request Jan 4, 2024
rebeccahum added a commit to Automattic/ElasticPress that referenced this pull request Jan 4, 2024
@felipeelia felipeelia modified the milestones: 5.1.0, 5.0.2 Jan 11, 2024
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.

2 participants