From ef8da9a3de9e199c69c002280cc063be75777f04 Mon Sep 17 00:00:00 2001 From: Michael Lin Date: Mon, 19 Oct 2026 17:03:23 +0000 Subject: [PATCH] searcher: derive cache size from any Kubernetes quantity via shared parser Extract the quantity parser from sourcegraph.redis.maxmemory into sourcegraph.quantityBytes and use it for searcher.storageSize too, so Ti, Ki, fractional and suffixless values work instead of failing the render. Compute 45% the same way searcher auto-sizes (floor(bytes/1e6)*45/100) rather than treating Gi as G, and fail when the result would be 0, since an explicit 0 disables eviction. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a14718-43f2-75ab-b1b7-f2ed264868db --- charts/sourcegraph/CHANGELOG.md | 2 +- charts/sourcegraph/README.md | 2 +- charts/sourcegraph/templates/_helpers.tpl | 28 +++++++--- .../templates/_helpers/_searcherCacheSize.tpl | 36 +++++++------ .../tests/searcherCacheSize_test.yaml | 53 ++++++++++++------- charts/sourcegraph/values.yaml | 4 +- 6 files changed, 78 insertions(+), 47 deletions(-) diff --git a/charts/sourcegraph/CHANGELOG.md b/charts/sourcegraph/CHANGELOG.md index c5413322..c3a3bacc 100644 --- a/charts/sourcegraph/CHANGELOG.md +++ b/charts/sourcegraph/CHANGELOG.md @@ -10,7 +10,7 @@ Use `**BREAKING**:` to denote a breaking change - Added opt-in Redis config management through `redisCache.config.enabled` and `redisStore.config.enabled` (both default to `false`). Default deployments retain their image configs, custom mounts, authentication, and memory settings. When enabled, the chart mounts `/etc/redis/redis.conf` and supports `existingConfig`, `maxmemory`, `maxmemoryRatio`, `maxmemoryPolicy`, and `additionalConfig`. See [Enabling Redis config management](README.md#enabling-redis-config-management) before opting in, especially when using custom images or Secret-mounted configs. - With Redis config management enabled, the chart sizes `maxmemory` at 75% of `resources.limits.memory` to leave room for overhead; this reduces, but does not eliminate, OOM risk. Ratios must be greater than 0 and less than 1. Auto-sizing is skipped in local development mode or when the limit is absent or unrecognised. At the default 7Gi limit, opting in lowers the effective cap from 6GiB to 5.25GiB: `redis-cache` evicts earlier, but `redis-store` uses `noeviction`, so its write-error ceiling drops too. Set `redisStore.config.maxmemory: 6gb` to keep the old ceiling if the pod has sufficient memory. -- `searcher.storageSize` now accepts a whole number of `M`, `Mi`, `G` or `Gi` (e.g. `150G`, `500000Mi`) when deriving `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB`. Previously anything other than `Gi` silently rendered both as `0`, which disables cache eviction and lets the cache volume fill up. Any other value now fails the render unless `searcher.autoCacheSize` is enabled. +- `searcher.storageSize` now accepts any Kubernetes quantity (e.g. `150G`, `1Ti`, `1.5Ti`, `500000M`) when deriving `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB`. Previously anything other than `Gi` silently rendered both as `0`, which disables cache eviction and lets the cache volume fill up. Unparseable values, or values below `3M`, now fail the render unless `searcher.autoCacheSize` is enabled. The derived size now matches searcher's own auto-sizing arithmetic (45% of the volume in decimal megabytes), so existing `Gi` deployments see a slightly higher value on upgrade, e.g. the default `26Gi` renders `12562` instead of `11700`. - Added a `network-policy` example, which limits Executor and Executor job pods to the frontend API - Corrected the external object storage examples to configure the shared store for frontend, worker, precise code intel, syntactic code intel, gitserver, and searcher, including credentials or workload service accounts as required. - Removed the unused application ports from the precise and syntactic code intel worker Deployments and Services; health checks and Prometheus metrics continue to use the debug server on port 6060. diff --git a/charts/sourcegraph/README.md b/charts/sourcegraph/README.md index 91f6444a..8c19366c 100644 --- a/charts/sourcegraph/README.md +++ b/charts/sourcegraph/README.md @@ -419,7 +419,7 @@ In addition to the documented values, all services also support the following va | searcher.serviceAccount.create | bool | `false` | Enable creation of ServiceAccount for `searcher` | | searcher.serviceAccount.name | string | `""` | Name of the ServiceAccount to be created or an existing ServiceAccount | | searcher.storageAnnotations | object | `{}` | Optional annotations to add to the `searcher` PVC | -| searcher.storageSize | string | `"26Gi"` | Size of the PVC for searcher pods to store cache data. Unless `autoCacheSize` is enabled, this must be a whole number of `M`, `Mi`, `G` or `Gi` (e.g. `150G`, `100Gi`) so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it | +| searcher.storageSize | string | `"26Gi"` | Size of the PVC for searcher pods to store cache data. Unless `autoCacheSize` is enabled, this must be a Kubernetes quantity (e.g. `150G`, `100Gi`, `1Ti`) of at least `3M`, so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it | | searcher.storageSubPath | string | `""` | Optional subPath for the `searcher` primary data volume mount | | sgTestConnection | object | `{"enabled":true}` | Enable the busybox connection test after deployment | | sourcegraph.affinity | object | `{}` | Global Affinity, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) | diff --git a/charts/sourcegraph/templates/_helpers.tpl b/charts/sourcegraph/templates/_helpers.tpl index fa1af3b4..cf4ca4af 100644 --- a/charts/sourcegraph/templates/_helpers.tpl +++ b/charts/sourcegraph/templates/_helpers.tpl @@ -313,6 +313,24 @@ checksum/auth: {{ toJson $checksum | sha256sum }} checksum/redis: {{ toJson $checksum | sha256sum }} {{- end -}} +{{/* +Convert a Kubernetes resource quantity (e.g. 7Gi, 1.5G, 500M, 1Ti, 6442450944) +to a whole number of bytes, rounded down. Renders an empty string when the +value is not a recognised quantity, so callers can decide whether to fall back +or fail. +Usage: include "sourcegraph.quantityBytes" .Values.searcher.storageSize +*/}} +{{- define "sourcegraph.quantityBytes" -}} +{{- $quantity := . | toString -}} +{{- $number := regexReplaceAll "^([0-9]+(\\.[0-9]+)?).*$" $quantity "${1}" -}} +{{- $suffix := regexReplaceAll "^[0-9]+(\\.[0-9]+)?" $quantity "" -}} +{{- /* Kubernetes quantity suffixes: binary (1024^n) and decimal (1000^n). */ -}} +{{- $units := dict "" 1.0 "k" 1e3 "M" 1e6 "G" 1e9 "T" 1e12 "P" 1e15 "E" 1e18 "Ki" 1024.0 "Mi" 1048576.0 "Gi" 1073741824.0 "Ti" 1099511627776.0 "Pi" 1125899906842624.0 "Ei" 1152921504606846976.0 -}} +{{- if and (regexMatch "^[0-9]+(\\.[0-9]+)?$" $number) (hasKey $units $suffix) -}} +{{- printf "%d" (int64 (floor (mulf (float64 $number) (index $units $suffix)))) -}} +{{- end -}} +{{- end -}} + {{/* Resolve the redis `maxmemory` directive for a service. Usage: include "sourcegraph.redis.maxmemory" (list . "redisCache") @@ -333,12 +351,8 @@ Resolution order: {{- if $config.maxmemory -}} {{- $config.maxmemory -}} {{- else if not $top.Values.sourcegraph.localDevMode -}} -{{- $limit := dig "resources" "limits" "memory" "" $values | toString -}} -{{- $number := regexReplaceAll "^([0-9]+(\\.[0-9]+)?).*$" $limit "${1}" -}} -{{- $suffix := regexReplaceAll "^[0-9]+(\\.[0-9]+)?" $limit "" -}} -{{- /* Kubernetes quantity suffixes: binary (1024^n) and decimal (1000^n). */ -}} -{{- $units := dict "" 1.0 "k" 1e3 "M" 1e6 "G" 1e9 "T" 1e12 "P" 1e15 "E" 1e18 "Ki" 1024.0 "Mi" 1048576.0 "Gi" 1073741824.0 "Ti" 1099511627776.0 "Pi" 1125899906842624.0 "Ei" 1152921504606846976.0 -}} -{{- if and (regexMatch "^[0-9]+(\\.[0-9]+)?$" $number) (hasKey $units $suffix) -}} +{{- $limit := include "sourcegraph.quantityBytes" (dig "resources" "limits" "memory" "" $values) -}} +{{- if $limit -}} {{- $ratio := 0.75 -}} {{- if hasKey $config "maxmemoryRatio" -}} {{- $ratio = float64 $config.maxmemoryRatio -}} @@ -346,7 +360,7 @@ Resolution order: {{- if not (and (gt $ratio 0.0) (lt $ratio 1.0)) -}} {{- fail (printf "%s.config.maxmemoryRatio must be greater than 0 and less than 1" $service) -}} {{- end -}} -{{- $bytes := floor (mulf (float64 $number) (index $units $suffix) $ratio) -}} +{{- $bytes := floor (mulf (float64 $limit) $ratio) -}} {{- if gt $bytes 0.0 -}} {{- printf "%d" (int64 $bytes) -}} {{- end -}} diff --git a/charts/sourcegraph/templates/_helpers/_searcherCacheSize.tpl b/charts/sourcegraph/templates/_helpers/_searcherCacheSize.tpl index f86f9672..f8dae0d7 100644 --- a/charts/sourcegraph/templates/_helpers/_searcherCacheSize.tpl +++ b/charts/sourcegraph/templates/_helpers/_searcherCacheSize.tpl @@ -1,20 +1,22 @@ {{/* -~45% of searcher.storageSize in megabytes, for SEARCHER_CACHE_SIZE_MB and -SYMBOLS_CACHE_SIZE_MB. Accepts a whole number of M, Mi, G or Gi (e.g. 150G, -100Gi, 500000M). Any other value fails the render: a non-numeric value would -otherwise cast to 0, which searcher treats as "never evict" and the cache -volume fills up. Binary units are treated as decimal, matching the original -"Gi * 450" math, so the result is slightly under 45% of the volume. +45% of searcher.storageSize in whole (decimal) megabytes, for +SEARCHER_CACHE_SIZE_MB and SYMBOLS_CACHE_SIZE_MB. Mirrors the arithmetic +searcher uses when it auto-sizes from the volume: floor(bytes / 1e6) * 45 / 100. +Accepts any Kubernetes quantity (e.g. 150G, 100Gi, 1.5Ti, 500000M). + +Anything unparseable, or anything so small the result would be 0, fails the +render: searcher treats an explicit 0 as "never evict" (unlike an unset +variable, which auto-sizes), so the cache volume would fill up. */}} {{- define "sourcegraph.searcher.cacheSizeMB" -}} -{{- $size := .Values.searcher.storageSize | default "26Gi" | toString }} -{{- if not (regexMatch "^[0-9]+(M|Mi|G|Gi)$" $size) }} -{{- fail (printf "searcher.storageSize must be a whole number of M, Mi, G or Gi (got %q), or set searcher.autoCacheSize=true" $size) }} -{{- end }} -{{- $decimal := trimSuffix "i" $size }} -{{- $megabytes := $decimal | trimSuffix "G" | trimSuffix "M" | int64 }} -{{- if hasSuffix "G" $decimal }} -{{- $megabytes = mul $megabytes 1000 }} -{{- end }} -{{- div (mul $megabytes 45) 100 }} -{{- end }} +{{- $size := .Values.searcher.storageSize | default "26Gi" | toString -}} +{{- $bytes := include "sourcegraph.quantityBytes" $size -}} +{{- if not $bytes -}} +{{- fail (printf "searcher.storageSize must be a Kubernetes quantity such as 150G, 100Gi or 1Ti (got %q), or set searcher.autoCacheSize=true" $size) -}} +{{- end -}} +{{- $megabytes := div (mul (div (int64 $bytes) 1000000) 45) 100 -}} +{{- if le $megabytes 0 -}} +{{- fail (printf "searcher.storageSize %s is too small to derive a non-zero cache size; use at least 3M, or set searcher.autoCacheSize=true" $size) -}} +{{- end -}} +{{- $megabytes -}} +{{- end -}} diff --git a/charts/sourcegraph/tests/searcherCacheSize_test.yaml b/charts/sourcegraph/tests/searcherCacheSize_test.yaml index e46b7783..08e3dbe5 100644 --- a/charts/sourcegraph/tests/searcherCacheSize_test.yaml +++ b/charts/sourcegraph/tests/searcherCacheSize_test.yaml @@ -2,18 +2,18 @@ suite: searcher cache size templates: - searcher/searcher.StatefulSet.yaml tests: -- it: should size both caches to ~45% of the default storageSize +- it: should size both caches to 45% of the default storageSize asserts: - contains: path: spec.template.spec.containers[0].env content: name: SEARCHER_CACHE_SIZE_MB - value: "11700" + value: "12562" - contains: path: spec.template.spec.containers[0].env content: name: SYMBOLS_CACHE_SIZE_MB - value: "11700" + value: "12562" - it: should accept storageSize in Gi set: searcher: @@ -23,12 +23,12 @@ tests: path: spec.template.spec.containers[0].env content: name: SEARCHER_CACHE_SIZE_MB - value: "45000" + value: "48318" - contains: path: spec.template.spec.containers[0].env content: name: SYMBOLS_CACHE_SIZE_MB - value: "45000" + value: "48318" - it: should accept storageSize in G set: searcher: @@ -57,45 +57,60 @@ tests: content: name: SEARCHER_CACHE_SIZE_MB value: "225000" +- it: should accept storageSize in Ti + set: + searcher: + storageSize: 1Ti + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: SEARCHER_CACHE_SIZE_MB + value: "494779" - contains: path: spec.template.spec.containers[0].env content: name: SYMBOLS_CACHE_SIZE_MB - value: "225000" -- it: should accept storageSize in Mi and round down to whole megabytes + value: "494779" +- it: should accept a fractional storageSize set: searcher: - storageSize: 25Mi + storageSize: 1.5G asserts: - contains: path: spec.template.spec.containers[0].env content: name: SEARCHER_CACHE_SIZE_MB - value: "11" + value: "675" +- it: should accept a suffixless byte count and round down to whole megabytes + set: + searcher: + storageSize: "6442450944" + asserts: - contains: path: spec.template.spec.containers[0].env content: - name: SYMBOLS_CACHE_SIZE_MB - value: "11" -- it: should fail on units other than M, Mi, G or Gi instead of disabling eviction + name: SEARCHER_CACHE_SIZE_MB + value: "2898" +- it: should fail on an unrecognised quantity instead of disabling eviction set: searcher: - storageSize: 1Ti + storageSize: 1Tb asserts: - failedTemplate: - errorMessage: searcher.storageSize must be a whole number of M, Mi, G or Gi (got "1Ti"), or set searcher.autoCacheSize=true -- it: should fail on fractional storageSize instead of disabling eviction + errorMessage: searcher.storageSize must be a Kubernetes quantity such as 150G, 100Gi or 1Ti (got "1Tb"), or set searcher.autoCacheSize=true +- it: should fail when the derived cache size would be 0 instead of disabling eviction set: searcher: - storageSize: 1.5G + storageSize: 2M asserts: - failedTemplate: - errorMessage: searcher.storageSize must be a whole number of M, Mi, G or Gi (got "1.5G"), or set searcher.autoCacheSize=true -- it: should not need a recognised storageSize unit when autoCacheSize is enabled + errorMessage: searcher.storageSize 2M is too small to derive a non-zero cache size; use at least 3M, or set searcher.autoCacheSize=true +- it: should not need a parseable storageSize when autoCacheSize is enabled set: searcher: autoCacheSize: true - storageSize: 1Ti + storageSize: 1Tb asserts: - notContains: path: spec.template.spec.containers[0].env diff --git a/charts/sourcegraph/values.yaml b/charts/sourcegraph/values.yaml index 21cd4644..74d4b7a3 100644 --- a/charts/sourcegraph/values.yaml +++ b/charts/sourcegraph/values.yaml @@ -1283,8 +1283,8 @@ searcher: # frozen to the initial `storageSize` and do not follow later disk growth. autoCacheSize: false # -- Size of the PVC for searcher pods to store cache data. Unless `autoCacheSize` - # is enabled, this must be a whole number of `M`, `Mi`, `G` or `Gi` (e.g. `150G`, `100Gi`) - # so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it + # is enabled, this must be a Kubernetes quantity (e.g. `150G`, `100Gi`, `1Ti`) of at + # least `3M`, so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it storageSize: 26Gi # -- Optional subPath for the `searcher` primary data volume mount storageSubPath: ""