feat: persist rolling-quota usage to SQLite (usage_db)#176
feat: persist rolling-quota usage to SQLite (usage_db)#176dobby-coder[bot] wants to merge 1 commit into
Conversation
The config schema already accepts `usage_db = "<path>"`, but `CryptifyConfig` never parsed it and the rolling-quota state lived only in the in-memory `Store.shared.state.usage` map, so any pod restart or redeploy wiped everyone's rolling quota. - Parse `usage_db: Option<String>` in `RawCryptifyConfig` / `CryptifyConfig` with a `usage_db()` accessor. - Back the usage map with SQLite (`rusqlite`, bundled) at that path. The DB is the source of truth; the in-memory map is a cache. On startup existing usage is loaded into the cache; every accounted upload is written through and stale rows outside the rolling window are pruned. - `usage_db` unset → in-memory only, preserving prior behaviour. A configured-but-unopenable DB panics at startup (loud config error). - Tests: persistence across a simulated restart, continued accumulation after restart, rolling-window eviction persisted to the DB, lazy in-memory eviction after reload, and config parsing. Closes #134 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Rules Dobby 2 — gatekeeper sign-off. ✅
Verdict is APPROVE — posted as COMMENT only because GitHub blocks approving your own PR (this PR is authored by dobby-coder[bot], the same identity reviewing it). Treat this as a clean approval, not withheld.
Ran the per-rule compliance sweep (all rules + cryptify repo-specific notes) and merged it with Review Dobby 2's findings. No blocking issues.
- Delivers everything the issue asked for:
usage_dbparsed inCryptifyConfig, rolling-quota state backed by SQLite as the source of truth, existing usage loaded on startup, every accounted upload written through, in-memory map kept as a cache. - Tests cover persistence across a simulated restart and rolling-window eviction (incl. eviction surviving restart). Previous step: build OK, 125 tests pass, fmt + clippy clean.
- Crash-safe write ordering (DB before cache) and panic-on-unopenable-DB at startup match the repo's config conventions.
- Test harness follows the documented figment +
#[rocket::async_test]patterns; temp DBs are UUID-isolated and clean up WAL/SHM sidecars.
One non-blocking nit left inline (unbounded usage table growth for one-shot/quiet senders) — pre-existing behaviour, not a regression, safe to defer.
| /// active senders. Errors are logged rather than propagated: a database | ||
| /// hiccup must not fail an otherwise-successful upload, and the in-memory | ||
| /// cache still reflects the record for the lifetime of the process. | ||
| fn record(&self, email: &str, bytes: u64, now: i64) { |
There was a problem hiding this comment.
Non-blocking nit: UsageDb::record only DELETEs stale rows for the email currently being written, so senders who upload once and then go quiet keep their rows in the usage table forever, and load_all() reloads ALL historical rows into memory on every startup (it intentionally does not prune). Quota stays correct (the in-memory cache is pruned lazily per-email on read), so this is not a correctness issue — the table and startup memory just grow with the total number of senders ever seen. The pre-existing in-memory behaviour also kept entries until next access, so this is a minor long-term cleanup, not a regression. Fine to defer.
There was a problem hiding this comment.
/dobby fix this issue. Clean entries from the db older than the quota limit itself
There was a problem hiding this comment.
/dobby fix this issue. Clean entries from the db older than the quota limit itself
|
Dobby looked at this but isn't routing it anywhere: The /dobby fix command requests a code change but no Code Change pipeline is available among the candidates If that's wrong, ping Dobby again or be explicit with |
|
Dobby looked at this but isn't routing it anywhere: The dobby fix command requests a code change but no Code Change pipeline is among the candidates If that's wrong, ping Dobby again or be explicit with |
|
/dobby fix — PR 176 (feat: persist rolling-quota usage to SQLite) has two failing CI checks: 'Build (amd64)' and 'Build (arm64)'. The root cause is that libsqlite3-sys v0.38.1 uses the unstable Rust feature cfg_select (rust-lang issue #115585), which is not yet stabilised in the Rust toolchain version pinned in the Dockerfile. Fix by pinning libsqlite3-sys to a version that does not use cfg_select (e.g. 0.30.x or whichever is the latest that compiles against the current pinned Rust toolchain), or by upgrading the Rust toolchain in the Dockerfile to a nightly/recent stable that includes cfg_select stabilisation. The expected end state is that both Build (amd64) and Build (arm64) jobs pass with the same Dockerfile Rust version or with a justified toolchain bump. |
Closes #134.
What
The config schema already accepts
usage_db = "<path>"(conf/config.dev.toml:13), butCryptifyConfignever parsed it and rolling-quota state lived only in the in-memoryStore.shared.state.usagemap. Any pod restart or redeploy wiped everyone's rolling quota, with no way to inspect/reset a single user without a code change.This backs the usage map with SQLite at the
usage_dbpath so quota survives restarts.Changes
src/config.rs— parseusage_db: Option<String>inRawCryptifyConfig/CryptifyConfig, addusage_db()accessor.src/store.rs— newUsageDb(rusqlite,bundledfeature). The DB is the source of truth; the in-memory map is a cache:load_all, ordered oldest-first).record_uploadwrites through to SQLite before updating the cache, then prunes rows outside the rolling window for that sender (DB stays bounded for active users).usage_dbunset → in-memory only, exactly as before. A configured-but-unopenable DB panics at startup (loud config error, same posture as a malformed config).src/main.rs—build_rocketpassesconfig.usage_db()intoStore::with_idle_ttl.Tests
All pass (
cargo test --all-targets: 127 tests), pluscargo fmt --all --checkandcargo clippy --all-targets -- -D warningsclean.New tests use a real on-disk temp SQLite file:
usage_survives_simulated_restart— record, drop store, reopen same path → usage reloaded, per-sender isolation preserved.restart_continues_accumulating— post-restart records add to reloaded totals.rolling_window_eviction_persists_across_restart— a record outside the window is deleted from the DB (not just the cache) and does not resurrect after restart.rolling_window_evicts_in_memory_after_reload— lazy in-memory eviction relative to callernow.config::usage_db_is_parsed_when_present/..._defaults_to_none_when_absent.Out of scope (follow-ups from the issue)
cryptify_data_dirlives in the postguard-ops Terraform repo, not here.🤖 Generated with Claude Code