Skip to content
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

Support context cancellation in Gateway when serving car files #9065

Open
3 tasks done
iand opened this issue Jun 28, 2022 · 5 comments
Open
3 tasks done

Support context cancellation in Gateway when serving car files #9065

iand opened this issue Jun 28, 2022 · 5 comments
Assignees
Labels
kind/enhancement A net-new feature or improvement to an existing feature topic/gateway Topic gateway

Comments

@iand
Copy link
Contributor

iand commented Jun 28, 2022

Checklist

  • My issue is specific & actionable.
  • I am not suggesting a protocol enhancement.
  • I have searched on the issue tracker for my issue.

Description

This is a low priority general enhancement to conserve resources when dealing with slow clients. Car files may be very large and take a long time to transfer and we should respect cancellation of the request context (due to timeout or explicit).

As of commit 862ce6b, car files are served in a single non-cancelable call https://github.com/ipfs/go-ipfs/blob/862ce6bb8f6ce24b91ed3dc59e7406faed34f583/core/corehttp/gateway_handler_car.go#L76

Supporting context cancellation would require go-car to support cancellation when writing and traversing a dag. Currently go-car v1 is being used. The top level function WriteCar accepts a context but it is not used for cancellation. The bulk of the work is performed by merkledag.Walk which only respects context cancellation in the parallel case, which requires an explicit concurrency option to be passed. Changing merkledag.sequentialWalkDepth to check for context cancellation would work here.

Alternatively switching to go-car v2 is an option, using NewSelectiveWriter but this would also require cancellation behavior to be pushed down into go-ipld-prime's traversal package. But see ipld/go-ipld-prime#154 and ipld/go-ipld-prime#303

@iand iand added the kind/enhancement A net-new feature or improvement to an existing feature label Jun 28, 2022
@lidel
Copy link
Member

lidel commented Jul 19, 2022

@schomatis mind adding this to your queue?

Only ask from my end is to keep returning CAR v1.

@lidel lidel added the topic/gateway Topic gateway label Jul 19, 2022
@lidel lidel added this to the Best Effort Track milestone Jul 19, 2022
@schomatis schomatis self-assigned this Jul 19, 2022
@schomatis
Copy link
Contributor

@lidel Can I have some context on #8758, please? Contrary to what I'm reading from Ian we seem to be in IPLD land and not merkledag (I'm only familiar with the second). I'm looking at traverseBlocks()/walkAdv_iterateSelective() but have no idea where is the traversal actually taking place and where would I plumb the context.

@iand
Copy link
Contributor Author

iand commented Jul 22, 2022

See https://github.com/ipfs-shipyard/gateway-prime (here) for a version of the gateway that uses IPLD and go-car v2

@schomatis
Copy link
Contributor

Thanks for the reference @iand. I'm still not sure if that is an answer to my request above or some more general context.

@iand
Copy link
Contributor Author

iand commented Jul 24, 2022

Thanks for the reference @iand. I'm still not sure if that is an answer to my request above or some more general context.

Just general context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature topic/gateway Topic gateway
Projects
No open projects
Status: 🛑 Blocked
Development

No branches or pull requests

3 participants