Skip to content

Commit

Permalink
Remove TTL header from cache key
Browse files Browse the repository at this point in the history
See [discussion on Discord](https://discord.com/channels/928484660785336380/928485908842426389/1231791730513412136). Multiple times, app developers have wanted to cache a piece of content until a particular time, rather than for a particular period. That requires setting ttl_seconds to a variable amount, calculated as the time remaining.

Currently, the cache client won't support that. It generates the cache key from the entire request, which includes the TTL seconds in a header. This change removes that header before generating the key. That seems sensible because the key should be about the content and how it would be fetched, but the TTL is about what to do with the response.

As a transitionary step, this writes to new TTL-less keys, but continues to look up keys with TTLs. Once all the old cache entries have expired, then we can stop looking up the old keys.
  • Loading branch information
dinosaursrarr committed Apr 23, 2024
1 parent ff8f053 commit 549b1bd
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
26 changes: 24 additions & 2 deletions runtime/httpcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const (
HTTPTimeout = 5 * time.Second
MaxResponseBytes = 20 * 1024 * 1024 // 20MB
HTTPCachePrefix = "httpcache"
TTLHeader = "X-Tidbyt-Cache-Seconds"
)

// Status codes that are cacheable as defined here:
Expand Down Expand Up @@ -70,7 +71,12 @@ func (c *cacheClient) RoundTrip(req *http.Request) (*http.Response, error) {
ctx, cancel := context.WithTimeout(ctx, HTTPTimeout)
defer cancel() // need to do this to not leak a goroutine

key, err := cacheKey(req)
key, err := cacheKey(req, false)
if err != nil {
return nil, fmt.Errorf("failed to generate cache key: %w", err)
}
// TODO: remove once old cache entries expire
keyWithTTL, err := cacheKey(req, true)
if err != nil {
return nil, fmt.Errorf("failed to generate cache key: %w", err)
}
Expand All @@ -83,6 +89,14 @@ func (c *cacheClient) RoundTrip(req *http.Request) (*http.Response, error) {
return res, nil
}
}
// TODO: remove once old entries expire
b, exists, err = c.cache.Get(nil, keyWithTTL)
if exists && err == nil {
if res, err := http.ReadResponse(bufio.NewReader(bytes.NewReader(b)), req); err == nil {
res.Header.Set("tidbyt-cache-status", "HIT")
return res, nil
}
}
}

resp, err := c.transport.RoundTrip(req.WithContext(ctx))
Expand All @@ -106,11 +120,19 @@ func (c *cacheClient) RoundTrip(req *http.Request) (*http.Response, error) {
return resp, err
}

func cacheKey(req *http.Request) (string, error) {
func cacheKey(req *http.Request, keep_ttl bool) (string, error) {
// TODO: remove keep_ttl param and make this always happen.
ttl := req.Header.Get(TTLHeader)
if !keep_ttl {
req.Header.Del(TTLHeader)
}
r, err := httputil.DumpRequest(req, true)
if err != nil {
return "", errors.Wrap(err, "failed to serialize request")
}
if ttl != "" {
req.Header.Set(TTLHeader, ttl)
}

h := sha256.Sum256(r)
key := hex.EncodeToString(h[:])
Expand Down
4 changes: 2 additions & 2 deletions runtime/testdata/httpcache.star
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def main(config):

resp = http.get(
url = "https://example.com",
ttl_seconds = 60,
ttl_seconds = 3,
)
assert.eq(resp.headers.get("Tidbyt-Cache-Status"), "HIT")

Expand All @@ -32,7 +32,7 @@ def main(config):
url = "https://example.com",
ttl_seconds = 60,
)
assert.eq(resp.headers.get("Tidbyt-Cache-Status"), "MISS")
assert.eq(resp.headers.get("Tidbyt-Cache-Status"), "HIT")

resp = http.post(
url = "https://example.com",
Expand Down

0 comments on commit 549b1bd

Please sign in to comment.