cache: generalize `Cache` into a key-value cache by hawkw · Pull Request #1683 · linkerd/linkerd2-proxy

hawkw · 2022-05-18T20:15:22Z

Currently, the linkerd-cache crate contains an implementation of a
cache where entries are evicted if they have not been accessed for a
timeout duration. This type currently implements the NewService trait
and requires that cached values are Services. It does not provide a
more general purpose key-value cache interface.

PR #1677 introduces dynamic discovery for inbound port policies. We
would like to implement a similar eviction strategy for these policy
watches. However, the policy watches are not used in a NewService
implementation, but in a CheckPolicy implementation.

Therefore, this branch rewrites the Cache type to serve as a
general-purpose key-value cache, and adds a new NewCachedService type
that implements NewService on top of Cache.

Currently, the `linkerd-cache` crate contains an implementation of a cache where entries are evicted if they have not been accessed for a timeout duration. This type currently implements the `NewService` trait and requires that cached values are `Service`s. It does not provide a more general purpose key-value cache interface. PR #1677 introduces dynamic discovery for inbound port policies. We would like to implement a similar eviction strategy for these policy watches. However, the policy watches are *not* used in a `NewService` implementation, but in a `CheckPolicy` implementation. Therefore, this branch rewrites the `Cache` type to serve as a general-purpose key-value cache, and adds a new `NewCachedService` type that implements `NewService` on top of `Cache`.

this won't actually be needed by policy stuff Signed-off-by: Eliza Weisman <[email protected]>

Signed-off-by: Eliza Weisman <[email protected]>

olix0r

note that there are a few warnings:

linkerd/cache/src/lib.rs

@olix0r

thanks @olix0r Co-authored-by: Oliver Gould <[email protected]>

Signed-off-by: Oliver Gould <[email protected]>

In some cases--like inbound policy discovery--we have a set of permanent values that should never be removed from the cache. This change updates the cache to only hold a handle when eviction is scheduled. No handle is held when the cache entry is permanent. This change also fixes a possible race condition. Previously, the eviction task could consume an entry's handle without locking the cache. This made it possible for a `get_or_insert_with` call to race against the eviction task, adding an entry that would be immediately evicted by the eviction task. Signed-off-by: Oliver Gould <[email protected]>

Signed-off-by: Eliza Weisman <[email protected]>

* Store permanent values in the cache (for #1683) In some cases--like inbound policy discovery--we have a set of permanent values that should never be removed from the cache. This change updates the cache to only hold a handle when eviction is scheduled. No handle is held when the cache entry is permanent. This change also fixes a possible race condition. Previously, the eviction task could consume an entry's handle without locking the cache. This made it possible for a `get_or_insert_with` call to race against the eviction task, adding an entry that would be immediately evicted by the eviction task. Signed-off-by: Oliver Gould <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>

hawkw · 2022-05-19T18:51:38Z

@olix0r I merged #1685 into this PR (after some manual cleanup). WDYT?

This release fixes a problem with HTTP/1.1 `CONNECT` requests. When a server responds to a `CONNECT` request with `content-length` or `transfer-encoding` headers (in violation of RFC 7231), the proxy must actively strip these headers to avoid making the Hyper server fail the response. This release also fixes an issue with the way proxies discover control plane components via DNS. When `SRV` records cannot be resolved, the proxy would no not necessarily fall back to resolving `A` records. This has been fixed. Finally, the inbound proxy can now discover policies dynamically. Ports that are configured in the `LINKERD2_PROXY_INBOUND_PORTS` are discovered as the proxy starts up; but now the proxy will discover policies for ports that are not in this list. The pod's default policy is used initially, but once a policy is received from the control plane it is used. --- * build(deps): bump syn from 1.0.93 to 1.0.94 (linkerd/linkerd2-proxy#1664) * build(deps): bump tj-actions/changed-files from 19 to 20 (linkerd/linkerd2-proxy#1665) * build(deps): bump rustls from 0.20.4 to 0.20.5 (linkerd/linkerd2-proxy#1666) * build(deps): bump ryu from 1.0.9 to 1.0.10 (linkerd/linkerd2-proxy#1667) * build(deps): bump tokio-util from 0.7.1 to 0.7.2 (linkerd/linkerd2-proxy#1668) * build(deps): bump itoa from 1.0.1 to 1.0.2 (linkerd/linkerd2-proxy#1669) * build(deps): bump tonic from 0.7.1 to 0.7.2 (linkerd/linkerd2-proxy#1652) * dns: Fall back to A record when SRV resolution fails (linkerd/linkerd2-proxy#1670) * Consolidate error chain inspection with `cause_ref` (linkerd/linkerd2-proxy#1671) * ci: change how warnings are denied on CI (linkerd/linkerd2-proxy#1662) * build(deps): bump proc-macro2 from 1.0.38 to 1.0.39 (linkerd/linkerd2-proxy#1673) * build(deps): bump libc from 0.2.125 to 0.2.126 (linkerd/linkerd2-proxy#1674) * build(deps): bump syn from 1.0.94 to 1.0.95 (linkerd/linkerd2-proxy#1675) * ci: Use the cargo-action-fmt setup action (linkerd/linkerd2-proxy#1672) * opencensus: Include empty generated protobuf (linkerd/linkerd2-proxy#1676) * build(deps): bump rustls from 0.20.5 to 0.20.6 (linkerd/linkerd2-proxy#1679) * Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (linkerd/linkerd2-proxy#1654)" (#1681) * build(deps): bump EmbarkStudios/cargo-deny-action from 1.2.17 to 1.3.0 (linkerd/linkerd2-proxy#1678) * build(deps): bump clang-sys from 1.3.1 to 1.3.2 (linkerd/linkerd2-proxy#1680) * cache: generalize `Cache` into a key-value cache (linkerd/linkerd2-proxy#1683) * build(deps): bump once_cell from 1.10.0 to 1.11.0 (linkerd/linkerd2-proxy#1687) * build(deps): bump EmbarkStudios/cargo-deny-action from 1.3.0 to 1.3.1 (linkerd/linkerd2-proxy#1686) * trace: add `/logs.json` endpoint to admin server (linkerd/linkerd2-proxy#1642) * Dynamically discover policies for undocumented ports (linkerd/linkerd2-proxy#1677) * Don't allow a policy to be used if `check_port_allowed` fails (linkerd/linkerd2-proxy#1689) * ci: Simplify release workflow (linkerd/linkerd2-proxy#1690) * build(deps): bump petgraph from 0.6.0 to 0.6.1 (linkerd/linkerd2-proxy#1696) * build(deps): bump actions/upload-artifact from 3.0.0 to 3.1.0 (linkerd/linkerd2-proxy#1692) * build(deps): bump tj-actions/changed-files from 20 to 20.1 (linkerd/linkerd2-proxy#1693) * build(deps): bump http-body from 0.4.4 to 0.4.5 (linkerd/linkerd2-proxy#1694) * build(deps): bump regex from 1.5.5 to 1.5.6 (linkerd/linkerd2-proxy#1695) * build(deps): bump regex-syntax from 0.6.25 to 0.6.26 (linkerd/linkerd2-proxy#1697) * http: Strip illegal headers from CONNECT responses (linkerd/linkerd2-proxy#1699) * dev: Replace `Makefile` with `justfile` (linkerd/linkerd2-proxy#1691) Signed-off-by: Oliver Gould <[email protected]>

hawkw added 7 commits May 18, 2022 11:28

get rid of annoying ref stuff

31443af

this won't actually be needed by policy stuff Signed-off-by: Eliza Weisman <[email protected]>

add Cached::uncached

a2f8b91

Signed-off-by: Eliza Weisman <[email protected]>

cache: allow overriding hasher

f0b2fee

Signed-off-by: Eliza Weisman <[email protected]>

cache: add Cache::from_iter

eb925f3

Signed-off-by: Eliza Weisman <[email protected]>

cache: add DerefMut

15e6fe1

Signed-off-by: Eliza Weisman <[email protected]>

rustfmt

1327399

hawkw requested a review from a team as a code owner May 18, 2022 20:15

hawkw mentioned this pull request May 18, 2022

evict inbound dynamic policy discovery watches #1684

Merged

olix0r reviewed May 18, 2022

View reviewed changes

linkerd/cache/src/lib.rs Outdated Show resolved Hide resolved

linkerd/cache/src/lib.rs Outdated Show resolved Hide resolved

hawkw and others added 3 commits May 18, 2022 13:45

fix warnings

679f341

thanks @olix0r Co-authored-by: Oliver Gould <[email protected]>

Revert unneeded Cargo.lock change

f956ed2

Signed-off-by: Oliver Gould <[email protected]>

newline

c714f71

Signed-off-by: Oliver Gould <[email protected]>

olix0r force-pushed the eliza/generalize-cache branch from da33130 to c714f71 Compare May 19, 2022 02:29

hawkw and others added 2 commits May 19, 2022 10:02

cache: add fixed (non-expiring) entries

3525055

Signed-off-by: Eliza Weisman <[email protected]>

hawkw requested a review from olix0r May 19, 2022 18:51

olix0r approved these changes May 19, 2022

View reviewed changes

hawkw merged commit 3d0eb33 into main May 19, 2022

hawkw deleted the eliza/generalize-cache branch May 19, 2022 19:43

olix0r mentioned this pull request May 24, 2022

proxy: v2.179.0 linkerd/linkerd2#8543

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cache: generalize `Cache` into a key-value cache#1683

cache: generalize `Cache` into a key-value cache#1683
hawkw merged 12 commits intomainfrom
eliza/generalize-cache

hawkw commented May 18, 2022

Uh oh!

olix0r left a comment

Uh oh!

Uh oh!

Uh oh!

hawkw commented May 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hawkw commented May 18, 2022

Uh oh!

olix0r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hawkw commented May 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants