feat: write-through cache

[jason-lynch]

Summary

Adds a write-through cache implementation to the storage package. The cache and its operations serve reads from an in-memory map, while writes are served from Etcd and, if successful, persisted to the in-memory map. This cache is useful for areas where we need to perform frequent range queries over small to moderate numbers of small values types. This implementation is designed to be easy and transparent to add to existing stores.

Testing

This cache is used by the following PR, so there are no functional changes from this PR.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 224a7184-5874-4877-a32a-a33cc3351f07

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

A generic in-memory cache layer for etcd-backed values is introduced with thread-safe operations, lifecycle management via Start/Stop, CRUD methods that synchronize with etcd, and integration with transactions to update cached entries after commits.

Changes

Cohort / File(s)	Summary
Cache Implementation server/internal/storage/cache.go, server/internal/storage/cache_test.go	Added generic Cache[V] implementation with prefix watch synchronization, thread-safe operations for Get/GetPrefix/Put/Create/Update/DeleteByKey/DeleteValue/DeletePrefix, error handling via channels, and comprehensive test coverage.
Interface Definitions server/internal/storage/interface.go	Added Cache[V Value] interface defining lifecycle, read, write-through, and error handling contracts; added CachedTxnOp interface for transaction operations to support cache updates.
Transaction Integration server/internal/storage/txn.go	Modified Commit to collect CachedTxnOp implementations and invoke UpdateCache() on each after etcd transaction commits and version updates complete.

Poem

🐰 A cache now blooms, so swift and true,
Through etcd's depths, our values flew,
With watches that hum and locks that hold,
Transactions and tests, a story told!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description includes a Summary section and Testing section but is missing required sections like Changes, Checklist items, and Notes for Reviewers as specified in the template.	Add the missing 'Changes' section with a bulleted list of what changed, complete the 'Checklist' with checkboxes for tests/docs/issue linking, and add 'Notes for Reviewers' section as needed.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat: write-through cache' directly and concisely summarizes the main change: adding a write-through cache implementation to the storage package.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/write-through-cache

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jason-lynch]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[jason-lynch]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

server/internal/storage/cache.go (1)

52-53: Use a package sentinel for the double-start case.

Returning a fresh errors.New(...) here makes this condition hard to match with errors.Is and breaks the package’s error taxonomy. Please promote it to a package-level sentinel such as ErrCacheAlreadyStarted.

As per coding guidelines, "Domain-specific errors should be defined in each package; API errors should be mapped to HTTP status codes via Goa".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/internal/storage/cache.go` around lines 52 - 53, Define a
package-level sentinel error named ErrCacheAlreadyStarted (e.g., var
ErrCacheAlreadyStarted = errors.New("cache already started")) and replace the
inline errors.New call in the method that checks c.op (the block that currently
returns errors.New("cache has already been started")) to return this sentinel
instead; update any callers/tests to use errors.Is(..., ErrCacheAlreadyStarted)
as needed and add a short comment on ErrCacheAlreadyStarted describing its
purpose.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/internal/storage/cache.go`:
- Around line 176-194: The write() path can reintroduce stale data because
deletes remove entries and lose revision/tombstone info; update the cache to
track per-key last-seen revision/tombstone (extend cachedValue with a revision
and tombstone flag) and make all cache mutations (write(), UpdateCache(),
DeleteByKey, DeleteValue, DeletePrefix and watch handlers) accept and compare a
commit/delete revision before applying so only monotonically newer revisions
win; plumbing: propagate the etcd commit/delete revision into the cache-update
calls, never blindly delete the map entry on a watched/local delete (mark as
tombstone with its revision), and ensure write() compares itemVersion and the
stored cachedValue.revision to avoid resurrecting older state.
- Around line 159-173: The PropagateErrors goroutine can block if it receives
from c.errCh and then a cancellation happens before sending to ch; update
cache[V].PropagateErrors to mirror watchOp.PropagateErrors by replacing the
direct send with a nested select: after receiving err := <-c.errCh, do a select
that either sends err to ch or observes ctx.Done() (and skips/returns) so the
goroutine never blocks on a full/unconsumed ch after cancellation; reference
c.errCh, ch, ctx.Done() and the PropagateErrors method when making the change.

---

Nitpick comments:
In `@server/internal/storage/cache.go`:
- Around line 52-53: Define a package-level sentinel error named
ErrCacheAlreadyStarted (e.g., var ErrCacheAlreadyStarted = errors.New("cache
already started")) and replace the inline errors.New call in the method that
checks c.op (the block that currently returns errors.New("cache has already been
started")) to return this sentinel instead; update any callers/tests to use
errors.Is(..., ErrCacheAlreadyStarted) as needed and add a short comment on
ErrCacheAlreadyStarted describing its purpose.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 34f23d93-7b12-4b05-b647-f79080112650

📥 Commits

Reviewing files that changed from the base of the PR and between fa49e10 and 31df024.

📒 Files selected for processing (4)

• server/internal/storage/cache.go
• server/internal/storage/cache_test.go
• server/internal/storage/interface.go
• server/internal/storage/txn.go