-
Notifications
You must be signed in to change notification settings - Fork 40
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: fnet-163 process dag-pb unixfs files #26
Conversation
async fn request(&self, cid: &Cid) -> Result<Bytes, IpldError> { | ||
let url = self.ipfs_url.clone(); | ||
let url = url.join(&format!("ipfs/{}?format=raw", cid))?; | ||
let response = reqwest::get(url).await?; | ||
response.bytes().await.map_err(Into::into) | ||
} |
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.
It seems like we're making an individual request for each ipld block here, however it would be much faster to gather blocks from a streamed CAR file and lazily evaluate whole files/folders from them as we get each block. Since car archives would include every ipld block needed for the root cid, and we'd no longer have to deal with TLS roundtripping in between each small piece of data, we'd maintain better network throughput and reduce latency on cold content
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.
Nice tip. I will address this in the next PR where I will also improve memory management.
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.
I created this ticket that I am going to address today https://linear.app/fleekxyz/issue/FNET-191/improve-ipld-download-and-memory-management @ozwaldorf. If you are ok, I will improve this in another PR.
Thanks for taking the time to review it @ozwaldorf
No description provided.