feat: set max number of car cids to resolve #24
Conversation
vasco-santos
force-pushed
the
feat/set-max-number-of-car-cids-to-resolve
branch
3 times, most recently
from
30f3426 to
9970a01
Compare
vasco-santos
force-pushed
the
feat/set-max-number-of-car-cids-to-resolve
branch
from
9970a01 to
1203d40
Compare
src/middleware.js
Outdated
| // the sub-requests limit and not serve content anyway | ||
| if (carCids.length > Number(env.MAX_CAR_CIDS_TO_RESOLVE)) { | ||
| throw new HttpError('number CAR CIDs is too large to resolve', { status: 501 }) | ||
| } |
There was a problem hiding this comment.
Move into the while loop above for quicker response?
wrangler.toml
Outdated
| @@ -27,6 +27,9 @@ r2_buckets = [ | |||
| [env.production.build] | |||
| command = "npm run build" | |||
|
|
|||
| [env.production.vars] | |||
| MAX_CAR_CIDS_TO_RESOLVE = "250" | |||
There was a problem hiding this comment.
It's tricky because a DAG might be split across 999 CARs but because of the path you might be able to extract /smol.json with the last sub-request.
There was a problem hiding this comment.
With our 10MB CAR shards it'll restrict freeway from serving anything that's over 2.5GB
There was a problem hiding this comment.
I understand the disadvantage for these use cases. However, at this point looks like the priority should be to reduce R2 ops and avoid potential error states mentioned in issue (break in mid stream without feedback for user, break after worker run out of sub-requests in general). When we have bigger sub-requests limit and better R2 perf we can increase these.
We can tweak more the number if you feel like there is a better compromise. Thoughts?
Co-authored-by: Alan Shaw <[email protected]>
vasco-santos
force-pushed
the
feat/set-max-number-of-car-cids-to-resolve
branch
from
2225c2b to
9d4c9ff
Compare
🤖 I have created a release *beep* *boop* --- ## [1.6.0](v1.5.3...v1.6.0) (2023-03-02) ### Features * set max number of car cids to resolve ([#24](#24)) ([94e65ea](94e65ea)) --- 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>
Sets max number of car CIDs to resolve based on issue and numbers suggested in #22
Later on, once bigger chunks changes is largely propagated, we can consider increasing this. Also, going to create an issue to add logging to
freewayso that we can log number of car CIDs when we throw this error back so that we can have better visibility on these numbers