[Miniflare 3] Implement new storage system #555
Conversation
|
|
The CI failure is the same as #551, and should be fixed once that's merged. |
Implements file-system backed and in-memory `ReadableStream`s supporting single and multiple ranges, and a file-system backed `WritableStream`. Closes DEVX-583 and DEVX-584
| // Validate cacheTtl, but ignore it as there's only one "edge location": | ||
| // the user's computer |
There was a problem hiding this comment.
I wonder if at some point we might want to add some random jitter to KV writes, to simulate the real consistency guarantees?
There was a problem hiding this comment.
I've so far avoided adding random failures to Miniflare, because I wanted tests using it to be as deterministic as possible, but it would definitely be interesting to explore. Could try simulate things like transient network failures too.
There was a problem hiding this comment.
To keep it deterministic, the API could perhaps include a way to set the next operation (specific or any) to fail the next one or more times?
| delete(id: BlobId): Promise<void>; | ||
| } | ||
|
|
||
| function generateBlobId(): BlobId { |
There was a problem hiding this comment.
What about just using a UUIDv4? Or a ULID if the time component is important?
There was a problem hiding this comment.
Could do, just wanted something practically guaranteed unique, without adding another dependency.
There was a problem hiding this comment.
Not remotely blocking for this PR, but I think crypto.randomUUID() should be a dependency-free way to get a UUID
There was a problem hiding this comment.
This is looking really really good—I'm very excited to see this in Miniflare! Two remaining questions, and then this is good to go from my perspective:
- It would be great to see some tests for this system (as far as I can tell only the blob storage system is tested right now)
- What's the migration story here? What happens when someone runs this new storage system in a project that previously had the old one? Can we add some messaging around that?
We have tests for the blob stores and the streams. We don't have tests for the SQL stuff since it's more or less
Miniflare should ignore the old data, unless there are keys like Unfortunately, there's no easy way for us to detect the old system. We could check if the directory is non-empty and doesn't contain |
I was specifically thinking about
This seems like a reasonable tradeoff—lets make sure we add this messaging when upgrading to this version of Miniflare in Wrangler |
Closes DEVX-585 and DEVX-586
Closes DEVX-587
KV and cache have similar storage requirements: they both need an expiring-key-value-metadata store. To simplify their implementation, implement an abstract store on top of the new storage system to provide this functionality. Closes DEVX-588
Whilst we are migrating gateways over to the new storage system, we'd like the `GatewayConstructor` type to use the old `Storage` type to keep TypeScript happy. This change adds a `getNewStorage()` method that gateways implemented using new storage should call to get an appropriate storage implementation.
Closes DEVX-589
Our new storage system is incompatible with how Workers Sites used to work. This change implements `sitesGatewayGet` and `sitesGatewayList` functions that behave like unsanitised `FileStorage` used to. We serve directly from disk with Workers Sites to ensure we always send the most up-to-date files. Otherwise, we'd have to copy files from disk to our own SQLite/blob store whenever any of them changed.
This PR implements a new underlying storage system for Miniflare, based off #525 and internal discussions. The storage system is split into 2 parts: an SQLite-backed metadata store, and a streaming file-system backed/in-memory blob store. Blobs use unguessable identifiers to support atomic transactions with the SQLite metadata store. See
BlobStorefor more details. This is a breaking change to the persistence format.This PR also re-implements the KV gateway using the new storage system, as an example of what a data store consuming this system might look like. Notably, this implementation uses an intermediate expiring key-value-metadata store, that will be shared by the Cache gateway in the future.
Best reviewed commit-by-commit.
Closes DEVX-583, DEVX-584, DEVX-585, DEVX-586, DEVX-587, DEVX-588 and DEVX-589.