treflehq / trefle-api

🍀 Trefle is a botanical JSON REST API for plants species, allowing you to search and query over all the registered species, and build the next gardening apps and farming robots.
https://trefle.io
GNU Affero General Public License v3.0
488 stars 51 forks source link

Address some N+1 queries in `SpeciesSerializer` #77

Closed chancancode closed 3 years ago

chancancode commented 3 years ago

From looking at the Skylight dashboard, we found some N+1/repeated queries on the Api::V1::SpeciesController#show endpoint.

Screen Shot 2020-12-17 at 3 54 55 PM

In the SpeciesSerializer, there are a few has_many associations that needs to be serialized. Some of these associations then uses serializers that themselves have other has_many associations that needs to be serialized. This resulted in the classic N+1 queries problem.

The usual way to address is to add an includes to the query so Rails knows to eager load all the nested associations with a single query. However, in this case, the query is built by Panko and there does not appear to be a way to tell Panko to do this.

As a workaround, we turned the has_many associations to a normal attribute, in which we manually build the query with the preload instruction and serialize them using ArraySerializer.

The tests appear to be broken even before this commit, so we are unable to run them against these changes. However, we visited the API endpoint locally and saw no changes to the JSON output, other than a few ordering differences.

chancancode commented 3 years ago

While working on this, we also noticed the cache for this response has a very low hit rate (less than 1%). We did confirm locally (and on the production API) that the cache can work, and when it does work it does result in significant time-savings.

So, our guess is it's probably a configuration issue where the cache size is way too small for the data set, similar to what we found on Code Triage in the past. Unfortunately we don't have access to the information to debug this further, so someone else may with access may have to look into that.

lambda2 commented 3 years ago

Thanks a lot for this ! 🙏