diff --git a/deploy/helm/container-cache/deploy/files/nginx.conf b/deploy/helm/container-cache/deploy/files/nginx.conf index cbe62212a..a58839ded 100644 --- a/deploy/helm/container-cache/deploy/files/nginx.conf +++ b/deploy/helm/container-cache/deploy/files/nginx.conf @@ -70,8 +70,16 @@ proxy_http_request_time_ms_total = prometheus:counter( "proxy_cache_http_requests_time_ms_total", "Sum of time elapsed between reading the first bytes from the client and writing a log entry after the last bytes were sent for each request", {"request_method", "cache_status", "http_status"}) + {{- /* + `route` distinguishes how a request was served under + consistent-hash routing, and is always "local" when it is off: + local served from this pod's own cache or origin fetch + relayed forwarded to the owner pod, so it paid the peer hop + peer served for another pod that relayed it here + Three bounded values, so this does not add unbounded cardinality. + */}} proxy_http_requests_count = prometheus:counter( - "proxy_cache_http_requests_count", "Count of requests with additional labels", {"request_method", "cache_status", "http_status"}) + "proxy_cache_http_requests_count", "Count of requests with additional labels", {"request_method", "cache_status", "http_status", "route"}) proxy_http_upstream_requests_count = prometheus:counter( "proxy_cache_http_upstream_requests_count", "Count of upstream requests with additional labels", {"request_method", "cache_status", "http_status"}) @@ -80,13 +88,28 @@ "proxy_cache_response_body_size_bytes", "Number of bytes sent to client, not counting the response header", {"cache_status", "http_status"}) proxy_cache_throughput = prometheus:histogram( - "proxy_cache_throughput", "Throughput of proxy cache(bytes/msec)", {"cache_status", "http_status"}, { {{ $.Values.metrics.throughputHistogramBuckets | default "25000000, 30000000, 35000000, 40000000, 50000000, 60000000, 80000000, 100000000" }} }) + "proxy_cache_throughput", "Throughput of proxy cache(bytes/msec)", {"cache_status", "http_status", "route"}, { {{ $.Values.metrics.throughputHistogramBuckets | default "25000000, 30000000, 35000000, 40000000, 50000000, 60000000, 80000000, 100000000" }} }) proxy_cache_upstream_throughput = prometheus:histogram( "proxy_cache_upstream_throughput", "Throughput of upstream server(bytes/msec)", {"host", "http_status"}, { {{ $.Values.metrics.throughputHistogramBuckets | default "25000000, 30000000, 35000000, 40000000, 50000000, 60000000, 80000000, 100000000" }} }) - metric_latency = prometheus:histogram("proxy_cache_request_duration_seconds", "HTTP request latency", {"host", "request_method", "cache_status", "http_status"}, {0.005, 0.01, 0.02, 0.03, 0.05, 0.075, 0.1, 0.2, 0.3, 0.4, 0.5, 0.75, 1, 1.5, 2, 3, 4, 5, 10}) - metric_upstream_latency = prometheus:histogram("proxy_cache_upstream_request_duration_seconds", "HTTP request latency", {"host", "request_method", "cache_status", "http_status"}, {0.005, 0.01, 0.02, 0.03, 0.05, 0.075, 0.1, 0.2, 0.3, 0.4, 0.5, 0.75, 1, 1.5, 2, 3, 4, 5, 10}) + {{- /* + Duration buckets must cover a whole object transfer, not an API + call. These are multi-hundred-MB model files, so a request that + takes tens of seconds is normal and says nothing about cache + health. With a 10s top bucket a large share of traffic landed in + +Inf, and histogram_quantile clamps at the last finite bucket, so + any reported high quantile was the bucket edge rather than a + measurement. Buckets past 10s exist to make those quantiles real. + */}} + metric_latency = prometheus:histogram("proxy_cache_request_duration_seconds", "HTTP request latency", {"host", "request_method", "cache_status", "http_status", "route"}, { {{ $.Values.metrics.durationHistogramBuckets | default "0.005, 0.01, 0.02, 0.03, 0.05, 0.075, 0.1, 0.2, 0.3, 0.4, 0.5, 0.75, 1, 1.5, 2, 3, 4, 5, 10, 15, 30, 60, 120, 300, 600" }} }) + metric_upstream_latency = prometheus:histogram("proxy_cache_upstream_request_duration_seconds", "HTTP request latency", {"host", "request_method", "cache_status", "http_status"}, { {{ $.Values.metrics.durationHistogramBuckets | default "0.005, 0.01, 0.02, 0.03, 0.05, 0.075, 0.1, 0.2, 0.3, 0.4, 0.5, 0.75, 1, 1.5, 2, 3, 4, 5, 10, 15, 30, 60, 120, 300, 600" }} }) metric_connections = prometheus:gauge("nginx_http_connections", "Number of HTTP connections", {"state"}) - metric_response_sizes = prometheus:histogram("proxy_cache_response_size_bytes", "Size of HTTP responses", nil,{1000,2000,4000,8000,16000,32000,64000,128000,256000,1048576, 10485760, 104857600, 1000000000, 10000000000, 100000000000}) + {{- /* + Response-size buckets previously jumped 100MB -> 1GB -> 10GB, so + essentially all model traffic fell in a single bucket and the + distribution was unreadable. The added steps give resolution + across the range these objects actually occupy. + */}} + metric_response_sizes = prometheus:histogram("proxy_cache_response_size_bytes", "Size of HTTP responses", nil, { {{ $.Values.metrics.responseSizeHistogramBuckets | default "1000, 2000, 4000, 8000, 16000, 32000, 64000, 128000, 256000, 1048576, 10485760, 104857600, 268435456, 536870912, 805306368, 1073741824, 2147483648, 5368709120, 10737418240, 107374182400" }} }) instance_info = prometheus:gauge("proxy_cache_instance_info", "Information about proxy cache instance", {"chart_version"}) instance_info:set(1, { "{{ $.Chart.Version }}" }) @@ -129,6 +152,13 @@ {{- if gt $n 1 }} server {{ $.Release.Name }}-peer-{{ mod (add $i 1) $n }}.{{ $.Release.Namespace }}.svc.cluster.local:{{ $port }} backup; {{- end }} + # Keep idle connections to the peer. Without this every relayed + # request pays a fresh TCP and TLS handshake, which is worst for the + # many small Range requests the hash spreads across owners. + # @cc_relay must also send `Connection ""` for these to be reused. + keepalive {{ ($.Values.consistentHashRouting).peerKeepaliveConnections | default 32 }}; + keepalive_timeout {{ ($.Values.consistentHashRouting).peerKeepaliveTimeout | default "60s" }}; + keepalive_requests {{ ($.Values.consistentHashRouting).peerKeepaliveRequests | default 1000 }}; } {{- end }} {{- end }} diff --git a/deploy/helm/container-cache/deploy/files/proxy-common.conf b/deploy/helm/container-cache/deploy/files/proxy-common.conf index ef9b73491..aa595adea 100644 --- a/deploy/helm/container-cache/deploy/files/proxy-common.conf +++ b/deploy/helm/container-cache/deploy/files/proxy-common.conf @@ -32,14 +32,32 @@ local upstream_time = tonumber(ngx.var.upstream_response_time) local host = ngx.var.host + -- How this request was served. Peer routing is optional, and when + -- it is off every request is served here, so "local" is accurate + -- and the lookup below is not emitted at all. The routing variable + -- is only declared when routing is enabled, and reading an + -- undeclared variable raises in OpenResty. + local route = "local" +{{- if ($.Values.consistentHashRouting).enabled }} + -- Non-empty owner means the route helper sent this to a peer, so it + -- paid the relay hop. The inbound relay marker means this pod is the + -- owner answering for a peer. + local cc_owner = ngx.var.cc_owner + if cc_owner ~= nil and cc_owner ~= "" then + route = "relayed" + elseif ngx.var.http_x_nvcf_cc_relayed == "1" then + route = "peer" + end +{{- end }} + if request_uri ~= nil and cache_status ~= nil and remote_addr ~= nil and not string.find(request_uri, "manifest") then if request_time ~= nil and request_time > 0 then proxy_http_request_time_ms_total:inc(request_time, {request_method, cache_status, http_status}) - proxy_http_requests_count:inc(1, {request_method, cache_status, http_status}) + proxy_http_requests_count:inc(1, {request_method, cache_status, http_status, route}) - metric_latency:observe(request_time, {host, request_method, cache_status, http_status}) + metric_latency:observe(request_time, {host, request_method, cache_status, http_status, route}) if body_size ~= nil then - proxy_cache_throughput:observe(body_size / request_time, {cache_status, http_status}) + proxy_cache_throughput:observe(body_size / request_time, {cache_status, http_status, route}) if upstream_time ~= nil and upstream_time > 0 then proxy_cache_upstream_throughput:observe(body_size / upstream_time, {host, http_status}) metric_upstream_latency:observe(upstream_time, {host, request_method, cache_status, http_status}) @@ -154,15 +172,38 @@ # every block, so the relay location is not duplicated per block. set $cc_owner ""; location @cc_relay { - # Relay to the owner pod: a pure stream that never writes the local - # cache or temp files; only the owner stores the object. lua-access - # runs on the owner, which sees the original request headers. + # Relay to the owner pod. lua-access runs on the owner, which sees the + # original request headers. + {{- $relayMinUses := int (($.Values.consistentHashRouting).relayCacheMinUses | default 0) }} + {{- if gt $relayMinUses 0 }} + # Objects pulled through this pod repeatedly are replicated locally + # after relayCacheMinUses requests, so hot objects stop paying the + # relay hop while one-off objects still live only on their owner. + # Keyed on $cc_hash_key, which each server block sets to the same + # value it uses for proxy_cache_key, so the local copy has exactly the + # owner's cache identity. Bounded by the same min_free eviction as + # every other zone. + proxy_cache proxy_ngc; + proxy_cache_key $cc_hash_key; + proxy_cache_min_uses {{ $relayMinUses }}; + proxy_cache_valid 200 206 {{ $.Values.cache.valid | default "31d" }}; + {{- else }} + # A pure stream that never writes the local cache; only the owner + # stores the object. Set consistentHashRouting.relayCacheMinUses to + # replicate hot objects locally instead. proxy_cache off; + {{- end }} proxy_http_version 1.1; proxy_pass_request_headers on; proxy_set_header Host $host; proxy_set_header Range $http_range; proxy_set_header X-NVCF-CC-Relayed "1"; + # Declaring any proxy_set_header here cancels inheritance of the + # server-level set, which includes `Connection ""`. Without repeating + # it nginx sends its default `Connection: close` and the upstream + # keepalive pool is never used, so every relay reconnects and + # re-handshakes TLS. + proxy_set_header Connection ""; # The peer serves the same ssl listener with a per-pod internal leaf # cert, so system-CA verification cannot pass. Verification is skipped # on this internal peer hop only; the tier is an internal, diff --git a/deploy/helm/container-cache/deploy/values.yaml b/deploy/helm/container-cache/deploy/values.yaml index 877730801..5671b73ac 100644 --- a/deploy/helm/container-cache/deploy/values.yaml +++ b/deploy/helm/container-cache/deploy/values.yaml @@ -191,11 +191,38 @@ consistentHashRouting: # Owner pods are in-cluster peers, so this stays short: a slow connect means # the owner is down, and failing over fast is correct. relayConnectTimeout: 2s + # Idle connections kept open to each owner pod. Without a pool every relayed + # request pays a fresh TCP and TLS handshake to the peer, which is worst for + # the many small Range requests the hash spreads across owners. Cheap: these + # are in-cluster peers and the pool is per worker process. + peerKeepaliveConnections: 32 + peerKeepaliveTimeout: 60s + peerKeepaliveRequests: 1000 + # Replicate an object locally after this many requests arrive here for an + # object owned by another pod, so hot objects stop paying the relay hop while + # one-off objects still live only on their owner. Storage is bounded by the + # same min_free eviction as every other zone, and the extra copies are by + # definition the objects being requested most. + # + # 0 disables local replication and restores the strict single-copy behavior, + # where every request for a non-owned object relays for the object's whole + # lifetime. + relayCacheMinUses: 3 # Metrics configuration. metrics: cacheMetricsStorageSize: 300m throughputHistogramBuckets: 25000000, 30000000, 35000000, 40000000, 50000000, 60000000, 80000000, 100000000 + # Request-duration buckets. These requests are whole model-file transfers, not + # API calls, so tens of seconds is normal and the top bucket has to cover a + # full object. If the largest finite bucket is below real traffic, everything + # above it lands in +Inf and histogram_quantile clamps to that edge, reporting + # the bucket boundary instead of a latency. + durationHistogramBuckets: 0.005, 0.01, 0.02, 0.03, 0.05, 0.075, 0.1, 0.2, 0.3, 0.4, 0.5, 0.75, 1, 1.5, 2, 3, 4, 5, 10, 15, 30, 60, 120, 300, 600 + # Response-size buckets, with resolution across the range model objects + # actually occupy. A 100MB -> 1GB -> 10GB ladder puts nearly all traffic in + # one bucket and hides the distribution entirely. + responseSizeHistogramBuckets: 1000, 2000, 4000, 8000, 16000, 32000, 64000, 128000, 256000, 1048576, 10485760, 104857600, 268435456, 536870912, 805306368, 1073741824, 2147483648, 5368709120, 10737418240, 107374182400 # Custom annotations and labels for pods. podAnnotations: {} diff --git a/deploy/helm/container-cache/tests/render-consistent-hash-test.sh b/deploy/helm/container-cache/tests/render-consistent-hash-test.sh index 80d5011c0..ae07bfc80 100755 --- a/deploy/helm/container-cache/tests/render-consistent-hash-test.sh +++ b/deploy/helm/container-cache/tests/render-consistent-hash-test.sh @@ -52,4 +52,49 @@ grep -q 'set $cc_hash_key "$request_method|$uri|$arg_versionId|$http_range"' "$T grep -q 'proxy_set_header X-NVCF-CC-Relayed "1"' "$TMP/on.yaml" || fail "relay hop must emit the one-hop marker" grep -q 'ngx.req.get_headers()\["X-NVCF-CC-Relayed"\]' "$TMP/on.yaml" || fail "cc-route.lua must reject an inbound relay marker (serve locally, prevents relay loops)" +echo "7. enabled: the peer hop reuses connections" +# Both halves are required. A keepalive pool with no `Connection ""` is dead +# weight, because nginx then sends its default `Connection: close` and every +# relayed request re-handshakes TLS. The header must be repeated inside +# @cc_relay specifically: declaring any proxy_set_header in a location cancels +# inheritance of the server-level set. +[ "$(count 'keepalive [0-9]+;' "$TMP/on.yaml")" = 3 ] || fail "each cc_owner upstream needs a keepalive pool" +awk '/location @cc_relay/,/^ *}$/' "$TMP/on.yaml" | grep -q 'proxy_set_header Connection ""' \ + || fail '@cc_relay must repeat Connection "" or the keepalive pool is never used' + +echo "8. enabled: hot objects replicate locally after relayCacheMinUses" +awk '/location @cc_relay/,/^ *}$/' "$TMP/on.yaml" | grep -q 'proxy_cache_min_uses 3' \ + || fail "relay must cache locally after the configured use threshold" +awk '/location @cc_relay/,/^ *}$/' "$TMP/on.yaml" | grep -q 'proxy_cache_key \$cc_hash_key' \ + || fail "relay cache key must match the owner's identity, not the default key" + +echo "8b. relayCacheMinUses=0 restores strict single-copy relaying" +helm template t "$CHART_DIR" --set consistentHashRouting.enabled=true --set replicaCount=3 \ + --set consistentHashRouting.relayCacheMinUses=0 > "$TMP/on-nocache.yaml" 2>/dev/null +awk '/location @cc_relay/,/^ *}$/' "$TMP/on-nocache.yaml" | grep -q 'proxy_cache off' \ + || fail "relayCacheMinUses=0 must leave the relay a pure stream" +if awk '/location @cc_relay/,/^ *}$/' "$TMP/on-nocache.yaml" | grep -q 'proxy_cache_min_uses'; then + fail "relayCacheMinUses=0 must not emit proxy_cache_min_uses" +fi + +echo "9. relay cost is observable, and duration buckets outlast a whole transfer" +# Without the route label a relayed request is indistinguishable from a local +# hit, which is what made the relay's latency cost unmeasurable. +grep -q '"cache_status", "http_status", "route"' "$TMP/on.yaml" \ + || fail "request/throughput metrics must carry the route label" +grep -q 'local route = "local"' "$TMP/on.yaml" || fail "route must default to local" +grep -q 'route = "relayed"' "$TMP/on.yaml" || fail "relayed requests must be labelled" +grep -q 'route = "peer"' "$TMP/on.yaml" || fail "requests served for a peer must be labelled" +# The objects here are whole model files, so a 10s ceiling put a large share of +# traffic in +Inf and histogram_quantile then reports the bucket edge, not a +# latency. +grep -q 'proxy_cache_request_duration_seconds' "$TMP/on.yaml" || fail "duration histogram missing" +awk '/proxy_cache_request_duration_seconds/{print; exit}' "$TMP/on.yaml" | grep -q '600' \ + || fail "duration buckets must extend past a full object transfer" + +echo "9b. route label renders even with routing disabled (no undeclared-variable read)" +grep -q 'local route = "local"' "$TMP/off.yaml" || fail "route label must still render when routing is off" +[ "$(count 'ngx.var.cc_owner' "$TMP/off.yaml")" = 0 ] \ + || fail "must not read the routing variable when it is undeclared (OpenResty raises)" + echo "PASS: all consistent-hash routing render assertions hold"