Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

pass in context for related calls to allow for caboose affinity #53

Closed
willscott opened this issue Feb 24, 2023 · 4 comments · Fixed by #54
Closed

pass in context for related calls to allow for caboose affinity #53

willscott opened this issue Feb 24, 2023 · 4 comments · Fixed by #54
Milestone

Comments

@willscott
Copy link
Collaborator

caboose was coded in it's current blockstore incarnation to expect that requests would have a key on the context for the 'root cid' of the request that could better group related inbound requests.

this is set via an affinityKey https://github.com/filecoin-saturn/caboose/blob/main/caboose.go#L134 to the config, and then
using context.WithValue() to set the key to a value when it makes its way to the blockstore.

We can probably have that value just be the inbound request URL and things will get much better
(e.g. the caching of block on L1s will actually work)

@welcome

This comment was marked as resolved.

@lidel
Copy link
Collaborator

lidel commented Feb 24, 2023

I think we have a pretty good candidate for “affinity” key in gateways: the content path (after subdomain/host header normalizations):

https://github.com/ipfs/go-libipfs/blob/6c0f842feed4c8a0ef24d38c94b6e28e622f25d7/gateway/handler.go#L358:

contentPath := ipath.New(r.URL.Path)

It will always look like /ip[nf]s/id/... and give us not only affinity per request, but across all requests for the same path, no matter if request is subdomain, dnslink hosting, or a plain path gateway.

Caboose could read it and decide if it wants to use the full path, or only the root identifier (which would improve affinity even further, grouping all block requests for the same content root together).

+ const GatewayContentPathKey = "contentPath"
contentPath := ipath.New(r.URL.Path)
+ ctx := context.WithValue(r.Context(), GatewayContentPathKey, contentPath)
+ r = r.WithContext(ctx)

@hacdias will you have time to wire this up in go-libipfs/gateway and bubble up to a PR here? 🙏

@lidel lidel added this to the M0.3 milestone Feb 24, 2023
@hacdias
Copy link
Collaborator

hacdias commented Feb 24, 2023

I also want to add here that since context keys should not be strings, the affinity key in caboose might have to be changed to interface{} or something: https://pkg.go.dev/context#WithValue - I really don't like the idea of sending information via context as it has created bugs in the past.

@lidel
Copy link
Collaborator

lidel commented Feb 24, 2023

I removed the need for interface{} and added us some type-safety in filecoin-saturn/caboose#46 which ensures we have no silent failures around affinity key – contentPath := affG.(ipath.Path).String() will loudly fail if we ever mess up with the type of gateway.ContentPathKey.

This should be good enough for now, we can refine what we do with contentPath later (as noted in comments of filecoin-saturn/caboose#46)

@lidel lidel closed this as completed in #54 Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants