PERF: Avoid parsing `Post#cooked` with Nokogiri for every search. (PR #10249)

GitHub

Guarding against N+1 in search here.

What is this constant?

Isn’t this going to randomly fail if we add/remove any other queries from the request? We’ve seen this problem before with the WebHookSerializer test that checks for a specific amount of attributes and breaks all the time.

This guard here is meant to set a baseline for the number of ActiveRecord queries that search should execute. Search has suffered from multiple N+1 queries and we have not had a way to catch those. If a change results in more number of queries executed on search, the developer would then have to justify the increase.

We’ve seen this problem before with the WebHookSerializer test that checks for a specific amount of attributes and breaks all the time.

We need to intentionally keep the webhook payload small so every additional attribute added to the payload has to be justified. The test breaking is a good thing to me because it allows us to review the new attributes that are added.

I get the idea, but I’m not really convinced that these tests failing causes the developer to do anything other than change the number and resubmit. I’ve never seen anyone justify a webhook attribute and I’ve seen the test fail many times.

I agree catching N+1 queries is important, I just don’t feel this is a good solution since there will be way more false positives than actual issues caught.

The reindex here is intentional so that we update search data for changes like

https://github.com/discourse/discourse/commit/2196d0b9ae660bab7991608b37d267beb1c16916 https://github.com/discourse/discourse/commit/5c230266d35f07ebadd758f077bc1f2ab5b80b82

Can we do this instead?

      if tags.present?