Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request Get Expiration With Get #56

Closed
MrSametBurgazoglu opened this issue Feb 21, 2024 · 10 comments · Fixed by #65
Closed

Request Get Expiration With Get #56

MrSametBurgazoglu opened this issue Feb 21, 2024 · 10 comments · Fixed by #65
Labels
enhancement New feature or request

Comments

@MrSametBurgazoglu
Copy link

Clarification and motivation

I have a script that updating some variables and users get them from cache. But I want to get expiration for showing them last update time.

Acceptance criteria

Get expiration time with get function or new function with expiration time

@MrSametBurgazoglu MrSametBurgazoglu added the enhancement New feature or request label Feb 21, 2024
@maypok86
Copy link
Owner

Hi, thanks for the issue.

I was going to rework the API a bit later. For now you can use something like this.

package main

import (
	"time"

	"github.com/maypok86/otter"
)

type WithTime[V any] struct {
	Value     V
	UpdatedAt time.Time // or in seconds
}

type Cache[K comparable, V any] struct {
	cache otter.Cache[K, WithTime[V]]
}

func NewCache[K comparable, V any](capacity int) (*Cache[K, V], error) {
	b, err := otter.NewBuilder[K, WithTime[V]](capacity)
	if err != nil {
		return nil, err
	}

	c, err := b.WithTTL(time.Hour).Build()
	if err != nil {
		return nil, err
	}

	return &Cache[K, V]{
		cache: c,
	}, nil
}

func (c *Cache[K, V]) Get(key K) (WithTime[V], bool) {
	return c.cache.Get(key)
}

func (c *Cache[K, V]) Set(key K, value V) bool {
	updatedAt := time.Now()
	return c.cache.Set(key, WithTime[V]{
		Value:     value,
		UpdatedAt: updatedAt,
	})
}

maypok86 added a commit that referenced this issue Mar 7, 2024
@maypok86 maypok86 mentioned this issue Mar 7, 2024
7 tasks
@maypok86
Copy link
Owner

maypok86 commented Mar 7, 2024

@MrSametBurgazoglu @proost

What do you think of this solution to this problem? It's a bit non-standard for Go, but allows otter to hide a lot of future features.

c, err := MustBuilder[int, int](256).
	WithTTL(time.Hour).
	Build()

...

// if you want to get the value without updating the eviction policy
value, ok := c.Advanced().GetQuietly(1)

...

// if you want to get an entry
entry, ok := c.Advanced().GetEntry(1)

...

// if you want to get an entry without updating the eviction policy
entry, ok := c.Advanced().GetEntryQuietly(1)

...

key := entry.Key()
value := entry.Value()
cost := entry.Cost()
expiration := entry.Expiration()
ttl := entry.TTL()
hasExpired := entry.HasExpired()

@proost
Copy link

proost commented Mar 9, 2024

@maypok86
First, I want this feature because i plan to apply PER

your proposed API can hide underneath a lot of features. what do you expect to user if user get cost, ttl?

GetEntryQuietly is good to mitigate cache miss storm. what is meaning of parameter?

And I think Advanced() is not intuitive name for accessing additional cache functionality. how about Details() or WithMetadata()?

@maypok86
Copy link
Owner

maypok86 commented Mar 9, 2024

your proposed API can hide underneath a lot of features. what do you expect to user if user get cost, ttl?

I assumed something like this

key := 1

...

func (c *Cache[K, V]) GetCost(key K) (uint32, error) {
    entry, ok := c.Advanced().GetEntryQuietly(key)
    if !ok {
        return 0, errors.New("a cache entry was not found")
    }
    return entry.Cost(), nil
}

what is meaning of parameter?

In all cases, 1 is just a key.

Advanced() is not intuitive name for accessing additional cache functionality.

This is a difficult moment. It is unclear which name is better to give. I was also thinking about something like Features or Details, but I'm very likely to add more than just functions to get values there, and then Details will become strange.

@proost
Copy link

proost commented Mar 10, 2024

This is a difficult moment. It is unclear which name is better to give. I was also thinking about something like Features or Details, but I'm very likely to add more than just functions to get values there, and then Details will become strange.

how about Extended()? Advanced() is unfamiliar name.

your proposed API can hide underneath a lot of features. what do you expect to user if user get cost, ttl?

I mean what is use case if user can get cost or ttl?

@maypok86
Copy link
Owner

how about Extended()?

Hmm, it looks interesting. But probably just Extension will be clearer.

@maypok86
Copy link
Owner

@proost So, I think we've come to an agreement, right?

@MrSametBurgazoglu
Copy link
Author

@maypok86 I apologize for not being able to answer. Speaking for myself, this is a good solution. But I didn't understand the meaning of updating eviction policy.

@maypok86
Copy link
Owner

maypok86 commented Mar 12, 2024

@MrSametBurgazoglu During a Get operation, caches usually additionally update the eviction policy to better predict eviction when the maximum size is reached based on recency (lru) and frequency (lfu). And more effective eviction policies use both of these metrics. But sometimes there are situations when you need to complete several requests that are not related to the main workload and you do not want them to damage the eviction policy. That is, if you want the cache on these requests to work simply as a hash table, you can use the GetQuietly or GetEntryQuietly methods.

@proost
Copy link

proost commented Mar 12, 2024

@maypok86 I agree. Thank you for a new feature!

maypok86 added a commit that referenced this issue Mar 12, 2024
maypok86 added a commit that referenced this issue Mar 12, 2024
maypok86 added a commit that referenced this issue Mar 12, 2024
maypok86 added a commit that referenced this issue Mar 12, 2024
maypok86 added a commit that referenced this issue Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants