Add an in-memory cache with per-entry expiry - #3
Conversation
Entries live for a fixed ttl set at construction. Get treats an expired entry as absent.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
@coderabbitai review |
|
@macroscopeapp review |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new internal/cache package that provides a small in-memory string cache with a construction-time TTL, where reads treat expired entries as absent.
Changes:
- Added
cache.Cachebacked by amap[string]entrywith per-entry expiration timestamps. - Implemented
Put, expiry-awareGet, andLenfor entry counting. - Added unit tests covering basic put/get and expiry behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/cache/cache.go | Adds the cache implementation, including TTL handling, Put, Get, and Len. |
| internal/cache/cache_test.go | Adds tests for basic retrieval and expired-entry behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| it, ok := c.items[key] | ||
| if !ok || time.Now().After(it.expires) { | ||
| return "", false | ||
| } |
| func TestExpiry(t *testing.T) { | ||
| c := New(-time.Second) // already expired | ||
| c.Put("k", "v") | ||
|
|
||
| if _, ok := c.Get("k"); ok { | ||
| t.Error("an expired entry should not be returned") | ||
| } | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3865546c68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !ok || time.Now().After(it.expires) { | ||
| return "", false |
There was a problem hiding this comment.
When a workload inserts many unique keys and later calls Get after they expire, this branch reports each entry as absent but never deletes it from c.items. The map therefore retains every expired key indefinitely and can grow without bound, contrary to Len's documented promise that expired entries are evicted by Get; delete the expired entry before returning.
Useful? React with 👍 / 👎.
| func (c *Cache) Put(key, value string) { | ||
| c.items[key] = entry{value: value, expires: time.Now().Add(c.ttl)} |
There was a problem hiding this comment.
When a Cache is shared by multiple goroutines, concurrent calls involving Put and any of Put, Get, or Len access this map without synchronization. This creates data races and can terminate the process with fatal error: concurrent map read and map write or concurrent map writes; protect all map accesses with a mutex or explicitly provide another concurrency-safe implementation.
Useful? React with 👍 / 👎.
githail
Bot
added
githail:reconciling
githail:incomplete
and removed
githail:reviewing
githail:reconciling
labels
User description
A small cache for values that expire after a fixed lifetime, set once at construction.
Gettreats an expired entry as absent.Lencounts everything held, including entries that have expired but not yet been evicted.Generated description
Below is a concise technical summary of the changes proposed in this PR:
Add an in-memory
Cachewith a construction-time TTL, supporting value insertion, expiry-aware retrieval, and entry counting. Verify normal reads and expired-entry behavior with tests.Latest Contributors(1)
Summary by cubic
Adds a small in-memory cache where entries expire after a fixed lifetime set at construction.
Gettreats an expired entry as absent, whileLenstill counts entries that have expired but not yet been evicted.Written for commit 3865546. Summary will update on new commits.
Note
Add in-memory cache with TTL-based per-entry expiry in
internal/cachecache.Cachebacked by amap[string]entry, where each entry stores a value and an expiration timestamp set tonow + TTLonPutGetreturnsfalsefor missing or expired keys, treating expired entries as absent without removing them from the mapLenreturns the raw map size, so it includes expired-but-not-evicted entries; callers relying onLenfor live counts may over-reportMacroscope summarized 3865546.