Skip to content

fix: shorten /routing/v1 stale windows #1195

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.

Sign up for GitHub

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

Open
lidel wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −27
Open

fix: shorten /routing/v1 stale windows #1195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ The following emojis are used to highlight certain changes:

### Changed

- `routing/http/server`: `/routing/v1` responses no longer let caches serve a two day old answer while the origin is healthy. `stale-while-revalidate` is now 10 minutes for responses with results and 1 minute for empty ones, which is enough to cover a background refresh. `stale-if-error` keeps the 48h Amino DHT expiration window for responses with results and drops to 1 hour for empty ones, since it only applies when the origin is failing. `max-age` is unchanged. The peer addresses in routing results come from short-lived sources such as relay reservations, so a stale window measured in days handed clients addresses that stopped working long ago. [#1195](https://github.com/ipfs/boxo/pull/1195)

### Removed

### Fixed
Expand Down
46 changes: 34 additions & 12 deletions routing/http/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ func (s *server) GetIPNS(w http.ResponseWriter, r *http.Request) {
if err != nil {
if errors.Is(err, routing.ErrNotFound) {
// Per IPIP-0513: Return 200 with text/plain to indicate no record found
setCacheControl(w, maxAgeWithoutResults, maxStale)
setCacheControlWithoutResults(w)
http.Error(w, fmt.Sprintf("delegate error: %s", err), http.StatusOK)
return
} else {
Expand Down Expand Up @@ -686,11 +686,24 @@ func (s *server) getClosestPeersNDJSON(w http.ResponseWriter, peersIter iter.Res
var (
// Rule-of-thumb Cache-Control policy is to work well with caching proxies and load balancers.
// If there are any results, cache on the client for longer, and hint any in-between caches to
// serve cached result and upddate cache in background as long we have
// result that is within Amino DHT expiration window
// serve cached result and update cache in background.
maxAgeWithResults = int((5 * time.Minute).Seconds()) // cache >0 results for longer
maxAgeWithoutResults = int((15 * time.Second).Seconds()) // cache no results briefly
maxStale = int((48 * time.Hour).Seconds()) // allow stale results as long within Amino DHT Expiration window

// stale-while-revalidate is served while the origin is healthy, so it only
// needs to cover a background refresh. Routing results churn much faster
// than the Amino DHT provider record expiration: the peer addresses in them
// come from short-lived sources such as relay reservations, so a window
// measured in days hands clients addresses that stopped working long ago.
staleWhileRevalidateWithResults = int((10 * time.Minute).Seconds())
staleWhileRevalidateWithoutResults = int((1 * time.Minute).Seconds())

// stale-if-error only applies when the origin is failing, where a stale
// answer beats no answer. Results stay usable as long as they are within the
// Amino DHT expiration window. An empty answer is worth little, so it is not
// held nearly as long.
staleIfErrorWithResults = int((48 * time.Hour).Seconds())
staleIfErrorWithoutResults = int((1 * time.Hour).Seconds())
)

func parsePeerID(pidStr string) (peer.ID, error) {
Expand Down Expand Up @@ -748,8 +761,16 @@ func parseKey(keyStr string) (cid.Cid, error) {
return cid.Cid{}, fmt.Errorf("unable to parse as CID or PeerID: %w", errors.Join(cidErr, pidErr))
}

func setCacheControl(w http.ResponseWriter, maxAge int, stale int) {
w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d, stale-while-revalidate=%d, stale-if-error=%d", maxAge, stale, stale))
func setCacheControl(w http.ResponseWriter, maxAge, staleWhileRevalidate, staleIfError int) {
w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d, stale-while-revalidate=%d, stale-if-error=%d", maxAge, staleWhileRevalidate, staleIfError))
}

func setCacheControlWithResults(w http.ResponseWriter) {
setCacheControl(w, maxAgeWithResults, staleWhileRevalidateWithResults, staleIfErrorWithResults)
}

func setCacheControlWithoutResults(w http.ResponseWriter) {
setCacheControl(w, maxAgeWithoutResults, staleWhileRevalidateWithoutResults, staleIfErrorWithoutResults)
}

// setIPNSCacheControl sets Cache-Control for an IPNS record response. An IPNS
Expand All @@ -767,17 +788,18 @@ func setIPNSCacheControl(w http.ResponseWriter, ttl int, remainingValidity int)
return
}
maxAge := min(max(0, ttl), remainingValidity)
setCacheControl(w, maxAge, remainingValidity-maxAge)
stale := remainingValidity - maxAge
setCacheControl(w, maxAge, stale, stale)
}

func writeJSONResult(w http.ResponseWriter, method string, val interface{ Length() int }) {
w.Header().Add("Content-Type", mediaTypeJSON)
w.Header().Add("Vary", "Accept")

if val.Length() > 0 {
setCacheControl(w, maxAgeWithResults, maxStale)
setCacheControlWithResults(w)
} else {
setCacheControl(w, maxAgeWithoutResults, maxStale)
setCacheControlWithoutResults(w)
}
w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat))

Expand All @@ -800,7 +822,7 @@ func writeJSONResult(w http.ResponseWriter, method string, val interface{ Length

func writeErr(w http.ResponseWriter, method string, statusCode int, cause error) {
if errors.Is(cause, routing.ErrNotFound) {
setCacheControl(w, maxAgeWithoutResults, maxStale)
setCacheControlWithoutResults(w)
}

w.WriteHeader(statusCode)
Expand Down Expand Up @@ -844,7 +866,7 @@ func writeResultsIterNDJSON[T types.Record](w http.ResponseWriter, resultIter it
if !hasResults {
hasResults = true
// There's results, cache useful result for longer
setCacheControl(w, maxAgeWithResults, maxStale)
setCacheControlWithResults(w)
}

_, err = w.Write(b)
Expand All @@ -866,7 +888,7 @@ func writeResultsIterNDJSON[T types.Record](w http.ResponseWriter, resultIter it

if !hasResults {
// There weren't results, cache for shorter but still send 200 per IPIP-0513
setCacheControl(w, maxAgeWithoutResults, maxStale)
setCacheControlWithoutResults(w)
w.WriteHeader(http.StatusOK)
}
}
30 changes: 15 additions & 15 deletions routing/http/server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,9 @@ func TestProviders(t *testing.T) {
require.Equal(t, "Accept", resp.Header.Get("Vary"))

if empty {
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=60, stale-if-error=3600", resp.Header.Get("Cache-Control"))
} else {
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))
}
requireCloseToNow(t, resp.Header.Get("Last-Modified"))

Expand Down Expand Up @@ -543,7 +543,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=60, stale-if-error=3600", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))

Expand Down Expand Up @@ -646,7 +646,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))

Expand Down Expand Up @@ -698,7 +698,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))

Expand Down Expand Up @@ -750,7 +750,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))

Expand All @@ -776,7 +776,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=60, stale-if-error=3600", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))

Expand Down Expand Up @@ -814,7 +814,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand Down Expand Up @@ -876,7 +876,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand All @@ -896,7 +896,7 @@ func TestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand Down Expand Up @@ -1020,7 +1020,7 @@ func TestGetClosestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=60, stale-if-error=3600", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))
})
Expand Down Expand Up @@ -1106,7 +1106,7 @@ func TestGetClosestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))

Expand All @@ -1133,7 +1133,7 @@ func TestGetClosestPeers(t *testing.T) {

require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=60, stale-if-error=3600", resp.Header.Get("Cache-Control"))

requireCloseToNow(t, resp.Header.Get("Last-Modified"))
})
Expand Down Expand Up @@ -1166,7 +1166,7 @@ func TestGetClosestPeers(t *testing.T) {

require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand Down Expand Up @@ -1235,7 +1235,7 @@ func TestGetClosestPeers(t *testing.T) {

require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=600, stale-if-error=172800", resp.Header.Get("Cache-Control"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand Down
Loading