Implement future works for Gache interface#147
Conversation
Implemented the methods `ExtendExpire`, `GetRefresh`, `GetRefreshWithDur`, `GetWithIgnoredExpire`, `Keys`, `Pop`, `SetIfNotExists`, and `SetWithExpireIfNotExists` in the `Gache` interface and `gache` struct. These methods were previously listed as TODOs. The implementation ensures thread safety using atomic operations and CAS loops on the underlying sharded map.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR implements 8 new methods for the Gache interface that were previously marked as "TODO Future works". These methods extend the cache's functionality with operations like expiration extension, refresh-on-read, conditional setting, and key enumeration.
Changes:
- Removed TODO comments for future work items from the Gache interface
- Added 8 new method signatures to the Gache interface
- Implemented all 8 methods with lock-free concurrency using CompareAndSwap patterns
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| g.expiration(key) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
ExtendExpire incorrectly handles values with no expiration (NoTTL). When val.expire is 0 or negative (indicating no expiration per isValid()), adding int64(addExp) to it results in an absolute timestamp near epoch rather than extending from current time. Consider checking if val.expire <= 0 and handling it appropriately, either by returning early or setting expire to fastime.UnixNanoNow() + int64(addExp).
| // If the current value has no TTL (expire <= 0), there is nothing to extend. | |
| if val.expire <= 0 { | |
| return | |
| } |
|
|
||
| // SetWithExpireIfNotExists sets key-value & expiration to Gache if it does not exist. | ||
| func (g *gache[V]) SetWithExpireIfNotExists(key string, val V, d time.Duration) { | ||
| exp := int64(d) |
There was a problem hiding this comment.
Inconsistent conversion pattern from time.Duration to int64. The existing codebase uses unsafe.Pointer conversion (e.g., line 240: *(*int64)(unsafe.Pointer(&expire))), but this line uses a direct cast int64(d). For consistency with the existing pattern, consider using *(*int64)(unsafe.Pointer(&d)).
| exp := int64(d) | |
| exp := *(*int64)(unsafe.Pointer(&d)) |
Implemented future works for Gache interface:
ExtendExpire,GetRefresh,GetRefreshWithDur,GetWithIgnoredExpire,Keys,Pop,SetIfNotExists,SetWithExpireIfNotExists.PR created automatically by Jules for task 10947422724461451886 started by @kpango