Skip to content

codeintel: Non-leaky reader cache

Warren Gifford requested to merge pci-reader-cache into master

Created by: efritz

See https://github.com/sourcegraph/sourcegraph/pull/11295 for a previous attempt.

We can no longer use ristretto for a connection cache as the API does not give enough information for when values are dropped under certain conditions. For example:

  • look up connection by filename, cache miss
  • create a new connection and add to cache
  • ristretto queues it onto the put buffer, Set returns true (we have to assume we're all good)
  • ristretto worker dequeues it from the put buffer
  • decides to drop value and does not all evict function -> connection leaks

That decision may be due to an eviction policy or a cost size. Additionally, the old way we were creating connection values was super racy and would try to open the reader multiple times if concurrent requests wanted answers for the same bundle. Because we run migrations on load, we can't hold a lock otherwise we'll have high contention.

This PR introduces a reader "cache" that has extremely short-lived handles on SQLite databases. These values are immediately evicted once their refcount goes to zero. This cache really only cuts the initialization cost when a migration needs to be run for a bundle that has a spike of concurrent requests.

Merge request reports