perf(gatsby-source-contentful): prevent redundant fs/remote fetches for base64

[pvdz]

Currently the base64 fetch will prefer to read from local cache if the url is known, or do a remote fetch if the url is unknown. It does not do an in-flight check which means the same url could be scheduled and fetched multiple times, as long as one has not completed. This applies to remote fetch and local cache fetches.

This PR introduces two caches (separate commits):

• an in flight cache containing any request that has not resolved
• a resolved cache, containing the payload of resolved requests

The function will now prefer to return the data sync, from the resolved cache. If the url is not known there then it will check the in flight cache. This cache may contain the promise if it is still in flight, or if the request rejected (it will not retry). If that cache doesn't have it then the disk cache is checked. If the disk cache has it then read it and return it. Otherwise do a remote fetch and return it. In both fetch cases, the read promise is stored in the in flight cache and when it resolves, it is removed from the in flight cache and its payload added to the resolved cache.

In the real world I stumbled on a customer where the site was triggering 245.000 base64 image requests (these are only debounced by the nodejs default limits and, to some degree, by the query concurrency count). It turned out that there were actually only 5.000 unique urls, so 240.000 requests were redundant.

While testing that same site we encountered curious inconsistent delays that tracked back to this function.

[vladar]

Should we clear the promise cache at some point?

Without it, long-running processes like previews/inc.builds will never re-request images even if they change?

[KyleAMathews]

We could revalidate them by storing the etag. This is what createRemoteFileNode does.

[pvdz]

In short, I think we don't need to worry about this

• The images are relatively small (these are forcibly requested with w=20 arg).
  • Sure, call me out in a year when that changes without considering this cache.
• There are many images. Revalidating them all will be quite expensive, even just for an etag check.
  • Furthermore, this is a CDN, I don't think the image ever mutate on the remote because it will get a different hash (url)
• In a single running process (whether that is preview, develop, or incremental build), the number of images that are deleted are not likely to be great on the regular. It is possible but I feel in that case, the overhead is acceptable.

Is my analysis accurate?

[pvdz]

Oh, important point as well; this is not a generic solution. This is specifically for Contentful and their CDN. So we don't need to worry about the "what if"s on that side. We can verify this with them if we doubt it.

[KyleAMathews]

Sure, call me out in a year when that changes without considering this cache.

🤣

[KyleAMathews]

I don't think the image ever mutate on the remote because it will get a different hash (url)

Yeah this is (probably?) true so we don't need to worry about invalidation. You could verify this trivially by trying to modify a photo on a node in Contentful and seeing if the ID is the same.

[pvdz]

(Verified this with Contentful)

[pieh]

Published in gatsby-source-contentful@4.2.1