Spike: persistent, multi-instance-safe storage — recommend Redis, prototype GigService

[balisdev]

Summary

Closes #181

Every domain service in this backend — EscrowService, GigService, UserProfileService, IpfsPinningService, NonceStoreService, plus several worker/saga stores found during the inventory — keeps its state in a process-local Map. This spike:

• Inventories every in-memory store in the codebase (§1 of the write-up), not just the five named in the issue.
• Evaluates Postgres, DynamoDB, and Redis against the actual access patterns found, and recommends Redis: it's already a hard dependency (REDIS_URL is documented as required in .env.example), and NonceStoreService/RateLimitGuard already establish a working, tested pattern for it in this exact codebase — every inventoried access pattern (point lookups, small indices, time-ordered range scans) maps directly onto Redis strings/sets/sorted sets, so this is consistent with existing practice, not new infrastructure.
• Flags EscrowService as a deliberate exception: it's money-adjacent/audit-sensitive, so the spike leaves the Redis-vs-relational call open rather than defaulting it, and hands that decision to its own follow-up issue.
• Prototypes the recommendation for GigService (the newest/smallest domain service, per the issue's own suggestion): gigs now persist as SET gig:{id}, with sorted sets for creation-order listing and open-gig expiry lookups, and a set per creator for the by-creator index — all written atomically via MULTI. Falls back to the original in-memory Map when Redis is unavailable, matching NonceStoreService's convention, logged at error level since that fallback reintroduces the exact divergence this spike exists to fix.
• Confirms compatibility with Add distributed lock so background sweep workers run as a single active instance #182 (the already-filed distributed-lock issue for the sweep workers) — same Redis instance, no conflicting infra choice. Locking itself is out of scope here.
• Flags blocking unknowns explicitly: no live Redis available to validate against in this environment/CI (mocked ioredis only, consistent with the rest of the repo's Redis tests), Redis durability/backup configuration unverified, single-instance Redis is a SPOF, and IpfsPinningService's raw-content Buffer map is a poor fit for Redis as general KV storage.

Full write-up: backend/PERSISTENT_STORAGE_SPIKE.md.

Follow-up implementation issues filed and linked from #181:

• Migrate EscrowService off in-memory storage #187 — Migrate EscrowService (Redis vs. relational decision left open)
• Migrate UserProfileService off in-memory storage #188 — Migrate UserProfileService
• Migrate IpfsPinningService's pin registry off in-memory storage #189 — Migrate IpfsPinningService's pin registry
• Migrate remaining worker/saga in-memory state stores to Redis #190 — Migrate the remaining smaller stores (DisputeSagaService, EscrowReconciliationStateStore, EventProcessorService, LedgerCursorService, MigrationStateStore)

Test plan

• npm run lint:check — clean (pre-existing warnings only, unrelated to this change)
• npm run format:check — clean
• npx tsc --noEmit — clean
• npm run test:ci — 356/356 tests passing (GigService's suite runs its full behavior twice — once against a mocked Redis client, once against the in-memory fallback — plus a dedicated Redis-failure test)
• npm run build — clean

[meshackyaro]

Summary

Closes #181

Every domain service in this backend — EscrowService, GigService, UserProfileService, IpfsPinningService, NonceStoreService, plus several worker/saga stores found during the inventory — keeps its state in a process-local Map. This spike:

• Inventories every in-memory store in the codebase (§1 of the write-up), not just the five named in the issue.
• Evaluates Postgres, DynamoDB, and Redis against the actual access patterns found, and recommends Redis: it's already a hard dependency (REDIS_URL is documented as required in .env.example), and NonceStoreService/RateLimitGuard already establish a working, tested pattern for it in this exact codebase — every inventoried access pattern (point lookups, small indices, time-ordered range scans) maps directly onto Redis strings/sets/sorted sets, so this is consistent with existing practice, not new infrastructure.
• Flags EscrowService as a deliberate exception: it's money-adjacent/audit-sensitive, so the spike leaves the Redis-vs-relational call open rather than defaulting it, and hands that decision to its own follow-up issue.
• Prototypes the recommendation for GigService (the newest/smallest domain service, per the issue's own suggestion): gigs now persist as SET gig:{id}, with sorted sets for creation-order listing and open-gig expiry lookups, and a set per creator for the by-creator index — all written atomically via MULTI. Falls back to the original in-memory Map when Redis is unavailable, matching NonceStoreService's convention, logged at error level since that fallback reintroduces the exact divergence this spike exists to fix.
• Confirms compatibility with Add distributed lock so background sweep workers run as a single active instance #182 (the already-filed distributed-lock issue for the sweep workers) — same Redis instance, no conflicting infra choice. Locking itself is out of scope here.
• Flags blocking unknowns explicitly: no live Redis available to validate against in this environment/CI (mocked ioredis only, consistent with the rest of the repo's Redis tests), Redis durability/backup configuration unverified, single-instance Redis is a SPOF, and IpfsPinningService's raw-content Buffer map is a poor fit for Redis as general KV storage.

Full write-up: backend/PERSISTENT_STORAGE_SPIKE.md.

Follow-up implementation issues filed and linked from #181:

• Migrate EscrowService off in-memory storage #187 — Migrate EscrowService (Redis vs. relational decision left open)
• Migrate UserProfileService off in-memory storage #188 — Migrate UserProfileService
• Migrate IpfsPinningService's pin registry off in-memory storage #189 — Migrate IpfsPinningService's pin registry
• Migrate remaining worker/saga in-memory state stores to Redis #190 — Migrate the remaining smaller stores (DisputeSagaService, EscrowReconciliationStateStore, EventProcessorService, LedgerCursorService, MigrationStateStore)

Test plan

• npm run lint:check — clean (pre-existing warnings only, unrelated to this change)
• npm run format:check — clean
• npx tsc --noEmit — clean
• npm run test:ci — 356/356 tests passing (GigService's suite runs its full behavior twice — once against a mocked Redis client, once against the in-memory fallback — plus a dedicated Redis-failure test)
• npm run build — clean

Thanks — this is an excellent, thorough spike and a very clean prototype. I appreciate the inventory, the trade-off reasoning, and the GigService implementation that falls back to the in-memory Map when Redis is unavailable. Tests, lint, and build passing is great.

A few required changes before I merge:

1. Document runtime behavior and expectations

   • In PERSISTENT_STORAGE_SPIKE.md (or README) clearly state the runtime fallback behavior: when Redis is unavailable the service falls back to the in-memory Map, the fallback is logged at error level, and what that means for data consistency in production.
   • Add an explicit recommendation for production (e.g., require REDIS_URL / fail-fast in prod, or list the circumstances where fallback is acceptable).

2. Add an integration smoke test against a real Redis instance

   • The unit tests exercising the mocked ioredis + in-memory fallback are good, but please add a CI smoke test (Docker Testcontainers or running redis:alpine in CI) to validate the real Redis interactions (MULTI/EXEC, sorted set behavior, TTLs/expiry). This guards against subtle behavior differences between the mock and production Redis.

3. Metrics and observability for the fallback path

   • Increment a metric (e.g. gig_persistence_fallback_total) and/or emit a structured log when the Redis fallback is used. Relying on an error log alone will make it hard to alert reliably.
   • Add brief guidance for an alert (e.g., alert if fallback rate > 0 for > 5m in prod).

4. Error handling / transaction verification

   • Ensure the code checks EXEC results and surfaces transaction failures (and retries or fails fast as appropriate). If not already covered, add a unit test that simulates EXEC failing and verifies behavior.

5. Ops notes: Redis durability & Sizing

   • Add a short operational note in the spike doc about Redis durability (AOF/RDB), replication, and the SPOF risk for single-instance Redis, and recommend next steps or ops owners to review this before rolling to prod.

6. Follow-up issue clarity

   • The PR links follow-ups (Migrate EscrowService off in-memory storage #187–Migrate remaining worker/saga in-memory state stores to Redis #190) — please add a one-line owner/priority for these (who's expected to pick each up and whether any must be done before a rollout).

Minor nits / suggestions:

• Consider making the fallback behaviour configurable by environment (e.g., allow fallback in dev/test, but fail startup in production).
• Add a small README section showing how to run the GigService examples locally (with and without Redis) so reviewers and devs can reproduce.

Overall: I'm strongly +1 on the direction and the recommendation to use Redis for these patterns. Address items 1–4 (docs, integration test, metrics, and transaction verification), push an update, and I will approve/merge.

[balisdev]

Thanks for the thorough review — pushed a follow-up commit addressing all 6 required items:

1. Runtime behavior/production recommendation documented — PERSISTENT_STORAGE_SPIKE.md §6 now spells out the fallback behavior and its implications, plus the new production fail-fast (see #4 below).
2. Real-Redis integration smoke test added — gig.service.redis-integration.spec.ts runs against an actual Redis (no mocking), covering MULTI/EXEC atomicity and sorted-set/set semantics. CI now spins up a redis:7-alpine service container and sets REDIS_URL for the test step. Verified locally against a real container before pushing (5/5 passing). GigService itself doesn't use key TTLs (its lifecycle is status-driven, not expiry-driven), so there's nothing TTL-specific to add — noted that explicitly in the test file's docstring.
3. Metrics/observability — every fallback now increments gig_persistence_fallback_total{operation="..."} via the existing MetricsService (exposed at /metrics). Added alerting guidance in §6 (alert on sustained non-zero over 5m).
4. Transaction verification — assertTransactionOk() inspects MULTI/EXEC's per-command results array (and a null exec() result) and treats any failed command as a full failure, since exec() can resolve successfully even when one queued command failed. Covered by dedicated unit tests.
5. Ops notes expanded — §7 now includes an AOF-vs-RDB recommendation and a sizing/eviction-policy note.
6. Follow-up issue clarity — added an owner/priority/blocks-rollout table to §8, and reflected the same on Migrate EscrowService off in-memory storage #187–Migrate remaining worker/saga in-memory state stores to Redis #190 themselves.

Also took the minor nits: fallback is now environment-gated (fails app startup in production if Redis isn't configured; still falls back in dev/test), and added a "running it locally" subsection to §4 with copy-pasteable commands for both the with- and without-Redis paths.

Full CI suite (lint, format, tsc, tests including the new integration suite, build) verified clean locally with a real Redis container before pushing.

[meshackyaro]

Thanks for the thorough review — pushed a follow-up commit addressing all 6 required items:

1. Runtime behavior/production recommendation documented — PERSISTENT_STORAGE_SPIKE.md §6 now spells out the fallback behavior and its implications, plus the new production fail-fast (see #4 below).
2. Real-Redis integration smoke test added — gig.service.redis-integration.spec.ts runs against an actual Redis (no mocking), covering MULTI/EXEC atomicity and sorted-set/set semantics. CI now spins up a redis:7-alpine service container and sets REDIS_URL for the test step. Verified locally against a real container before pushing (5/5 passing). GigService itself doesn't use key TTLs (its lifecycle is status-driven, not expiry-driven), so there's nothing TTL-specific to add — noted that explicitly in the test file's docstring.
3. Metrics/observability — every fallback now increments gig_persistence_fallback_total{operation="..."} via the existing MetricsService (exposed at /metrics). Added alerting guidance in §6 (alert on sustained non-zero over 5m).
4. Transaction verification — assertTransactionOk() inspects MULTI/EXEC's per-command results array (and a null exec() result) and treats any failed command as a full failure, since exec() can resolve successfully even when one queued command failed. Covered by dedicated unit tests.
5. Ops notes expanded — §7 now includes an AOF-vs-RDB recommendation and a sizing/eviction-policy note.
6. Follow-up issue clarity — added an owner/priority/blocks-rollout table to §8, and reflected the same on Migrate EscrowService off in-memory storage #187–Migrate remaining worker/saga in-memory state stores to Redis #190 themselves.

Also took the minor nits: fallback is now environment-gated (fails app startup in production if Redis isn't configured; still falls back in dev/test), and added a "running it locally" subsection to §4 with copy-pasteable commands for both the with- and without-Redis paths.

Full CI suite (lint, format, tsc, tests including the new integration suite, build) verified clean locally with a real Redis container before pushing.

Thanks — I appreciate the quick follow-up and the thoroughness here. I reviewed the updates and, CI passes, this looks ready to merge.

What I checked and am happy to see:

• The spike doc now documents the runtime fallback behavior and gives a clear production recommendation.
• Fallback-to-memory is configurable (disabled/strict in prod) so we don't silently reintroduce divergence in production.
• An integration smoke test was added that runs against a real Redis (dockerized in CI) and covers MULTI/EXEC, sorted-set behavior, and TTL/expiry semantics.
• A metric (e.g. gig_persistence_fallback_total) is emitted on fallback and structured logs remain in place.
• The Redis MULTI/EXEC path checks results and has a test exercising an EXEC failure path.
• Ops notes about durability, replication, and SPOF were added to the spike doc.
• Follow-up issues (Migrate EscrowService off in-memory storage #187–Migrate remaining worker/saga in-memory state stores to Redis #190) now include owners/priorities.

Two final asks before I merge:

1. If the CI Redis smoke test is flaky in the runner, add a short retry or health-check step so it doesn't produce intermittent failures.
2. Add a one-line entry in CHANGELOG.md (or the spike doc header) noting the new dependency/requirement for production deployments (Redis required / fail-fast).

If those two small items are added (or you prefer to follow up with a quick patch), Looks good to me and ready to merge.

Thanks again — this moves us forward on a safe, well-tested path to persistent, multi-instance-safe storage.

[balisdev]

Both addressed:

1. CI Redis reliability — the service container already had --health-cmd "redis-cli ping" gating job startup, but added an explicit "Wait for Redis" retry step (15×2s TCP-connect retries) right before the test step as a belt-and-suspenders guard, in case the health check passes before the port is fully accepting connections in a given run.
2. New production requirement flagged — added a one-line ⚠️ New production requirement callout right in PERSISTENT_STORAGE_SPIKE.md's header (no CHANGELOG.md exists in this repo yet, so went with the doc-header alternative you offered) noting REDIS_URL is now required under NODE_ENV=production.

Pushed as a830557. Thanks again for the review — let me know if there's anything else before merge.

[meshackyaro]

Approved — great work.

This is a thorough, well-reasoned spike and a clean prototype. CI is green and I reviewed the updates: the spike doc, the configurable fallback (disabled by default in prod), the real-Redis integration smoke test in CI, the fallback metric, MULTI/EXEC transaction checks and tests, ops notes on durability/replication/SPOF, and the follow-up issues with owners/priorities.

Thanks for the excellent work.