Fix PostgreSQL load when linking in announcements

[ClearlyClaire]

Fixes #13245 by caching status lookups

Since statuses are supposed to be known already and we only
need their URLs and a few other things, caching them should
be fine.

Since it's only used by announcements so far, there won't
be much statuses to cache.

[abcang]

I think it is very dangerous to search by url.
Running EXPLAIN on my personal instance took about 8 seconds, but I'm guessing that it will be even slower and take several minutes on large instances like mastodon.social and Pawoo.
Currently, there is no index, so it is very slow because it is searching by sequential scan.
Also, this slow query will be executed too if the announcement contains a URL other than the Mastodon instance (for example, YouTube).

EXPLAIN ANALYZE SELECT "statuses".* FROM "statuses" WHERE "statuses"."deleted_at" IS NULL AND "statuses"."url" = 'https://testingmstdn.abcang.net/' ORDER BY "statuses"."id" DESC LIMIT 1

Limit  (cost=444085.66..444085.67 rows=1 width=325) (actual time=9161.576..9161.576 rows=0 loops=1)
  ->  Sort  (cost=444085.66..444085.67 rows=1 width=325) (actual time=9161.575..9161.575 rows=0 loops=1)
        Sort Key: id DESC
        Sort Method: quicksort  Memory: 25kB
        ->  Seq Scan on statuses  (cost=0.00..444085.65 rows=1 width=325) (actual time=9161.567..9161.567 rows=0 loops=1)
              Filter: ((deleted_at IS NULL) AND ((url)::text = 'https://testingmstdn.abcang.net/'::text))
              Rows Removed by Filter: 7817774
Planning time: 0.214 ms
Execution time: 9161.636 ms

Due to the slowness of the query, it is expected that the announcement API will be run by many users before the query results are recorded in the cache, resulting in many slow queries.

[abcang]

I think it is better to stop searching by url. If you stop searching by url, it will not be displayed seamlessly for the URL of other instances, but I did not think it was important.

[ClearlyClaire]

Hm… I don't know how likely it is that API calls are going to be made while the query is going to run: individual users should only call the API on fetch, not receiving from the streaming API, which is the first thing that happens.

Anyway, yeah, it may not be worth it to perform that query for remote toots…

On the other hand, it's not the only place we perform such queries (e.g., the search box, this is required to find private toots you have the URL to). So maybe we do need an additional index…

[abcang]

Indeed, we may not need to think too much about running the announcement API by many user before query results are cached…
I think it's important to fix the search so that it takes less than a few minute.

On the other hand, it's not the only place we perform such queries (e.g., the search box, this is required to find private toots you have the URL to).

I noticed this issue while investigating the announcement API. The search API Issue has already been created. #13251

[nightpool]

I guess I'm just confused why we need to do this query at all. can't we do it once when updating the status and store the association in a jsonb column?

…

On Thu, Mar 12, 2020, 10:38 AM abcang ***@***.***> wrote: Indeed, we may not need to think too much about running the announcement API by many user before query results are cached… I think it's important to fix the search so that it takes less than a few minute. On the other hand, it's not the only place we perform such queries (e.g., the search box, this is required to find private toots you have the URL to). I noticed this issue while investigating the announcement API. The search API Issue has already been created. #13251 <#13251> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#13250 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABZCV2W6AMVTFDA7DCTKULRHDXVXANCNFSM4LFYPSOA> .

[ClearlyClaire]

For announcements that could definitely work, yeah. It wouldn't for URL search though.

[ClearlyClaire]

Pushed a commit to perform the lookup when saving the announcement, making sure this is not done more often than needed.
However, this means the action of posting an announcement can timeout, now… and I'm not comfortable with that…
I think the following further changes can be made:

• move the status resolution from a before_validation hook to the publishing worker (and make all announcements unpublished until the worker is actually called to avoid clients pulling in incomplete announcement objects)
• fetch toots instead of looking them up in the database… those are public toots so that is possible, and also, faster in general

[abcang]

I agree with how to fetch toot. Even if a worker searches an external URL in the background, I think that it will be a slow query and the load on the DB will not change. (I think a large instance will take at least a few minutes to query for an external URL.)

[ClearlyClaire]

ok changed the PR so that:

• lookups are not made when serializing, but when publishing (in the publishing worker)
• lookups for remote URLs are done by fetching the toot (which cost is only dependent on the remote server, and limited by our timeouts) rather than from the database (which cost is dependent on the number of toots known to the instance)
• remote lookups are cached so that URLs don't get unnecessarily re-fetched when updating an announcement

[Gargron]

I am a bit concerned about the inconsistency that we have an explicit field for status_ids but not for account_ids or tag_ids, which itself is not exactly consistent with statuses having mentions and statuses_tags tables for storing such relations.

[ClearlyClaire]

Yeah, I'm a bit unsure about this one, but I don't think we need more than an array here. Also, accounts and tags are significantly faster to fetch since we have an index on that, but not for URLs. There is kind of a precedent with reports and conversations holding arrays of toots/accounts too.

[abcang]

@Gargron Is this not included in v3.1.3?

[Gargron]

Oops, I thought I merged. Thank you!