feat: sqlite cache store

[flakey5]

Note: this is a draft since #3562 isn't landed. Until it is landed, this will be based off of that pr's branch. For the actual diff see flakey5/undici@flakey5/3231...flakey5:undici:flakey5/20240910/sqlite-cache-store

This relates to...

Adding client side http caching (#3562)

Rationale

Adds a sqlite cache store option given the --experimental-sqlite flag is provided

Changes

Features

• Cache store using sqlite as a backend

Bug Fixes

n/a

Breaking Changes and Deprecations

n/a

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

cc @mcollina @ronag

[mcollina]

Can you add some docs?

[mcollina]

lgtm

[mcollina]

lgtm

[ronag]

• Unnecessarily complicated by locking.
• How does it "prune" i.e. free space?
• Why errorCallback?

[ronag]

IMHO I would think you could do this quite similar to the "simplified" memory store.

[flakey5]

Why errorCallback?

Goes back to the discussion we were having in the original caching pr for how to handle errors, we decided to handle them silently so the request still goes through.

Here it's just being used for handling json parse errors, since those shouldn't ever happen I would be in favor of just throwing outright though

How does it "prune" i.e. free space?

It prunes the entire database of expired values when we come across one when looking up a key, we can make this more stricter though (i.e. run the purge query on every write), wdyt?

[ronag]

It prunes the entire database of expired values when we come across one when looking up a key, we can make this more stricter though (i.e. run the purge query on every write), wdyt?

When the database is full (we are missing maxSize) we should remove entries whether they are expired or not. I think we should have something like, while beyond maxCount or maxSize drop 10% of the oldest entries.

[flakey5]

I omitted maxCount here since it would break if there were multiple processes using a db for cache

If we do want it, I think we might be able to get away with it being stored somewhere in the db and being modified only with a procedure.

[ronag]

If we do want it, I think we might be able to get away with it being stored somewhere in the db and being modified only with a procedure.

I believe sqlite has meta commands for that, @mcollina ?

[ronag]

Can't you just always just overwrite? Does it update and insert need to be separate ops?

[flakey5]

I think so but I'm not sure how messy it would be compared to just having two queries

[ronag]

Does update fail if the value doesn't exist? Doesn't sql have an insert or update command?

[flakey5]

Does update fail if the value doesn't exist

It fails in the sense that it noops.

Doesn't sql have an insert or update command?

I'm not sure how well insert or update would work here based on how we're checking if there's a already cached value though. The only reliable and fast way we have to compare two cached values is with their ids. Otherwise, we need to compare their url, method, and vary headers to see if they were the same. The url and method comparisons are simple and can be done in sql, but comparing the vary headers is more difficult since it's stringified json. We can't compare it in sql since we don't know if the properties are in the same order, meaning we need to parse then compare (like what #findValues does).

I was experimenting with hashing the vary headers (sort properties alphabetically -> stringify -> crypto.createHash), however, it was a lot more complicated than just having two separate queries. It also would've only benefited writes, for reads we would still need to fetch each response and compare the vary headers like we already do

[metcoder95]

do we also support arrays?

[flakey5]

Of cache keys? Currently no

[ronag]

Why not make the database primary key a hash of url + method + vary headers?

[flakey5]

Why not make the database primary key a hash of url + method + vary headers?

That would still lead to the same state that I talked about here #3657 (comment)

I was experimenting with hashing the vary headers (sort properties alphabetically -> stringify -> crypto.createHash), however, it was a lot more complicated than just having two separate queries. It also would've only benefited writes, for reads we would still need to fetch each response and compare the vary headers like we already do

[mcollina]

CI is very red :(

[mcollina]

If we do want it, I think we might be able to get away with it being stored somewhere in the db and being modified only with a procedure.

I believe sqlite has meta commands for that, @mcollina ?

I think this would do it:

PRAGMA page_count;
PRAGMA page_size;

Then do page_count * page_size.

[mcollina]

Why not make the database primary key a hash of url + method + vary headers?

@ronag I think this is possible, but I would likely investigate it after this lands. I think we would need to bring in https://www.npmjs.com/package/safe-stable-stringify to be able to have "stable" JSONs.

[ronag]

Why not make the database primary key a hash of url + method + vary headers?

@ronag I think this is possible, but I would likely investigate it after this lands. I think we would need to bring in https://www.npmjs.com/package/safe-stable-stringify to be able to have "stable" JSONs.

What's wrong with e.g:

function makeKey({ method, origin, path, vary }) {
  const hasher = crypto.createHash('sha256')
  hasher.update(origin.toLowerCase())
  hasher.update(method.toLowerCase())
  hasher.update(path ?? '')
  hasher.update(vary?.map(x => x.toLowerCase()).sort().join(',') ?? '')
  return hasher.digest('hex')
}

[mcollina]

That would be very slow unfortunately :(. Unless we really need it.

[ronag]

What about https://www.sqlite.org/lang_upsert.html?

[flakey5]

I'm not sure how we could incorporate that since the only unique key we have is the id