feat: chunk data store#17296
Conversation
🦋 Changeset detectedLatest commit: ca9a758 The changes in this PR will be included in the next version bump. This PR includes changesets to release 393 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
yanthomasdev
left a comment
There was a problem hiding this comment.
A couple bikeshedding questions from me.
| 'astro': minor | ||
| --- | ||
|
|
||
| Adds a new experimental `dataStore` option for controlling how the content layer persists its data store |
There was a problem hiding this comment.
dataStore feels a bit too broad, I wonder if that might feel misleading and indicate this also controls other data like images or assets? Since this is only content collections related, I am wondering if maybe this feature would be better named something like "Collections storage" (there's probably a better name than this suggestion)
There was a problem hiding this comment.
What about collectionStorageKind or just collectionStorage? I am not attached to the name, so if there's a better name, it's more than welcome
There was a problem hiding this comment.
I chose collectionStorage. Happy to change again if we find a better name
ArmandPhilippot
left a comment
There was a problem hiding this comment.
A few file that haven't been updated to single-file, otherwise LGTM docs-wise. And, I don't have a better name to suggest so I think it's fine. Thank you both!
Co-authored-by: Armand Philippot <git@armand.philippot.eu>
| for (const [collectionName, entries] of sortCollections(collections)) { | ||
| const chunks: string[][] = []; | ||
| // Bound the size of any single serialized string. | ||
| for (const chunkedEntries of chunkMap(entries, CHUNK_ENTRY_LIMIT)) { |
There was a problem hiding this comment.
The problem I see is that it splits on number of entries and then a size. That means that you could have one single entry that exceeds the size limit, which means you exceed the platform limits anyways.
There was a problem hiding this comment.
Done, I removed the number of entries as threshold ca9a758 (this PR)
|
I don't think it should split based on number of entries and file size. I think it should only be on file size. In addition to the reason given in the inline comment, having many very small entries doesn't have any downsides I can think of. Doing it purely based on file size is both simpler for implementation and simpler for the user to reason about. I also would expect the file size to be configurable, as different platforms might have different limits. |
Changes
This PR adds a new experimental option called
dataStore. It allows to split the data into multiple chunks.I also did some refactor, so that we can prepare different data sources e.g. sqlite. For this reason, the functions of the interface are also async.
Chunks are created when:
Testing
Added various tests for:
Docs
withastro/docs#14210
/cc @withastro/maintainers-docs for feedback!