From e20331197c5e13579e3740d4e626b69784a73ec1 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:18:05 -0600 Subject: [PATCH] searcher: accept storageSize in M, Mi, G and Gi for cache sizing SEARCHER_CACHE_SIZE_MB and SYMBOLS_CACHE_SIZE_MB were derived with trimSuffix "Gi" | mul 450, so a storageSize like 150G left a non-numeric string that sprig cast to 0. searcher treats 0 as "never evict", so both caches grew unbounded until the PVC filled and the pod failed to start with "failed to setup TMPDIR: ... no space left on device". Move the math into a sourcegraph.searcher.cacheSizeMB helper in its own file under templates/_helpers/. It accepts a whole number of M, Mi, G or Gi and fails the render on anything else, unless searcher.autoCacheSize is enabled (the env vars are omitted in that case). Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a146da-9505-715c-949c-1146cc4e7e33 --- charts/sourcegraph/CHANGELOG.md | 1 + charts/sourcegraph/README.md | 2 +- .../templates/_helpers/_searcherCacheSize.tpl | 20 ++++ .../searcher/searcher.StatefulSet.yaml | 19 +-- .../tests/searcherCacheSize_test.yaml | 109 ++++++++++++++++++ charts/sourcegraph/values.yaml | 4 +- 6 files changed, 137 insertions(+), 18 deletions(-) create mode 100644 charts/sourcegraph/templates/_helpers/_searcherCacheSize.tpl create mode 100644 charts/sourcegraph/tests/searcherCacheSize_test.yaml diff --git a/charts/sourcegraph/CHANGELOG.md b/charts/sourcegraph/CHANGELOG.md index e0186812..c5413322 100644 --- a/charts/sourcegraph/CHANGELOG.md +++ b/charts/sourcegraph/CHANGELOG.md @@ -10,6 +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. - 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 b55aa5e2..91f6444a 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 | +| 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.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/_searcherCacheSize.tpl b/charts/sourcegraph/templates/_helpers/_searcherCacheSize.tpl new file mode 100644 index 00000000..f86f9672 --- /dev/null +++ b/charts/sourcegraph/templates/_helpers/_searcherCacheSize.tpl @@ -0,0 +1,20 @@ +{{/* +~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. +*/}} +{{- 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 }} diff --git a/charts/sourcegraph/templates/searcher/searcher.StatefulSet.yaml b/charts/sourcegraph/templates/searcher/searcher.StatefulSet.yaml index c18e71ee..76c71d47 100644 --- a/charts/sourcegraph/templates/searcher/searcher.StatefulSet.yaml +++ b/charts/sourcegraph/templates/searcher/searcher.StatefulSet.yaml @@ -62,24 +62,11 @@ spec: # sizes its cache to ~45% of the live cache volume, tracking PVC # expansion automatically. SEARCHER_CACHE_SIZE_MB / SYMBOLS_CACHE_SIZE_MB # are deprecated in favor of this auto-detection. + # Both caches share the PVC below and each gets ~45% of it. - name: SEARCHER_CACHE_SIZE_MB - # Set the cache size to ~45% of the PVC size - {{- if .Values.searcher.storageSize }} - value: {{ trimSuffix "Gi" .Values.searcher.storageSize | mul 450 | quote }} - {{- else }} - # This value is ~45% of the default value for - # storageSize in the VolumeClaimTemplate below - value: "11700" - {{- end }} + value: {{ include "sourcegraph.searcher.cacheSizeMB" . | quote }} - name: SYMBOLS_CACHE_SIZE_MB - # Set the cache size to ~45% of the PVC size - {{- if .Values.searcher.storageSize }} - value: {{ trimSuffix "Gi" .Values.searcher.storageSize | mul 450 | quote }} - {{- else }} - # This value is ~45% of the default value for - # storageSize in the VolumeClaimTemplate below - value: "11700" - {{- end }} + value: {{ include "sourcegraph.searcher.cacheSizeMB" . | quote }} {{- end }} - name: POD_NAME valueFrom: diff --git a/charts/sourcegraph/tests/searcherCacheSize_test.yaml b/charts/sourcegraph/tests/searcherCacheSize_test.yaml new file mode 100644 index 00000000..e46b7783 --- /dev/null +++ b/charts/sourcegraph/tests/searcherCacheSize_test.yaml @@ -0,0 +1,109 @@ +suite: searcher cache size +templates: +- searcher/searcher.StatefulSet.yaml +tests: +- 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" + - contains: + path: spec.template.spec.containers[0].env + content: + name: SYMBOLS_CACHE_SIZE_MB + value: "11700" +- it: should accept storageSize in Gi + set: + searcher: + storageSize: 100Gi + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: SEARCHER_CACHE_SIZE_MB + value: "45000" + - contains: + path: spec.template.spec.containers[0].env + content: + name: SYMBOLS_CACHE_SIZE_MB + value: "45000" +- it: should accept storageSize in G + set: + searcher: + storageSize: 150G + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: SEARCHER_CACHE_SIZE_MB + value: "67500" + - contains: + path: spec.template.spec.containers[0].env + content: + name: SYMBOLS_CACHE_SIZE_MB + value: "67500" + - equal: + path: spec.volumeClaimTemplates[0].spec.resources.requests.storage + value: 150G +- it: should accept storageSize in M + set: + searcher: + storageSize: 500000M + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: SEARCHER_CACHE_SIZE_MB + value: "225000" + - 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 + set: + searcher: + storageSize: 25Mi + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: SEARCHER_CACHE_SIZE_MB + value: "11" + - 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 + set: + searcher: + storageSize: 1Ti + 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 + set: + searcher: + storageSize: 1.5G + 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 + set: + searcher: + autoCacheSize: true + storageSize: 1Ti + asserts: + - notContains: + path: spec.template.spec.containers[0].env + content: + name: SEARCHER_CACHE_SIZE_MB + any: true + - notContains: + path: spec.template.spec.containers[0].env + content: + name: SYMBOLS_CACHE_SIZE_MB + any: true diff --git a/charts/sourcegraph/values.yaml b/charts/sourcegraph/values.yaml index c96683f9..21cd4644 100644 --- a/charts/sourcegraph/values.yaml +++ b/charts/sourcegraph/values.yaml @@ -1282,7 +1282,9 @@ searcher: # This tracks PVC expansion automatically, whereas the hardcoded env vars are # 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 + # -- 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 storageSize: 26Gi # -- Optional subPath for the `searcher` primary data volume mount storageSubPath: ""