feat: add dependencies for index/add invocations
#363
Conversation
View stack outputs
|
| @@ -66,6 +66,7 @@ export function UploadApiStack({ stack, app }) { | |||
| customDomain, | |||
| defaults: { | |||
| function: { | |||
| timeout: '30 seconds', | |||
There was a problem hiding this comment.
Existing indexer lambda (the thing that reads CAR files and writes to dynamodb) seems to max out at ~14s, setting to 30s to be safe.
| /** @param {import('@web3-storage/upload-api').ShardedDAGIndex} index */ | ||
| async publish (index) { | ||
| /** @type {import('multiformats').MultihashDigest[]} */ | ||
| const items = [...index.shards.keys()] |
There was a problem hiding this comment.
Can you please add comment explaining this, I'm very confused why would keys be digests and then why do we add more digests later. I'm sure it will become more clear as I reed more code, yet comment here would make it easier for a new reader to follow the logic.
There was a problem hiding this comment.
I found my answer here, yet I'm not sure we should advertise shard multihashes unless they were explicitly listed under slices. Rational is here is if user didn't index it they don't want it to be advertised, also we would not find those in content claims and serve over bitswap, wolud we ?
| items.push(digest) | ||
| records.push({ | ||
| digest, | ||
| location: new URL(`https://w3s.link/blob/${base58btc.encode(digest.bytes)}`), |
There was a problem hiding this comment.
Nit: could we use raw CIDs instead ? Reduced API surface would be better IMO
| } | ||
| } | ||
|
|
||
| export class BlocksCarsPositionStore { |
There was a problem hiding this comment.
I think I would call this BlockIndexStore or BlockRangeStore. Either way seems like dropping Cars from name would be a good idea.
| for (const res of results) { | ||
| if (res.error) return res | ||
| } | ||
| return ok({}) |
There was a problem hiding this comment.
I wonder if we should put records first and only push items to queue if that was a success. Otherwise we may end up with having blocks advertised on IPNI but have no ability to serve those because we don't have index for them.
| } | ||
| }) | ||
|
|
||
| export class MultihashesQueue { |
There was a problem hiding this comment.
I suggest calling this something like BlockAdventismentPublisher or something along those lines, I think it is more important to describe the role/job component has in the system than it is to describe mechanics of how it does it. In this instance it is component for announcing blocks on the network fact that it uses queues and multihashes are implementation details.
| try { | ||
| const items = [...digests] | ||
| while (true) { | ||
| const batch = items.splice(0, 10) |
There was a problem hiding this comment.
Why 10 at a time ? Should this be a part of the config with 10 as an implicit default ? Still worth adding a comment why the default of 10 was chosen.
There was a problem hiding this comment.
It's the max number of messages you can send to an SQS queue in a batch.
| const batch = items.splice(0, 10) | ||
| if (!batch.length) break | ||
|
|
||
| const dedupedBatch = new Set(batch.map(s => base58btc.encode(s.bytes))) |
There was a problem hiding this comment.
Sounds like deduping on line 81 would make more sense as that would eliminate dupes if they are 10 items apart also.
| useIPNIService(multihashesQueue, carsBlocksPositionStore), | ||
| { | ||
| /** @param {import('multiformats').MultihashDigest} digest */ | ||
| async query (digest) { |
There was a problem hiding this comment.
Nit: I think calling this has or contains would be far less confusing
There was a problem hiding this comment.
Yes I did consider this. However I don't believe there's an explicit IPNI API for that, and I thought at some point we might want query to actually return the result of querying IPNI for a multihash.
Should have changed in w3up types before now...
upload-api/test/helpers/queue.js
Outdated
| } | ||
|
|
||
| return messages | ||
| } |
Upgrades to
@web3-storage/upload-apiv13 and adds dependencies forindex/addinvocations.Adds multihashes to the Elastic IPFS SQS multihashes queue and adds block index data to Dynamo
blocks-cars-positiontable.Note: I did not also generate dudewhere/satnav indexes. Hoverboard reads directly from dynamo and Freeway uses location claims (generated from dynamo). I don't think we need them anymore.