Skip to content

Commit c3ca70d

Browse files
marcleblanc2ampagent
marcleblanc2
and
ampagent
committed
searcher: accept storageSize in G as well as 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 that accepts a whole number of G or Gi and fails the render on anything else, unless searcher.autoCacheSize is enabled (env vars are omitted in that case). Amp-Thread-ID: https://ampcode.com/threads/T-01a0ca05-5e6e-769c-9aec-5f4e207ddb9b Co-authored-by: Amp <amp@ampcode.com>
1 parent 9e7fa4b commit c3ca70d

6 files changed

Lines changed: 94 additions & 18 deletions

File tree

‎charts/sourcegraph/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Use `**BREAKING**:` to denote a breaking change
88

99
## Unreleased
1010

11+
- `searcher.storageSize` now accepts `G` as well as `Gi` when deriving `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB`. Previously a value like `150G` silently rendered both as `0`, which disables cache eviction and lets the cache volume fill up. Any other unit now fails the render unless `searcher.autoCacheSize` is enabled
1112
- Added a `network-policy` example, which limits Executor and Executor job pods to the frontend API
1213
- 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.
1314
- 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.

‎charts/sourcegraph/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ In addition to the documented values, all services also support the following va
348348
| searcher.serviceAccount.create | bool | `false` | Enable creation of ServiceAccount for `searcher` |
349349
| searcher.serviceAccount.name | string | `""` | Name of the ServiceAccount to be created or an existing ServiceAccount |
350350
| searcher.storageAnnotations | object | `{}` | Optional annotations to add to the `searcher` PVC |
351-
| searcher.storageSize | string | `"26Gi"` | Size of the PVC for searcher pods to store cache data |
351+
| 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 `G` or `Gi` (e.g. `150G`, `100Gi`) so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it |
352352
| searcher.storageSubPath | string | `""` | Optional subPath for the `searcher` primary data volume mount |
353353
| sgTestConnection | object | `{"enabled":true}` | Enable the busybox connection test after deployment |
354354
| 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) |

‎charts/sourcegraph/templates/_helpers.tpl‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,20 @@ app.kubernetes.io/name: jaeger
187187
{{- end }}
188188
{{- end }}
189189

190+
{{/*
191+
~45% of searcher.storageSize in megabytes, for SEARCHER_CACHE_SIZE_MB and
192+
SYMBOLS_CACHE_SIZE_MB. Accepts a whole number of G or Gi (e.g. 150G, 100Gi).
193+
Any other unit fails the render: a non-numeric value would otherwise cast to 0,
194+
which searcher treats as "never evict" and the cache volume fills up.
195+
*/}}
196+
{{- define "sourcegraph.searcher.cacheSizeMB" -}}
197+
{{- $size := .Values.searcher.storageSize | default "26Gi" | toString }}
198+
{{- if not (regexMatch "^[0-9]+Gi?$" $size) }}
199+
{{- fail (printf "searcher.storageSize must be a whole number of G or Gi (got %q), or set searcher.autoCacheSize=true" $size) }}
200+
{{- end }}
201+
{{- $size | trimSuffix "Gi" | trimSuffix "G" | mul 450 }}
202+
{{- end }}
203+
190204
{{- define "sourcegraph.databaseAuth" -}}
191205
{{- $top := index . 0 -}}
192206
{{- $service := index . 1 -}}

‎charts/sourcegraph/templates/searcher/searcher.StatefulSet.yaml‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,11 @@ spec:
6262
# sizes its cache to ~45% of the live cache volume, tracking PVC
6363
# expansion automatically. SEARCHER_CACHE_SIZE_MB / SYMBOLS_CACHE_SIZE_MB
6464
# are deprecated in favor of this auto-detection.
65+
# Both caches share the PVC below and each gets ~45% of it.
6566
- name: SEARCHER_CACHE_SIZE_MB
66-
# Set the cache size to ~45% of the PVC size
67-
{{- if .Values.searcher.storageSize }}
68-
value: {{ trimSuffix "Gi" .Values.searcher.storageSize | mul 450 | quote }}
69-
{{- else }}
70-
# This value is ~45% of the default value for
71-
# storageSize in the VolumeClaimTemplate below
72-
value: "11700"
73-
{{- end }}
67+
value: {{ include "sourcegraph.searcher.cacheSizeMB" . | quote }}
7468
- name: SYMBOLS_CACHE_SIZE_MB
75-
# Set the cache size to ~45% of the PVC size
76-
{{- if .Values.searcher.storageSize }}
77-
value: {{ trimSuffix "Gi" .Values.searcher.storageSize | mul 450 | quote }}
78-
{{- else }}
79-
# This value is ~45% of the default value for
80-
# storageSize in the VolumeClaimTemplate below
81-
value: "11700"
82-
{{- end }}
69+
value: {{ include "sourcegraph.searcher.cacheSizeMB" . | quote }}
8370
{{- end }}
8471
- name: POD_NAME
8572
valueFrom:

‎charts/sourcegraph/tests/searcherCacheSize_test.yaml‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
suite: searcher cache size
2+
templates:
3+
- searcher/searcher.StatefulSet.yaml
4+
tests:
5+
- it: should size both caches to ~45% of the default storageSize
6+
asserts:
7+
- contains:
8+
path: spec.template.spec.containers[0].env
9+
content:
10+
name: SEARCHER_CACHE_SIZE_MB
11+
value: "11700"
12+
- contains:
13+
path: spec.template.spec.containers[0].env
14+
content:
15+
name: SYMBOLS_CACHE_SIZE_MB
16+
value: "11700"
17+
- it: should accept storageSize in Gi
18+
set:
19+
searcher:
20+
storageSize: 100Gi
21+
asserts:
22+
- contains:
23+
path: spec.template.spec.containers[0].env
24+
content:
25+
name: SEARCHER_CACHE_SIZE_MB
26+
value: "45000"
27+
- contains:
28+
path: spec.template.spec.containers[0].env
29+
content:
30+
name: SYMBOLS_CACHE_SIZE_MB
31+
value: "45000"
32+
- it: should accept storageSize in G
33+
set:
34+
searcher:
35+
storageSize: 150G
36+
asserts:
37+
- contains:
38+
path: spec.template.spec.containers[0].env
39+
content:
40+
name: SEARCHER_CACHE_SIZE_MB
41+
value: "67500"
42+
- contains:
43+
path: spec.template.spec.containers[0].env
44+
content:
45+
name: SYMBOLS_CACHE_SIZE_MB
46+
value: "67500"
47+
- equal:
48+
path: spec.volumeClaimTemplates[0].spec.resources.requests.storage
49+
value: 150G
50+
- it: should fail on units other than G or Gi instead of disabling eviction
51+
set:
52+
searcher:
53+
storageSize: 1Ti
54+
asserts:
55+
- failedTemplate:
56+
errorMessage: searcher.storageSize must be a whole number of G or Gi (got "1Ti"), or set searcher.autoCacheSize=true
57+
- it: should not need a G or Gi storageSize when autoCacheSize is enabled
58+
set:
59+
searcher:
60+
autoCacheSize: true
61+
storageSize: 1Ti
62+
asserts:
63+
- notContains:
64+
path: spec.template.spec.containers[0].env
65+
content:
66+
name: SEARCHER_CACHE_SIZE_MB
67+
any: true
68+
- notContains:
69+
path: spec.template.spec.containers[0].env
70+
content:
71+
name: SYMBOLS_CACHE_SIZE_MB
72+
any: true

‎charts/sourcegraph/values.yaml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1214,7 +1214,9 @@ searcher:
12141214
# This tracks PVC expansion automatically, whereas the hardcoded env vars are
12151215
# frozen to the initial `storageSize` and do not follow later disk growth.
12161216
autoCacheSize: false
1217-
# -- Size of the PVC for searcher pods to store cache data
1217+
# -- Size of the PVC for searcher pods to store cache data. Unless `autoCacheSize`
1218+
# is enabled, this must be a whole number of `G` or `Gi` (e.g. `150G`, `100Gi`)
1219+
# so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it
12181220
storageSize: 26Gi
12191221
# -- Optional subPath for the `searcher` primary data volume mount
12201222
storageSubPath: ""

0 commit comments

Comments
 (0)