Automattic / Automattic/lightweight-term-count-update

Improve performance for (child) attachments

Open
#5 2 comments 0 reactions 1 assignee Claimed by @trepmal View on GitHub
Dominant language
PHP
Stars
34
Forks
9
PR merge metrics
No merged PRs in 30d

Description

From #2 ([direct link to PR comment](https://github.com/Automattic/lightweight-term-count-update/pull/2#discussion_r108429086)):

> > This returns only latest 10 attachments. I'm not sure whether setting the `posts_per_page` should be set to `-1` or whether we should come up with some clever direct SQL. Thoughts?
>
> I was wondering that too. I'm not sure what to do here, to be honest, but I do have some suggestions. To maintain parity with core, we would need to set `'posts_per_page' => -1` to catch all possible attachments, but that could theoretically be worse than `_update_post_term_count()`. Here are a few ideas:
>
> 1. We could query just for `'fields' => 'ids'`. All we ever use of the post objects are the `ID` and `post_type`, and we already know the post type is `attachment`. This would keep the query light, but it still means that every attachment could theoretically trigger another query to update the term count, so if a post has 100k attachments we could theoretically be looking at 100,002 total sql queries.
> * A sub-option here would be to refactor the code to be able to pool the attachment queries into one query. That would turn the 100,002 queries down to 1 or 2 queries, where one of them would have 100k+ `tt_id`s in the `IN()` clause.
> 2. We could do a quick count synchronously for the post and then setup an asynchronous cron task to count attachments, if there are any (or if there are more than, say, 100).
> 3. Count the attachments, and if the number is high, just let `_update_post_term_count()` run as it usually would. Additionally, we could add a filter to optionally override it, and then people can intervene with custom code if it's a real problem for them, and do something else like (1) or (2) or offload the query to Elasticsearch or something.
>
> (1) or (1a) seem like the best options to me of what I've considered. (2) feels over-engineered for the 0.001%, or whatever hilariously low percentage it would be, of use cases that could be problematic. (3) is a fine option too -- we fixed all the use cases most people will ever encounter, and for that one in a million, we simply left it as-is but opened a door for that site to easily solve their problem.

For the first release of the plugin, we went with option (1). This does have potential to be slow, as noted above, so other options should still be considered.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.