Skip to content

fix: tier 1 correctness and ops fixes (AOF recovery, healthcheck, helm secrets) - #355

Merged
kacy merged 1 commit into
mainfrom
cleanup/tier1-correctness
Jul 3, 2026
Merged

fix: tier 1 correctness and ops fixes (AOF recovery, healthcheck, helm secrets)#355
kacy merged 1 commit into
mainfrom
cleanup/tier1-correctness

Conversation

@kacy

@kacy kacy commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Part 1 of a 5-PR cleanup series from a full-codebase audit.

What

  • AOF recovery (correctness): on a mid-file CRC mismatch, recover_shard previously kept the partially-applied AOF prefix in the recovered state while logging "using snapshot state only". The kept prefix could be internally inconsistent (a SET applied, a later DEL lost — a state that never existed). Recovery now snapshots the pre-replay state and rolls back to it on error, making the behavior match the log. Two new tests cover mid-file corruption and the inconsistent-prefix case; both fail against the old code.
  • Shutdown fsync: the final AOF sync() error on clean shutdown was discarded (let _ =); it's now logged as an error since it means writes may be lost.
  • Dockerfile HEALTHCHECK: the previous check (wget against the metrics port) could never pass — wget isn't installed in the runtime image and the metrics port is disabled by default. Switched to ember-server --healthcheck (what docker-compose already uses).
  • Helm secrets: requirepass was injected as a plaintext env var visible in kubectl describe. The chart now creates a Secret (or uses ember.existingSecret) and mounts it as a file via EMBER_REQUIREPASS_FILE. Also: appVersion now tracks the real release (0.4.9) instead of latest, and the default image repository is fully qualified.

Verification

  • cargo test -p ember-persistence -p emberkv-core --features protobuf: 708 tests pass; the two new recovery tests were confirmed to fail with the fix reverted.
  • cargo clippy -D warnings clean on touched crates.
  • Helm changes not rendered locally (no helm binary in this environment) — relying on the CI helm lint/helm template job; please eyeball the Secret template.

Note (pre-existing, unchanged): --healthcheck sends PING, so with requirepass set the container reports unhealthy — same limitation the docker-compose healthcheck already has. Worth a follow-up if auth'd deployments need docker healthchecks.

- AOF recovery: roll back to snapshot state on mid-file corruption
  instead of silently keeping a partially-applied (potentially
  inconsistent) AOF prefix while claiming snapshot-only recovery;
  add tests for mid-file CRC corruption and the SET-kept/DEL-lost
  inconsistency case
- shard: log the final AOF sync error on shutdown instead of
  swallowing it
- Dockerfile: fix HEALTHCHECK — wget is not installed in the runtime
  image and the metrics port is disabled by default; use the built-in
  ember-server --healthcheck instead (same as docker-compose)
- helm: stop injecting requirepass as a plaintext env var; store it
  in a Secret mounted as a file via EMBER_REQUIREPASS_FILE, support
  existingSecret; set appVersion to 0.4.9, qualify image repository

Claude-Session: https://claude.ai/code/session_017uab7k2MyLpWAQwFDNt4tw
@kacy
kacy merged commit 1be2250 into main Jul 3, 2026
7 of 8 checks passed
@kacy
kacy deleted the cleanup/tier1-correctness branch July 3, 2026 16:14
kacy added a commit that referenced this pull request Jul 3, 2026
- AOF recovery: roll back to snapshot state on mid-file corruption
  instead of silently keeping a partially-applied (potentially
  inconsistent) AOF prefix while claiming snapshot-only recovery;
  add tests for mid-file CRC corruption and the SET-kept/DEL-lost
  inconsistency case
- shard: log the final AOF sync error on shutdown instead of
  swallowing it
- Dockerfile: fix HEALTHCHECK — wget is not installed in the runtime
  image and the metrics port is disabled by default; use the built-in
  ember-server --healthcheck instead (same as docker-compose)
- helm: stop injecting requirepass as a plaintext env var; store it
  in a Secret mounted as a file via EMBER_REQUIREPASS_FILE, support
  existingSecret; set appVersion to 0.4.9, qualify image repository
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.

1 participant