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

gateway: use regular cache-control for CARs #295

Merged
merged 1 commit into from Jul 6, 2022
Merged

Conversation

lidel
Copy link
Member

@lidel lidel commented Jul 4, 2022

I remember discussing this with some gateway operators, but forgot to fix this before #283 got merged.

This PR removes special-casing of CAR responses,
and makes them follow the same Cache-Control rules as raw blocks and regular files:

  • Cache-Control: public, max-age=29030400, immutable must be returned for
    every immutable resource under /ipfs/ namespace.
  • Cache-Control: public, max-age=<ttl> should be returned for mutable
    resources under /ipns/{id-with-ttl}/

Rationale

This fix already landed in ipfs/kubo#9080 and will be released in kubo 0.14-rc1.

The initial draft used Cache-Control: no-cache, no-transform for CARs as a precaution, as desired behavior and response "determinism" was unclear.

  • Main question was: is behavior in go-ipfs the canonical one, or do we want to allow future implementers to do more efficient things like return blocks in the (random) order they arrive from datastore?
  • Second question: what is lesser evil: caching partial CAR responses (in rare case connection gets dropped mid-stream), or not caching them at all.

After talking with gateway operators, and folks who push CARs over HTTP the feedback was unanimous: caching CARs the same way regular files/blocks already are is better.

Handling partial CAR responses

Regular HTTP practices apply: a client can easily detect partial DAG and fetch missing parts in new requests, or retry the original one with Cache-Control: no-cache.

Handling undefined block order

With regard to "block order" in a CAR, I believe it is already covered by use of weak Etag for CARs:

  • When a gateway can’t guarantee byte-for-byte identical responses, a “weak”
    etag should be used. For example, if CAR is streamed, and blocks arrive in
    non-deterministic order, the response should have Etag: W/"bafy…foo.car"

We are still disabling range requests if block order is not deterministic:

Accept-Ranges: none should be returned with application/vnd.ipld.car responses if the block order in CAR stream is not deterministic.

With these, there is no need to disable HTTP caching, as CARs will be logically equivalent, and reusing cached response is beneficial.

cc @gmasgras @thattommyhall @thibmeu @ribasushi @mikeal (just FYI, no need to respond, I believe we want to maximize HTTP caching of CAR responses, but lmk if any concerns)

Forgot to fix this before specs were merged. 
We already use weak Etag for CARs, no need to disable HTTP caching.
lidel added a commit to ipfs/kubo that referenced this pull request Jul 4, 2022
@lidel lidel self-assigned this Jul 4, 2022
Jorropo pushed a commit to ipfs/kubo that referenced this pull request Jul 6, 2022
@lidel lidel merged commit f24617d into main Jul 6, 2022
@lidel lidel deleted the fix/cache-control-car branch July 6, 2022 16:49
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant