-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use blockly indexes #51
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left few non blocking suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wild! Great work. 🚀
nits listed inline
src/lib/car-index.js
Outdated
async get (cid) { | ||
const key = cidToKey(cid) | ||
let indexItem = this.#cache.get(key) | ||
if (indexItem) { | ||
if (cid.code !== raw.code) { | ||
await this.#readIndex(cid) | ||
} | ||
} else { | ||
await this.#readIndex(cid) | ||
indexItem = this.#cache.get(key) | ||
if (!indexItem) return | ||
} | ||
return { cid, ...indexItem } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs some comments to explain why it is like this... it hard to guess why we'd want to readIndex for a cid when we just got it from the cache... you explained that this is pre-warming the cache for the next level, and alternative patterns are less efficient, but it's not possible to know that from the code as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comments, hopefully just the right amount 😀
import { base58btc } from 'multiformats/bases/base58' | ||
import { UniversalReader } from 'cardex/universal' | ||
import { MultiIndexReader } from 'cardex/multi-index' | ||
import defer from 'p-defer' | ||
|
||
/** | ||
* @typedef {import('multiformats').UnknownLink} UnknownLink | ||
* @typedef {import('cardex/multi-index/api').MultiIndexItem & import('cardex/multihash-index-sorted/api').MultihashIndexItem} IndexEntry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MultiIndexItem being "an index with many indexes in" and "MultihashIndexItem" being an index of multihashes and this type being potentially either of the, is going to really hard to discuss as humans without getting tied in knots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this makes it easier to understand but this is the type heirarchy:
interface IndexItem {
digest: Uint8Array
offset: number
}
interface MultihashIndexItem extends IndexItem {
multihash: MultihashDigest
}
interface MultiIndexItem extends IndexItem {
origin: CARLink
}
So MultihashIndexItem
, MultiIndexItem
are both IndexItem
s, it's just MultihashIndexItem
have a multihash
property and MultiIndexItem
have a origin
.
The following index types output the following items:
Index Type | Item Type |
---|---|
IndexSorted |
IndexItem |
MultihashIndexSorted |
MultihashIndexItem |
MultiIndex |
MultiIndexItem (an IndexItem or MultihashIndexItem with an additional origin property) |
I should probably change the MultiIndexItem
type to be something like this.
type MultiIndexItem = (IndexItem & { origin: CARLink }) | (MultihashIndexItem & { origin: CARLink })
...but I'm not sure that's helping.
🤖 I have created a release *beep* *boop* --- ## [2.5.0](v2.4.0...v2.5.0) (2023-06-13) ### Features * use blockly indexes ([#51](#51)) ([7086932](7086932)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR adds support for "block link indexes" as a fallback, if content cannot be served by DUDEWHERE and SATNAV.
It binds a new bucket
BLOCKLY
(BLOCK+LYnks) and adds a new index implementation that uses the files stored in it.Blockly is a collection of multi-indexes for individual blocks. Each index contains the CID of a CAR file the block may be found in and the block's byte offset. The index also contains the same detail for any links the block has.