Delete expired items before setting #132
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@patrickmn for your review 🙏 |
jaimem88
force-pushed
the
fix/48-delete-before-setting
branch
from
0293691
to
59cc448
Compare
ayufan
reviewed
Oct 13, 2020
Check if an item exists and it has expired before setting so that onEvicted is actually called. Fixes patrickmn#48. Add test and split func Use nanoseconds instead Call onEvicted if item existed before setting Call onEvicted if item.Expired
jaimem88
force-pushed
the
fix/48-delete-before-setting
branch
from
aee9b1a
to
38560f5
Compare
|
No. Always.
We overwrite an existing item. It should be comparison tha the item that we
overwrite is a different one. So, this comparison is deliberate. Usage of
expired produces wrong behavior.
…On Thu, 15 Oct 2020 at 01:58, Jaime Martinez ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In cache.go
<#132 (comment)>:
> @@ -57,6 +57,10 @@ func (c *cache) Set(k string, x interface{}, d time.Duration) {
if d > 0 {
e = time.Now().Add(d).UnixNano()
}
+
+ // delete before setting and call onEvicted
+ c.deleteIfExpired(k)
updated :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#132 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASOSQK3G7GDYXNVFTMGGCLSKY3KJANCNFSM4SNYEWEA>
.
|
|
@patrickmn Would you mind looking at this? |
|
Note this is a incompatible change, and some people may be relying on the current behaviour. Also this library doesn't seem maintained at the moment, so little chance it will get merged. It's better to just maintain your own fork if you need this behaviour. |
|
Trying to find a cache with TTL that has been rigorously tested and this seems to be a popular choice. |
Repository owner
deleted a comment from
pablodz
Jan 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check if an item exists and it has expired before setting so that
onEvicted is actually called.
Fixes #48.