Skip to content

fix(helm): reduce Hindsight DB pressure and align probes#17

Closed
allyblockcast[bot] wants to merge 3 commits into
mainfrom
fix/BLO-13560-hindsight-db-pressure-liveness
Closed

fix(helm): reduce Hindsight DB pressure and align probes#17
allyblockcast[bot] wants to merge 3 commits into
mainfrom
fix/BLO-13560-hindsight-db-pressure-liveness

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Jul 8, 2026

Copy link
Copy Markdown

Summary

  • point API liveness/readiness at /version instead of /health
  • point worker liveness/readiness at /metrics instead of /health
  • set worker DB-pressure caps for batch size 1, max slots 2, consolidation slot 1, and retain slot/concurrency 1

Verification

  • helm lint helm/hindsight
  • helm template hindsight helm/hindsight --set worker.enabled=true --set worker.replicaCount=4 --set api.readinessProbe.httpGet.path=/version --set worker.readinessProbe.httpGet.path=/metrics --set worker.env.HINDSIGHT_API_WORKER_RETAIN_MAX_SLOTS=1

Live rollout note

  • Live patch attempt for BLO-14004 was blocked by Kubernetes RBAC: system:serviceaccount:paperclip:paperclip-k8s-mcp-ns-rw cannot patch apps/deployments in namespace hindsight. A scoped infra permission/unblocker is needed before rollout/soak can complete.

@allyblockcast

allyblockcast Bot commented Jul 8, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-13560
🔗 Paperclip issue: BLO-14004

1 similar comment
@allyblockcast

allyblockcast Bot commented Jul 8, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-13560
🔗 Paperclip issue: BLO-14004

@allyblockcast

allyblockcast Bot commented Jul 8, 2026

Copy link
Copy Markdown
Author

@ally Please review the Helm remediation for BLO-14004/BLO-13567. Focus on whether the API /version probes, worker /metrics probes, and worker retain/consolidation slot defaults match the intended live DB-pressure constraints without breaking the chart's StatefulSet worker identity contract.

@allyblockcast

allyblockcast Bot commented Jul 8, 2026

Copy link
Copy Markdown
Author

Closing in favor of rebased replacement PR #18 to avoid force-pushing the conflicting branch.

@allyblockcast allyblockcast Bot closed this Jul 8, 2026
@allyblockcast

allyblockcast Bot commented Jul 8, 2026

Copy link
Copy Markdown
Author

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.

reviewed head: 59d01a7

Critical Issues (1)

  • [native-codex] helm/hindsight/values.yaml:167-171HINDSIGHT_API_WORKER_MAX_SLOTS=2 with CONSOLIDATION_MAX_SLOTS=1 + RETAIN_MAX_SLOTS=1 sums to exactly 2, which zeroes the worker's shared task pool (shared_pool_size = max(0, max_slots - sum(reservations)) in hindsight_api/worker/poller.py). file_convert_retain, refresh_mental_model, graph_maintenance, and import_documents all have a default reservation of 0 in hindsight_api/config.py's WORKER_SLOT_RESERVATION_TYPES and can only draw from that shared pool — reserved slots are explicitly "cannot be used by other types" per the poller's own docstring. With shared_pool_size == 0, every task of those four operation types will never be claimed by any worker running this config: not slowed down, permanently starved. Under the old defaults (max_slots=10, consolidation=2, everything else 0), those types shared an 8-slot pool and worked fine.
    • Confirm whether graph maintenance / mental-model refresh / document import / file-convert-retain are actually unused in this deployment. If any of them are live, this change silently breaks them the moment it rolls out — it won't show up as an error, just as work that never completes. If they are genuinely unused, consider reserving 0 total (letting retain+consolidation share the 2 slots via the shared pool, e.g. drop the explicit RETAIN_MAX_SLOTS) or bump WORKER_MAX_SLOTS by 1 so a shared slot survives, rather than reserving 100% of capacity.

Strengths

  • The probe-path swap is well-targeted: /health calls app.state.memory.health_check() (DB connectivity check) on both API and worker, while /version and /metrics are DB-free — this directly addresses the stated DB-pressure goal without weakening liveness/readiness signal quality.
  • HINDSIGHT_API_WORKER_BATCH_SIZE: "1" and the tightened DB pool sizes are consistent, reasonable throttles for an embedded/shared Postgres deployment, and match the env var names actually read in hindsight_api/config.py.

Recommended Action

  1. Resolve the Critical slot-starvation issue before merge — verify the four now-unreachable operation types are safe to permanently disable, or leave a shared-pool slot.
  2. Everything else in this diff (probe paths, DB pool sizing, batch size) looks correct and ready.

Self-review comment mode: this PR was authored by the same bot identity; formal review/approval must come from a human or a distinct reviewer identity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants