Add support for retrieval of PDP pieces without CDN enabled #23
Conversation
There was a problem hiding this comment.
I find it difficult to review this pull request when we don't know the high-level design for checking the retrievability of PDP deals.
For example, depending on that design, we may not need to support all three possible values for the withCDN filter. Maybe all we need is two modes:
- Sample all PDP deals & retrieve directly from SP
- Sample CDN deals only & retrieve via FilCDN
Then we need to consider how to report the outcomes. Currently, filcdn-bot is highly tailored to our needs:
- Produce synthetic traffic to populate our dashboard.
- Alert us when data is not retrievable (but ignore failures caused by SPs.
What outputs does FilOz need from the checker testing direct retrievals of all PDP deals?
In the linked document it's stated that "For non-CDN proofsets, run retrieval checks at a lower frequency (e.g., every hour at random)". We're already doing a similar thing where we're testing a random deals (only in our case we're doing it every second, but that's adjustable). https://github.com/filcdn/bot/blob/a064225ba5da95752c9cc84abfbd648a6b50b677/bin/bot.js#L40-L52 However this PR does not introduce testing of non-CDN deals, it only enables the support for it.
This update introduces support for two (similar) modes:
However, we could change the fist model to sample for first random PDP deal if
At the moment of publishing this PR it was unclear who would operate this bot and collect data for non-CDN deals. Now when we know that FilCDN will continue to operate the bot in extended capacity we'd need to discuss on how we should collect the stats. This could be also done is some follow-up PRs.
@jennijuju would you be able to comment on this? |
| proofSetId: setId, | ||
| }) | ||
| if (!baseUrl) { | ||
| console.error( |
There was a problem hiding this comment.
| console.error( | |
| throw new Error( |
Otherwise withCDN=false tests can silently fail
index.js
Outdated
| if (withCDN) { | ||
| baseUrl = `https://${clientAddress}.${CDN_HOSTNAME}` | ||
| } else { | ||
| baseUrl = await maybeGetResolvedProofSetRetrievalUrl({ |
There was a problem hiding this comment.
This repo needs to be updated to new naming (data set, fwss, piece, etc)
There was a problem hiding this comment.
I will take care of this now
|
I propose to close this PR as no longer relevant. See https://github.com/FilOzone/dealbot
|
@bajtos You're right, the deal bot now has retrieval component built-in. Closing in favor of https://github.com/FilOzone/dealbot. |
Add support for retrieval of pieces from PDP data sets that don't have
withCDNflag enabled. Changes do not include testing of such deals, it only extends the support with ability to do so.Related issue https://www.notion.so/filecoindev/Deal-bot-24cdc41950c1809484d7f2741cdddf53?source=copy_link