storage: three backup jobs that delete nothing, two of which never ran - #1114
Open
mdheller wants to merge 1 commit into
Open
storage: three backup jobs that delete nothing, two of which never ran#1114mdheller wants to merge 1 commit into
mdheller wants to merge 1 commit into
Conversation
None of the estate's PVC definitions had a retention policy. Three
concrete cases, all of which look healthy and none of which bound
anything.
workspace-mail-backup
A full tarball of /maildata every night into a fixed 20Gi volume, with
no deletion of any kind. It is not already full only because the image
it referenced never existed, so the job has never run — the retention
bug is latent, not fixed. Fixed BEFORE it is allowed to schedule.
Now: prune by count (7) and by total size (14GB), refuse rather than
fill (MIN_FREE_MB, exit 1 -> KubeJobFailed, which is a live alert),
verify the archive is readable before declaring success, and emit an
EvidenceReceipt whose hash is the archive digest.
Proved against seeded fixtures:
9 existing -> pruned 3 -> wrote 1 -> 7 retained
refusal -> status="denied" receipt, nothing written, exit 1
budget -> prunes until under MAX_TOTAL_MB
mesh-qdrant snapshots
Three independent defects meant this CronJob has never produced a
single snapshot, silently:
- `args:` against curlimages/curl, whose ENTRYPOINT is curl, so the
container ran `curl /bin/sh -c "<script>"`. The shell never ran.
- hostname `qdrant`, but the Service is `mesh-qdrant`.
- `jq` is not in that image.
It also only ever POSTed, never DELETEd, so snapshots would have grown
without bound had it worked. And qdrant's default snapshots_path is
/qdrant/snapshots, which is NOT on the PVC — snapshots would have gone
to the pod's ephemeral layer and vanished on restart.
All four fixed; QDRANT__STORAGE__SNAPSHOTS_PATH now points onto the
volume so growth is at least visible to the capacity guard.
Proved against a mock qdrant API: 9 snapshots + 1 new -> pruned 3 ->
7 retained; unreachable qdrant -> status="denied" receipt, exit 1.
That run caught a real bug — the name extractor was anchored to compact
JSON and silently returned zero collections against a spaced response,
reporting a clean no-op backup. Now whitespace-tolerant.
clickhouse cell analytics
All 7 tables are monthly-partitioned with no TTL anywhere, so a TTL
DELETE drops whole parts cheaply. Added: 12 months for scores and
notification metrics, 24 for the reputation/quality/feedback facts.
Verified by applying the DDL to a scratch database on the live server —
all 7 report HAS TTL — then dropping it.
mesh-xl-cache
Not abandoned. `Used By: <none>` is the designed steady state: it caches
model weights for the scale-to-zero mesh-vllm-xl seat so a warm-up reads
cache (~2-3 min) instead of re-downloading (~15 min). Annotated on the
object so a storage audit does not have to re-derive that from the
manifest. Not deleted.
Two findings that are NOT fixed here, deliberately, and are reported for
Wave 0:
- infra/datastores/clickhouse/cell/**.sql is never applied. The runner
at tools/eval_fabric_migrate.py globs "*.sql" non-recursively, so it
never descends into cell/. Confirmed empirically: none of the 7 tables
exists in the live server, nor do the eval_fabric ones. The TTL added
here is correct but inert until that is fixed. tools/ is a live lane,
so it is not touched.
- The live clickhouse-data volume is held by hellgraph.events, a
ReplacingMergeTree with NO PARTITION BY and no TTL. That is the actual
retention exposure on that PVC, and it is a different table from the
ones named in the brief. 686M/20G today, so not urgent.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR hardens Kubernetes storage-related backups and retention by adding explicit pruning/guardrails to backup CronJobs, making qdrant snapshots durable and correctly executed, adding ClickHouse TTL-based retention, and documenting an intentionally unmounted model-cache PVC to avoid mistaken reclamation.
Changes:
- Add retention-by-count/size, free-space refusal, verification, and JSON receipts to the workspace-mail backup CronJob.
- Fix mesh-qdrant snapshot CronJob execution (entrypoint/hostname/tools), add per-collection pruning, and configure qdrant snapshots to live on the PVC.
- Add TTL DELETE policies to ClickHouse “cell analytics” tables; annotate mesh-xl-cache PVC with rationale and cost.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/k8s/workspace-mail/base/backup-cronjob.yaml | Adds pruning, disk-space preflight refusal, archive verification, and receipt emission for mail backups. |
| infra/k8s/mesh-qdrant/base/statefulset.yaml | Forces qdrant snapshots_path onto the PVC so snapshots are durable and capacity-visible. |
| infra/k8s/mesh-qdrant/base/backup-cronjob.yaml | Fixes the CronJob shell execution, targets correct Service, removes jq dependency, adds retention deletes + receipts. |
| infra/datastores/clickhouse/cell/0001_personal_intelligence_cell_analytics.sql | Adds table TTL retention windows (12/24 months) to control storage growth. |
| deploy/serving/mesh-xl-a100.yaml | Documents why an apparently “unused” PVC is intentionally unmounted (scale-to-zero cache). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+105
to
+112
| FREE=$(df -Pm /backup | awk 'NR==2 {print $4}') | ||
| log "preflight: ${FREE}MB free, need >= ${MIN_FREE_MB}MB" | ||
| if [ "${FREE}" -lt "${MIN_FREE_MB}" ]; then | ||
| log "REFUSED: only ${FREE}MB free on /backup, below MIN_FREE_MB=${MIN_FREE_MB}" | ||
| printf '{"version":"0.1","receipt_id":"evr-mail-backup-refused-%s","created_at":"%s","service_ref":"infra/k8s/workspace-mail","action":"mail-backup","status":"denied","subject_ref":"pvc/workspace-mail-backup","hash":"","hash_algo":"sha256","metrics":{"free_mb":%s,"min_free_mb":%s,"retained":%s}}\n' \ | ||
| "${STAMP}" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "${FREE}" "${MIN_FREE_MB}" "${COUNT}" | ||
| exit 1 | ||
| fi |
Comment on lines
+58
to
+61
| names() { grep -oE '"name"[[:space:]]*:[[:space:]]*"[^"]*"' | cut -d'"' -f4; } | ||
|
|
||
| COLLECTIONS=$(curl -sf --max-time 30 "${QDRANT_URL}/collections" | names || true) | ||
| if [ -z "${COLLECTIONS}" ]; then |
Comment on lines
+65
to
+72
| if ! curl -sf --max-time 10 "${QDRANT_URL}/readyz" >/dev/null 2>&1; then | ||
| log "REFUSED: qdrant unreachable at ${QDRANT_URL}" | ||
| printf '{"version":"0.1","receipt_id":"evr-qdrant-snapshot-unreachable-%s","created_at":"%s","service_ref":"infra/k8s/mesh-qdrant","action":"qdrant-snapshot","status":"denied","subject_ref":"pvc/qdrant-storage","hash":"","hash_algo":"sha256","metrics":{"collections":0}}\n' \ | ||
| "$(date -u +%Y%m%dT%H%M%SZ)" "${NOW}" | ||
| exit 1 | ||
| fi | ||
| log "qdrant reachable but reports zero collections — nothing to snapshot" | ||
| fi |
Comment on lines
+89
to
+91
| SNAPS=$(curl -sf --max-time 30 "${QDRANT_URL}/collections/${col}/snapshots" | names || true) | ||
| TOTAL=$(printf '%s\n' "${SNAPS}" | grep -c . || true) | ||
| if [ "${TOTAL}" -gt "${RETAIN}" ]; then |
| fi | ||
|
|
||
| # --- 4. write ---------------------------------------------- | ||
| if ! tar czf "${OUT}" /maildata; then |
Member
Author
|
Merge-gate disposition (Copilot 3-channel + adversarial pass) Rejected / already-mitigated — qdrant Acknowledged, non-blocking: refusal-receipt Note: on its own, #1114's mail-backup still green-passes an empty maildir archive — that gap is closed by the stacked #1120 (size gate). Merge them in order. Verdict: MERGE-READY (stack base; land #1120 immediately after). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
None of the estate's PVC definitions had a retention policy. Three concrete cases — all of which look healthy, none of which bound anything.
workspace-mail-backupA full tarball of
/maildataevery night into a fixed 20Gi volume, deleting nothing.successfulJobsHistoryLimit: 3prunes Job objects, not tarballs.It is not already full only because the image it referenced never existed, so the job has never run. The retention bug is latent, not fixed — which is why it is fixed here, before it is allowed to schedule.
Now: prune by count (7) and by total size (14GB); refuse rather than fill (
MIN_FREE_MB, exit 1 → Job failure →KubeJobFailed, a live alert); verify the archive is readable before declaring success; emit an EvidenceReceipt whosehashis the archive digest.Order matters — prune, preflight, write, verify, receipt. Writing first would need headroom for N+1 backups, which is exactly the margin that does not exist.
Proved against seeded fixtures:
mesh-qdrantsnapshotsThree independent defects mean this CronJob has never produced a single snapshot, silently:
args:againstcurlimages/curl, whoseENTRYPOINTiscurl— the container rancurl /bin/sh -c "<script>". The shell never executed.qdrant, but the Service ismesh-qdrant.jqis not in that image.It also only ever
POSTed, neverDELETEd, so snapshots would have grown without bound had it worked. And qdrant's defaultsnapshots_pathis/qdrant/snapshots, not on the PVC — snapshots would have landed on the pod's ephemeral layer and vanished on every restart. So the premise "writes snapshots onto the volume it snapshots" was generous: they went nowhere durable, with no pruning either way.All four fixed, plus
QDRANT__STORAGE__SNAPSHOTS_PATHonto the volume so growth is at least visible to the capacity guard.Proved against a mock qdrant API (the StatefulSet is in desired state but has not materialised in-cluster, so this could not be exercised live):
That run caught a real bug: the name extractor was anchored to compact JSON and silently returned zero collections against a spaced response — reporting a clean no-op backup. Now whitespace-tolerant.
clickhousecell analyticsAll 7 tables are monthly-partitioned with no
TTLanywhere, so aTTL … DELETEdrops whole parts cheaply. Added: 12 months for signal scores and notification metrics, 24 for the reputation / source-quality / feedback facts.Proved by applying the DDL to a scratch database on the live server — all 7 report
HAS TTL— then dropping it.mesh-xl-cache— not abandonedUsed By: <none>is the designed steady state.deploy/serving/mesh-xl-a100.yamldocuments it:mesh-vllm-xlruns atreplicas: 0for $0 idle, and this 120Gi volume caches model weights so a warm-up reads cache (~2–3 min) instead of re-downloading (~15 min). Annotated on the object so a storage audit does not have to re-derive that from the manifest. Not deleted. Idle cost ~USD 13/mo.Deliberately NOT fixed here — for Wave 0
The
cell/DDL is never applied.tools/eval_fabric_migrate.pyglobs"*.sql"non-recursively, so it never descends intocell/. Confirmed empirically — none of the 7 tables exists in the live server, nor do theeval_fabricones; only twohellgraphtables created by something else. The TTL added here is correct but inert until that is fixed.tools/is a live lane, so it is untouched.The live
clickhouse-dataexposure is a different table. It is held byhellgraph.events— aReplacingMergeTreewith noPARTITION BYand no TTL, so a TTL would have to rewrite whole parts. 686M/20G today, so not urgent, but it is the actual retention risk on that PVC.Coordination
infra/k8s/workspace-mail/**,infra/k8s/mesh-qdrant/**,infra/datastores/clickhouse/cell/**,deploy/serving/mesh-xl-a100.yaml. No overlap with #1091, #1105, #1112, #1113,images.yml,infra/k8s/zot/**,apps/health-twin/**, ortools/.kubectl kustomizeon both overlays, and the fixture/mock runs above.